lkml.org 
[lkml]   [2008]   [Oct]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [BUG] SLOB's krealloc() seems bust
From
Date

On Wed, 2008-10-08 at 16:11 +1100, Nick Piggin wrote:
> On Wednesday 08 October 2008 15:54, Nick Piggin wrote:
> > On Wednesday 08 October 2008 15:46, Matt Mackall wrote:
> > > On Wed, 2008-10-08 at 15:22 +1100, Nick Piggin wrote:
> > > > On Wednesday 08 October 2008 10:08, Matt Mackall wrote:
>
> > > > > diff -r 5e32b09a1b2b mm/slob.c
> > > > > --- a/mm/slob.c Fri Oct 03 14:04:43 2008 -0500
> > > > > +++ b/mm/slob.c Tue Oct 07 18:05:15 2008 -0500
> > > > > @@ -514,9 +514,11 @@
> > > > > return 0;
> > > > >
> > > > > sp = (struct slob_page *)virt_to_page(block);
> > > > > - if (slob_page(sp))
> > > > > - return ((slob_t *)block - 1)->units + SLOB_UNIT;
> > > > > - else
> > > > > + if (slob_page(sp)) {
> > > > > + int align = max(ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
> > > > > + unsigned int *m = (unsigned int *)(block - align);
> > > > > + return SLOB_UNITS(*m); /* round up */
> > > > > + } else
> > > > > return sp->page.private;
> > > > > }
> > > >
> > > > Yes, I came up with nearly the same patch before reading this
> > > >
> > > > --- linux-2.6/mm/slob.c 2008-10-08 14:43:17.000000000 +1100
> > > > +++ suth/mm/slob.c 2008-10-08 15:11:06.000000000 +1100
> > > > @@ -514,9 +514,11 @@ size_t ksize(const void *block)
> > > > return 0;
> > > >
> > > > sp = (struct slob_page *)virt_to_page(block);
> > > > - if (slob_page(sp))
> > > > - return (((slob_t *)block - 1)->units - 1) * SLOB_UNIT;
> > > > - else
> > > > + if (slob_page(sp)) {
> > > > + int align = max(ARCH_KMALLOC_MINALIGN,
> > > > ARCH_SLAB_MINALIGN); + unsigned int *m = (unsigned int
> > > > *)(block - align); + return *m + align;
> > > > + } else
> > > > return sp->page.private;
> > > > }
> > > >
> > > > However, mine is lifted directly from kfree, wheras you do something a
> > > > bit different. Hmm, ksize arguably could be used to find the underlying
> > > > allocated slab size in order to use a little bit more than we'd asked
> > > > for. So probably we should really just `return *m` (don't round up or
> > > > add any padding).
> > >
> > > Huh? ksize should report how much space is available in the buffer. If
> > > we request 33 bytes from SLUB and it gives us 64, ksize reports 64. If
> > > we request 33 bytes from SLOB and it gives us 34, we should report 34.
> >
> > Oh.. hmm yeah right, I didn't realise what you were doing there.
> > OK, so your patch looks good to me then (provided it is diffed against
> > the previous one, for Linus).
>
> OK, no, that's why I got confused. SLOB_UNITS will round you up to the
> next SLOB_UNIT. You'd then have to multiply by SLOB_UNIT to get back to
> bytes.

Damnit, how many ways can we get confused by these little details? I'll
spin a final version and run it against the test harness shortly.

--
Mathematics is the supreme nostalgia of our time.



\
 
 \ /
  Last update: 2008-10-08 07:21    [W:0.069 / U:1.216 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site