lkml.org 
[lkml]   [2006]   [Dec]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2.6.20-rc1 00/10] Kernel memory leak detector 0.13
    Hi Ingo,

    On 16/12/06, Ingo Molnar <mingo@elte.hu> 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 majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-12-17 00:41    [W:0.029 / U:0.576 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site