All of lore.kernel.org
 help / color / mirror / Atom feed
From: Catalin Marinas <catalin.marinas@arm.com>
To: Pekka Enberg <penberg@cs.helsinki.fi>
Cc: linux-kernel@vger.kernel.org, Matt Mackall <mpm@selenic.com>,
	Christoph Lameter <cl@linux-foundation.org>
Subject: Re: [PATCH 2.6.28-rc5 03/11] kmemleak: Add the memory allocation/freeing hooks
Date: Mon, 24 Nov 2008 10:18:45 +0000	[thread overview]
Message-ID: <1227521925.8377.37.camel@pc1117.cambridge.arm.com> (raw)
In-Reply-To: <1227514753.3718.21.camel@penberg-laptop>

Pekka,

On Mon, 2008-11-24 at 10:19 +0200, Pekka Enberg wrote:
> On Fri, 2008-11-21 at 11:07 +0000, Catalin Marinas wrote:
> > On Thu, 2008-11-20 at 21:30 +0200, Pekka Enberg wrote:
> > > > @@ -2610,6 +2611,9 @@ static struct slab *alloc_slabmgmt(struct kmem_cache *cachep, void *objp,
> > > >                /* Slab management obj is off-slab. */
> > > >                slabp = kmem_cache_alloc_node(cachep->slabp_cache,
> > > >                                              local_flags & ~GFP_THISNODE, nodeid);
> > > > +               /* only scan the list member to avoid false negatives */
> > > > +               memleak_scan_area(slabp, offsetof(struct slab, list),
> > > > +                                 sizeof(struct list_head));
> > > 
> > > I find this comment somewhat confusing. Does it mean we _must_ scan
> > > the list members to avoid false negatives (i.e. leaks that happened
> > > but were not reported) or that if we scan the whole of struct slab, we
> > > get false negatives?
> > 
> > It's been some time since I first added this and I may not remember the
> > full details but it's the latter case - it should avoid scanning
> > slabp->s_mem because (my understanding) is that it may contain a pointer
> > to an allocated block. Kmemleak only allows adding what sections to
> > scan, so in this case only the list_head is relevant.
> > 
> > Let me know if my understanding is correct and I'll make the comment
> > more clear.
> 
> Well, slab->s_mem simply points to the slab (i.e. page) itself. So I
> suppose we need to ignore ->s_mem to avoid scanning the same slab twice?

Kmemleak never scans a block twice (well, during a scanning session), it
just increases a reference count for the objects referred via the
scanned object. Since slab structures are allocated via
kmem_cache_alloc_noe, they'll be tracked by kmemleak and scanned
(otherwise they would be memory leaks).

My understanding is that the ->s_mem value points to the slab itself but
the same pointer might actually be the beginning of an allocated memory
block, hence we get at least one reference to this block.

-- 
Catalin


  reply	other threads:[~2008-11-24 10:19 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-20 11:30 [PATCH 2.6.28-rc5 00/11] Kernel memory leak detector (updated) Catalin Marinas
2008-11-20 11:30 ` [PATCH 2.6.28-rc5 01/11] kmemleak: Add the base support Catalin Marinas
2008-11-20 11:58   ` Ingo Molnar
2008-11-20 19:35   ` Pekka Enberg
2008-11-21 12:07     ` Catalin Marinas
2008-11-24  8:16       ` Pekka Enberg
2008-11-24  8:19         ` Pekka Enberg
2008-12-03 18:12   ` Paul E. McKenney
2008-12-04 12:14     ` Catalin Marinas
2008-12-04 16:55       ` Paul E. McKenney
2008-12-06 23:07         ` Catalin Marinas
2008-12-07 23:19           ` Paul E. McKenney
2008-11-20 11:30 ` [PATCH 2.6.28-rc5 02/11] kmemleak: Add documentation on the memory leak detector Catalin Marinas
2008-11-20 11:30 ` [PATCH 2.6.28-rc5 03/11] kmemleak: Add the memory allocation/freeing hooks Catalin Marinas
2008-11-20 12:00   ` Ingo Molnar
2008-11-20 19:30   ` Pekka Enberg
2008-11-21 11:07     ` Catalin Marinas
2008-11-24  8:19       ` Pekka Enberg
2008-11-24 10:18         ` Catalin Marinas [this message]
2008-11-24 10:35           ` Pekka Enberg
2008-11-24 10:43             ` Catalin Marinas
2008-11-20 11:30 ` [PATCH 2.6.28-rc5 04/11] kmemleak: Add modules support Catalin Marinas
2008-11-20 12:03   ` Ingo Molnar
2008-11-20 11:30 ` [PATCH 2.6.28-rc5 05/11] kmemleak: Add support for i386 Catalin Marinas
2008-11-20 12:16   ` Ingo Molnar
2008-11-20 11:31 ` [PATCH 2.6.28-rc5 06/11] kmemleak: Add support for ARM Catalin Marinas
2008-11-20 11:31 ` [PATCH 2.6.28-rc5 07/11] kmemleak: Remove some of the kmemleak false positives Catalin Marinas
2008-11-20 12:09   ` Ingo Molnar
2008-11-20 11:31 ` [PATCH 2.6.28-rc5 08/11] kmemleak: Enable the building of the memory leak detector Catalin Marinas
2008-11-20 11:31 ` [PATCH 2.6.28-rc5 09/11] kmemleak: Keep the __init functions after initialization Catalin Marinas
2008-11-20 11:31 ` [PATCH 2.6.28-rc5 10/11] kmemleak: Simple testing module for kmemleak Catalin Marinas
2008-11-20 12:11   ` Ingo Molnar
2008-11-20 11:31 ` [PATCH 2.6.28-rc5 11/11] kmemleak: Add the corresponding MAINTAINERS entry Catalin Marinas
2008-11-20 12:10 ` [PATCH 2.6.28-rc5 00/11] Kernel memory leak detector (updated) Ingo Molnar
2008-11-20 17:54   ` Catalin Marinas
2008-11-20 12:22 ` Ingo Molnar
2008-11-20 18:10   ` Catalin Marinas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1227521925.8377.37.camel@pc1117.cambridge.arm.com \
    --to=catalin.marinas@arm.com \
    --cc=cl@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpm@selenic.com \
    --cc=penberg@cs.helsinki.fi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.