Re: [PATCH 2.6.17-rc6 7/9] Remove some of the kmemleak false positives

[Jeremy Fitzhardinge <jeremy@goop.org>]

Ingo Molnar wrote:
> For a GC a false negative is no big problem - it will reduce the 
> efficiency of the GC a bit, but that's all. For leak detection, if we 
> happen to have a persistent false pointer in .data (or any other 
> persistently allocated memory), it may prevent the detection of a leak 
> permanently - at least for that bootup. Statistically it could still be 
> found on other systems, but it would be better to have a design that 
> will eventually lead to having no false negatives.
>
> But it's not just about the amount of false negatives, but also about 
> the overhead of scanning. You are concentrated on embedded systems with 
> small RAM - but most of the testers will be running this with at last 
> 1GB of RAM - which is _alot_ of memory to scan.
>   
It seems to me that most types with any pointers are fairly 
pointer-dense.  There's not much point in trying to skip a couple of 
non-pointers nested among a dozen others; once you've worn the cost of 
pulling in the cache line there's not much else to worry about.  The 
most useful thing is to distinguish between completely pointerless 
allocations and allocations which have pointers.  Pointerless 
allocations are generally just data (strings, numbers, user data), and 
so are a waste of effort to scan, and possibly full of false pointers.  
In the kernel, you could probably do it by making it a property of 
slabs, assume all kmalloc allocations are pointerful (perhaps add 
GFP_POINTERLESS), and make sure all user-data pages are considered 
pointerless.

False pointers in kernel allocations can be avoided in a few ways.  The 
first, obviously, is the make sure all memory is initialized to a known 
non-pointer value.  The second is to ignore pointers which don't point 
near the start of an allocated region (possibly unsafe in the kernel, 
depending on the definition of "near").  You can get more sophisticated 
from there; the Boehm GC keeps tracks of things which look like pointers 
but turn out not to be (they don't point to allocated memory); it marks 
that memory as being unusable, so that the false pointer won't get 
mistaken for one later on, with the obvious risk that lots of false 
pointers can render large parts of your heap address space unusable.

In general, false pointers aren't a huge problem.  They'll generally 
lead to a bounded number of allocations being unreported as leaks; its 
highly unlikely that a large heap graph will remain hidden from a leak 
checker forever; espectially since kernel pointers are fairly unlike 
other kinds of data (large enough to not be aliased to most normal 
integer values, don't look like strings, and there are no FP numbers in 
the kernel).

> (But, if it's not possible to implement it in a sane manner then that's 
> not an issue either - it's rather the false positives that must be 
> avoided.)
>   

There's some risk of false positives.  You can imagine cases where the 
last reference to a block is transformed into a bus address, and in 
effect a piece of hardware holds it.  You don't get to know about the 
pointer until the hardware gives it back.  You might want a GFP_ROOT 
flag (or whatever), to mark a block as being always referenced in order 
to suppress these cases.

> there are a couple of possibilities.
>
> If the ID is string based then you dont even have to touch containr_of() 
> calls - just generate the typename string via the "#y" stringification 
> preprocessor directive, where 'y' is the second parameter of 
> container_of().
> [...]
> it needs some thought, but this way it's quite possible to build-time 
> map types to IDs.
>   

This seems pretty over-engineered.  I wouldn't go this far unless you're 
actually seeing performance/correctness problems, and a simple 
with/without pointers flag isn't enough.  It also doesn't address the 
most troublesome source of false pointers: stacks.  There is all sorts 
of junk lying around on stacks, and you can have an old dead pointer 
sitting there pinning old dead memory for a long time.

    J