On Wed, Dec 23 2015, Verma, Vishal L wrote:
> On Tue, 2015-12-22 at 16:34 +1100, NeilBrown wrote:
>> On Sat, Dec 05 2015, Verma, Vishal L wrote:
>>
>> > On Fri, 2015-12-04 at 15:30 -0800, James Bottomley wrote:
>> > [...]
>> > > > +ssize_t badblocks_store(struct badblocks *bb, const char *page,
>> > > > size_t len,
>> > > > + int unack)
>> > > [...]
>> > > > +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
>>
>> > 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
signature.asc
Description: PGP signature
