Re: [PATCH 2.6.20-rc1 00/10] Kernel memory leak detector 0.13

[Ingo Molnar <mingo@elte.hu>]

* Catalin Marinas <catalin.marinas@gmail.com> wrote:

> > hm, even on vanilla you might run into problems in slab_destroy(), 
> > there we hold the l3 lock.
> 
> It seems that slab_destroy doesn't take the l3 lock again if it is 
> already held, otherwise it would fail without kmemleak. However, I 
> can't guarantee that even a minor change wouldn't break kmemleak.

slab_destroy() is careful to not take the /same/ lock. But kmemleak does 
an unconditional kfree() and i dont see what saves us from a rare but 
possible deadlock here.

> > yeah, delayed RCU freeing might work better.
> 
> I could also use a simple allocator based on alloc_pages since 
> kmemleak doesn't track pages. [...]

actually, i'm quite sure we want to track pages later on too, any reason 
why kmemleak shouldnt cover them?

But i agree that using a separate allocator would be the right choice 
here. Once the page allocator will be added to the tracking mechanism we 
can exclude those allocations by doing wrappers. I.e. you shouldnt just 
add the memleak_*() functions to the page allocator directly - wrap the 
existing calls cleanly, and thus preserve a non-tracked API variant. For 
the SLAB this is hard because the SLAB has so many internal dependencies 
- but the buddy is a 'core' allocator.

and those non-wrapped APIs would also be useful for the SLAB to call 
into - that way we could exclude the tracking of SLAB allocations. 
(unless someone wants to track/debug the SLAB implementation itself.)

> [...] It could be so simple that it would never need to free any 
> pages, just grow the size as required and reuse the freed memleak 
> objects from a list.

sounds good to me. Please make it a per-CPU pool. We'll have to fix the 
locking too, to be per-CPU - memleak_lock is quite a scalability problem 
right now. (Add a memleak_object->cpu pointer so that freeing can be 
done on any other CPU as well.)

> This would simplify the recursiveness and also work on any other slab 
> allocator (looking back at the amount of time I spend to sort out the 
> recursiveness and locking dependencies, I could've implemented a full 
> allocator).

yes.

	Ingo