On Wed, 2015-12-23 at 10:06 +1100, NeilBrown wrote:
> 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.
I agree, the typecasting with __get_free_page is pretty ugly. I'll
change it back to kzalloc.
Thanks,
-Vishal
signature.asc
Description: This is a digitally signed message part
