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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ingo,

On 16/12/06, Ingo Molnar <[email protected]> wrote:
FYI, i'm working on integrating kmemleak into -rt. Firstly, i needed the
fixes below when applying it ontop of 2.6.19-rt15.

Do you need these fixes to avoid a compiler error? If yes, this is
caused by a bug in gcc-4.x. The kmemleak container_of macro has
protection for non-constant offsets passed to container_of but the
faulty gcc always returns true for builtin_contant_p, even when this
is not the case. Previous versions (3.4) or one of the latest 4.x gcc
don't have this bug.

I wouldn't extend kmemleak to work around a gcc bug which was already fixed.

Secondly, i'm wondering about the following recursion:

 [<c045a7e1>] rt_spin_lock_slowlock+0x98/0x1dc
 [<c045b16b>] rt_spin_lock+0x13/0x4b
 [<c018f155>] kfree+0x3a/0xce
 [<c0192e79>] hash_delete+0x58/0x5f
 [<c019309b>] memleak_free+0xe9/0x1e6
 [<c018ed2e>] __cache_free+0x27/0x414
 [<c018f1d0>] kfree+0xb5/0xce
 [<c02788dd>] acpi_ns_get_node+0xb1/0xbb
 [<c02772fa>] acpi_ns_root_initialize+0x30f/0x31d
 [<c0280194>] acpi_initialize_subsystem+0x58/0x87
 [<c06a4641>] acpi_early_init+0x4f/0x12e
 [<c06888bc>] start_kernel+0x41b/0x44b

kfree() within kfree() ... this probably works on the upstream SLAB
allocator but makes it pretty nasty to sort out SLAB locking in -rt.

I test kmemleak with lockdep enabled but I eliminated all the
dependencies on the vanilla kernel. When kfree(hnode) is called (in
hash_delete), no kmemleak locks are held and hence no dependency on
the kmemleak locks (since kmemleak is protected against re-entrance).
My understanding is that slab __cache_free is re-entrant anyway
(noticed this when using radix-tree instead of hash in kmemleak and
got some lockdep reports on l3->list_lock and memleak_lock) and
calling it again from kmemleak doesn't seem to have any problem on the
vanilla kernel.

In the -rt kernel, is there any protection against a re-entrant
__cache_free (via cache_flusharray -> free_block -> slab_destroy) or
this is not needed?

Wouldnt it be better to just preallocate the hash nodes, like lockdep
does, to avoid conceptual nesting? Basically debugging infrastructure
should rely on other infrastructure as little as possible.

It would indeed be better to avoid using the slab infrastructure (and
not worry about kmemleak re-entrance and lock dependecies). I'll have
a look on how this is done in lockdep since the preallocation size
isn't known. There are also the memleak_object structures that need to
be allocated/freed. To avoid any locking dependencies, I ended up
delaying the memleak_object structures freeing in an RCU manner. It
might work if I do the same with the hash nodes.

also, the number of knobs in the Kconfig is quite large:

I had some reasons and couldn't find a unified solution, but probably
only for one or two if them:

 CONFIG_DEBUG_MEMLEAK=y
 CONFIG_DEBUG_MEMLEAK_HASH_BITS=16

For my limited configurations (an x86 laptop and several ARM embedded
platforms), 16 bits were enough. I'm not sure this is enough on a
server machine for example.

 CONFIG_DEBUG_MEMLEAK_TRACE_LENGTH=4

I thought this is a user preference. I could hard-code it to 16.
What's the trace length used by lockdep?

 CONFIG_DEBUG_MEMLEAK_PREINIT_OBJECTS=512

I can probably hard-code this as well. This is a buffer to temporary
store memory allocations before kmemleak is fully initialised.
Kmemleak gets initialised quite early in the start_kernel function and
shouldn't be that different in other kernel configurations.

 CONFIG_DEBUG_MEMLEAK_SECONDARY_ALIASES=y
 CONFIG_DEBUG_MEMLEAK_TASK_STACKS=y

These could be eliminated as well.

There are also:

CONFIG_DEBUG_MEMLEAK_REPORT_THLD - can be removed
CONFIG_DEBUG_MEMLEAK_REPORTS_NR - can be removed
CONFIG_DEBUG_KEEP_INIT - this might be useful for other tools that
store the backtrace and display it at a later time. Could be made more
generic.

plus the Kconfig dependency on SLAB_DEBUG makes it less likely for
people to spontaneously try kmemleak. I'd suggest to offer KMEMLEAK
unconditionally (when KERNEL_DEBUG is specified) and simply select
SLAB_DEBUG.

I just followed the DEBUG_SLAB_LEAK configuration but I don't have any
problem with making it more visible.

Thanks for your comments.

--
Catalin
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux