On Wed, 2015-12-23 at 10:06 +1100, NeilBrown wrote:

On Wed, Dec 23 2015, Verma, Vishal L wrote:

quoted

On Tue, 2015-12-22 at 16:34 +1100, NeilBrown wrote:

quoted

On Sat, Dec 05 2015, Verma, Vishal L wrote:

quoted

On Fri, 2015-12-04 at 15:30 -0800, James Bottomley wrote:
[...]

quoted

quoted

+ssize_t badblocks_store(struct badblocks *bb, const char
*page,
size_t len,
+			int unack)

[...]

quoted

+int badblocks_init(struct badblocks *bb, int enable)
+{
+	bb->count = 0;
+	if (enable)
+		bb->shift = 0;
+	else
+		bb->shift = -1;
+	bb->page = kmalloc(PAGE_SIZE, GFP_KERNEL);

Why not __get_free_page(GFP_KERNEL)?  The problem with kmalloc
of
an
exactly known page sized quantity is that the slab tracker for
this
requires two contiguous pages for each page because of the
overhead.

Cool, I didn't know about __get_free_page - I can fix this up
too.

I was reminded of this just recently I thought I should clear up
the
misunderstanding.

kmalloc(PAGE_SIZE) does *not* incur significant overhead and
certainly
does not require two contiguous free pages.
If you "grep kmalloc-4096 /proc/slabinfo" you will note that both
objperslab and pagesperslab are 1.  So one page is used to store
each
4096 byte allocation.

To quote the email from Linus which reminded me about this

quoted

If you
want to allocate a page, and get a pointer, just use
"kmalloc()".
Boom, done!

https://lkml.org/lkml/2015/12/21/605

There probably is a small CPU overhead from using kmalloc, but no
memory
overhead.

Thanks Neil.
I just read the rest of that thread - and I'm wondering if we should
change back to kzalloc here.

The one thing __get_free_page gets us is PAGE_SIZE-aligned memory.
Do
you think that would be better for this use? (I can't think of any).
If
not, I can send out a new version reverting back to kzalloc.

kzalloc(PAGE_SIZE) will also always return page-aligned memory.
kzalloc returns a void*, __get_free_page returns unsigned long.  For
that reason alone I would prefer kzalloc.

But I'm not necessarily suggesting you change the code.  I just wanted
to clarify a misunderstanding.  You should produce the
code that you are
most happy with.

I agree, the typecasting with __get_free_page is pretty ugly. I'll
change it back to kzalloc.

Thanks,
	-Vishal