Thread (21 messages) 21 messages, 5 authors, 2015-12-23

Re: [PATCH v2 1/3] badblocks: Add core badblock management code

From: NeilBrown <hidden>
Date: 2015-12-22 23:06:26
Also in: linux-scsi

On Wed, Dec 23 2015, Verma, Vishal L wrote:
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.

NeilBrown

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help