With the slab allocator, if off-slab management is enabled for the
kmem_caches used by kmemleak, it leads to recursive calls into
kmemleak_alloc(). Off-slab management can be triggered by other config
options increasing the slab size, e.g. DEBUG_PAGEALLOC.
Reported-by: Tetsuo Handa <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: Christoph Lameter <[email protected]>
Signed-off-by: Catalin Marinas <[email protected]>
---
mm/slab.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/mm/slab.c b/mm/slab.c
index 7dfa481..646db30 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2261,9 +2261,11 @@ kmem_cache_create (const char *name, size_t size, size_t align,
/*
* Determine if the slab management is 'on' or 'off' slab.
* (bootstrapping cannot cope with offslab caches so don't do
- * it too early on.)
+ * it too early on. Always use on-slab management when
+ * SLAB_NOLEAKTRACE to avoid recursive calls into kmemleak)
*/
- if ((size >= (PAGE_SIZE >> 3)) && !slab_early_init)
+ if ((size >= (PAGE_SIZE >> 3)) && !slab_early_init &&
+ !(flags & SLAB_NOLEAKTRACE))
/*
* Size is large, assume best to place the slab management obj
* off-slab (should allow better packing of objs).
Hi Catalin,
Catalin Marinas wrote:
> With the slab allocator, if off-slab management is enabled for the
> kmem_caches used by kmemleak, it leads to recursive calls into
> kmemleak_alloc(). Off-slab management can be triggered by other config
> options increasing the slab size, e.g. DEBUG_PAGEALLOC.
>
> Reported-by: Tetsuo Handa <[email protected]>
> Cc: Pekka Enberg <[email protected]>
> Cc: Christoph Lameter <[email protected]>
> Signed-off-by: Catalin Marinas <[email protected]>
Forcing slabs to use on-slab management is pretty bad from memory
consumption point of view. Wouldn't it be better to annotate the
recursive calls somehow?
Pekka
> ---
> mm/slab.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/mm/slab.c b/mm/slab.c
> index 7dfa481..646db30 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -2261,9 +2261,11 @@ kmem_cache_create (const char *name, size_t size, size_t align,
> /*
> * Determine if the slab management is 'on' or 'off' slab.
> * (bootstrapping cannot cope with offslab caches so don't do
> - * it too early on.)
> + * it too early on. Always use on-slab management when
> + * SLAB_NOLEAKTRACE to avoid recursive calls into kmemleak)
> */
> - if ((size >= (PAGE_SIZE >> 3)) && !slab_early_init)
> + if ((size >= (PAGE_SIZE >> 3)) && !slab_early_init &&
> + !(flags & SLAB_NOLEAKTRACE))
> /*
> * Size is large, assume best to place the slab management obj
> * off-slab (should allow better packing of objs).
>
Hi Pekka,
On Thu, 2009-10-15 at 21:59 +0300, Pekka Enberg wrote:
> Catalin Marinas wrote:
> > With the slab allocator, if off-slab management is enabled for the
> > kmem_caches used by kmemleak, it leads to recursive calls into
> > kmemleak_alloc(). Off-slab management can be triggered by other config
> > options increasing the slab size, e.g. DEBUG_PAGEALLOC.
> >
> > Reported-by: Tetsuo Handa <[email protected]>
> > Cc: Pekka Enberg <[email protected]>
> > Cc: Christoph Lameter <[email protected]>
> > Signed-off-by: Catalin Marinas <[email protected]>
>
> Forcing slabs to use on-slab management is pretty bad from memory
> consumption point of view. Wouldn't it be better to annotate the
> recursive calls somehow?
The are annotated using SLAB_NOLEAKTRACE but off-slab management uses a
general cachep and we cannot use this flag on them as we end up ignoring
kmalloc allocations. An alternative is to use GFP_ flag rather than a
SLAB_ one to track the recursive calls.
I could also rework the way hooks were added to slab.c but currently
they follow the same places as kmemcheck_slab_alloc and
lockdep_trace_alloc.
Anyway, the kmemleak caches only need off-slab management when
DEBUG_PAGEALLOC is enabled but in this case the memory consumption is
high anyway.
--
Catalin
On Fri, Oct 16, 2009 at 3:08 PM, Catalin Marinas
<[email protected]> wrote:
> On Thu, 2009-10-15 at 21:59 +0300, Pekka Enberg wrote:
>> Catalin Marinas wrote:
>> > With the slab allocator, if off-slab management is enabled for the
>> > kmem_caches used by kmemleak, it leads to recursive calls into
>> > kmemleak_alloc(). Off-slab management can be triggered by other config
>> > options increasing the slab size, e.g. DEBUG_PAGEALLOC.
>> >
>> > Reported-by: Tetsuo Handa <[email protected]>
>> > Cc: Pekka Enberg <[email protected]>
>> > Cc: Christoph Lameter <[email protected]>
>> > Signed-off-by: Catalin Marinas <[email protected]>
>>
>> Forcing slabs to use on-slab management is pretty bad from memory
>> consumption point of view. Wouldn't it be better to annotate the
>> recursive calls somehow?
>
> The are annotated using SLAB_NOLEAKTRACE but off-slab management uses a
> general cachep and we cannot use this flag on them as we end up ignoring
> kmalloc allocations. An alternative is to use GFP_ flag rather than a
> SLAB_ one to track the recursive calls.
>
> I could also rework the way hooks were added to slab.c but currently
> they follow the same places as kmemcheck_slab_alloc and
> lockdep_trace_alloc.
>
> Anyway, the kmemleak caches only need off-slab management when
> DEBUG_PAGEALLOC is enabled but in this case the memory consumption is
> high anyway.
Oh, right, I misread the patch, sorry. Please feel free to put the
patch in kmemleak.git:
Reviewed-by: Pekka Enberg <[email protected]>