2.6.16-rc5-mm2 - kzalloc() considered harmful for debugging.

2.6.16-rc5-mm2 - kzalloc() considered harmful for debugging.

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1038 bytes --]

So I've got a (probably self-inflicted) memory leak in slab-64 and slab-32.
Rebuild the kernel with CONFIG_DEBUG_SLAB, reboot, and wait for a bit of
leak to pile up, and then echo 'slab-32 0 0 0' > /proc/slabinfo

And ta-DA! the top offender is... (drum roll): <kzalloc+0xe/0x36>

Blargh.  It's tempting to do something like this in include/linux/slab.h:

#ifdef CONFIG_SLAB_DEBUG
static inline void* kzalloc(size_t size, gfp_t flags)
{
        void *ret = kmalloc(size, flags);
        if (ret)
                memset(ret, 0, size);
        return ret;
}
#else
extern void *kzalloc(size_t, gfp_t);
#end

or maybe some ad-crock macro implementation, just so the actual calling site of
kmalloc is recorded, rather than losing the caller of kzalloc.

Only problems are that (a) changing CONFIG_DEBUG_SLAB will probably recompile
the world rather than just mm/slab.c, and (b) it's 7AM and I've been chasing this
for 6 hours and not sure how to handle the actual body in mm/util.c (wrap the
kzalloc() with a #ifndev CONFIG_DEBUG_SLAB maybe)?

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: 2.6.16-rc5-mm2 - kzalloc() considered harmful for debugging.

[Pekka Enberg]

Hi Valdis,

On 12/18/05, Valdis.Kletnieks@vt.edu <Valdis.Kletnieks@vt.edu> wrote:
> Blargh.  It's tempting to do something like this in include/linux/slab.h:
>
> #ifdef CONFIG_SLAB_DEBUG
> static inline void* kzalloc(size_t size, gfp_t flags)
> {
>         void *ret = kmalloc(size, flags);
>         if (ret)
>                 memset(ret, 0, size);
>         return ret;
> }
> #else
> extern void *kzalloc(size_t, gfp_t);
> #end

I don't see CONFIG_SLAB_DEBUG in 2.6.15-rc5-mm2. Is it an external patch?

> or maybe some ad-crock macro implementation, just so the actual calling site of
> kmalloc is recorded, rather than losing the caller of kzalloc.

I would prefer this. Both kzalloc and kstrdup should override the call
site of kmalloc. Preferably, all of them should use something like
__kmalloc_tracked() and pass the call site as an parameter.

                          Pekka

Re: 2.6.16-rc5-mm2 - kzalloc() considered harmful for debugging.

[Andrew Morton]

Valdis.Kletnieks@vt.edu wrote:
>
> So I've got a (probably self-inflicted) memory leak in slab-64 and slab-32.
> Rebuild the kernel with CONFIG_DEBUG_SLAB, reboot, and wait for a bit of
> leak to pile up, and then echo 'slab-32 0 0 0' > /proc/slabinfo
> 
> And ta-DA! the top offender is... (drum roll): <kzalloc+0xe/0x36>
> 
> Blargh.  It's tempting to do something like this in include/linux/slab.h:
> 
> #ifdef CONFIG_SLAB_DEBUG
> static inline void* kzalloc(size_t size, gfp_t flags)
> {
>         void *ret = kmalloc(size, flags);
>         if (ret)
>                 memset(ret, 0, size);
>         return ret;
> }
> #else
> extern void *kzalloc(size_t, gfp_t);
> #end

That would work.

Or we could special-case kzalloc() and kstrdup() in slab.c - use
builtin_return_address(1) if builtin_return_address(0) is within those
functions.  Dunno if that's worth the fuss though.

Re: 2.6.16-rc5-mm2 - kzalloc() considered harmful for debugging.

[Keith Owens]

On Mon, 19 Dec 2005 02:20:59 -0800, 
Andrew Morton <akpm@osdl.org> wrote:
>Or we could special-case kzalloc() and kstrdup() in slab.c - use
>builtin_return_address(1) if builtin_return_address(0) is within those
>functions.  Dunno if that's worth the fuss though.

builtin_return_address(1) on i386 only works with stack frame pointers.
Without stack frame pointers, builtin_return_address(1) gets garbage.
builtin_return_address(0) is the only value that is guaranteed to work
with and without stack frame pointers.

Even with frame pointers, tail recursion confuses builtin_return_address(1)
on i386.