Re: [patch 09/10] slub: Rearrange #ifdef CONFIG_SLUB_DEBUG in calculate_sizes()

Re: [patch 09/10] slub: Rearrange #ifdef CONFIG_SLUB_DEBUG in calculate_sizes()

[Pekka Enberg]

Christoph Lameter wrote:
> Group SLUB_DEBUG code together to reduce the number of #ifdefs.
> 
> Signed-off-by: Christoph Lameter <clameter@sgi.com>

This doesn't just rearrange #ifdefs, it moves the poisoning checks under 
#ifdef too (which is safe). You might want to mention that in the 
changelogs.

Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 09/10] slub: Rearrange #ifdef CONFIG_SLUB_DEBUG in calculate_sizes()

[Christoph Lameter]

On Fri, 29 Feb 2008, Pekka Enberg wrote:

> Christoph Lameter wrote:
> > Group SLUB_DEBUG code together to reduce the number of #ifdefs.
> > 
> > Signed-off-by: Christoph Lameter <clameter@sgi.com>
> 
> This doesn't just rearrange #ifdefs, it moves the poisoning checks under
> #ifdef too (which is safe). You might want to mention that in the changelogs.

Ok. Will do.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 06/10] slub: Use the objsize from the kmem_cache_cpu structure

[Pekka Enberg]

Christoph Lameter wrote:
> No need to access the kmem_cache structure. We have the same value
> in kmem_cache_cpu.
> 
> Signed-off-by: Christoph Lameter <clameter@sgi.com>

Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 08/10] slub: Remove BUG_ON() from ksize and omit checks for !SLUB_DEBUG

[Pekka Enberg]

Christoph Lameter wrote:
> The BUG_ONs are useless since the pointer derefs will lead to
> NULL deref errors anyways. Some of the checks are not necessary
> if no debugging is possible.
> 
> Signed-off-by: Christoph Lameter <clameter@sgi.com>

> +#ifdef CONFIG_SLUB_DEBUG
>  	/*
>  	 * Debugging requires use of the padding between object
>  	 * and whatever may come after it.
> @@ -2634,7 +2632,7 @@ size_t ksize(const void *object)
>  	 */
>  	if (s->flags & (SLAB_DESTROY_BY_RCU | SLAB_STORE_USER))
>  		return s->inuse;
> -
> +#endif

Why are you wrapping the SLAB_DESTORY_BY_RCU case with CONFIG_SLUB_DEBUG 
too?

			Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 08/10] slub: Remove BUG_ON() from ksize and omit checks for !SLUB_DEBUG

[Christoph Lameter]

On Fri, 29 Feb 2008, Pekka Enberg wrote:

> Why are you wrapping the SLAB_DESTORY_BY_RCU case with CONFIG_SLUB_DEBUG too?

Mistake on my part. Corrected patch follows:

Re: [patch 08/10] slub: Remove BUG_ON() from ksize and omit checks for !SLUB_DEBUG

[Pekka J Enberg]

On Fri, 29 Feb 2008, Pekka Enberg wrote:
> > Why are you wrapping the SLAB_DESTORY_BY_RCU case with CONFIG_SLUB_DEBUG too?

On Fri, 29 Feb 2008, Christoph Lameter wrote:
> Mistake on my part. Corrected patch follows:
> 
> From 74d3c465f217f3103e7a6d21cb090497386de8be Mon Sep 17 00:00:00 2001
> From: Christoph Lameter <clameter@sgi.com>
> Date: Fri, 15 Feb 2008 23:45:25 -0800
> Subject: [PATCH] slub: Remove BUG_ON() from ksize and omit checks for !SLUB_DEBUG
> 
> The BUG_ONs are useless since the pointer derefs will lead to
> NULL deref errors anyways. Some of the checks are not necessary
> if no debugging is possible.
> 
> Signed-off-by: Christoph Lameter <clameter@sgi.com>

Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 07/10] slub: Add kmalloc_large_node() to support kmalloc_node fallback

[Pekka Enberg]

Christoph Lameter wrote:
> Slub is missing some NUMA support for large kmallocs. Provide that.
> 
> Signed-off-by: Christoph Lameter <clameter@sgi.com>

Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 04/10] slub: Remove useless checks in alloc_debug_processing

[Pekka Enberg]

Christoph Lameter wrote:
> Alloc debug processing is never called with a NULL object pointer.
> No reason to check for NULL.
> 
> Signed-off-by: Christoph Lameter <clameter@sgi.com>

Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 03/10] slub: Remove objsize check in kmem_cache_flags()

[Pekka Enberg]

Christoph Lameter wrote:
> There is no page->offset anymore and also no associated limit on the number
> of objects. The page->offset field was removed for 2.6.24. So the check
> in kmem_cache_flags() is now also obsolete (should have been dropped
> earlier, somehow a hunk vanished).
> 
> Signed-by: Christoph Lameter <clameter@sgi.com>

Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 01/10] Revert "unique end pointer" patch

[Pekka Enberg]

Christoph Lameter wrote:
> This only made sense for the alternate fastpath which was reverted last week.
> 
> Mathieu is working on a new version that addresses the fastpath issues but that
> new code first needs to go through mm and it is not clear if we need the
> unique end pointers with his new scheme.
> 
> Signed-off-by: Christoph Lameter <clameter@sgi.com>

Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 05/10] slub: Remove slub_nomerge

[Pekka Enberg]

Christoph Lameter wrote:
> No one has used that option for a long time and AFAICT its currently utterly
> useless.
> 
> Signed-off-by: Christoph Lameter <clameter@sgi.com>

Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 05/10] slub: Remove slub_nomerge

[Hugh Dickins]

On Thu, 28 Feb 2008, Christoph Lameter wrote:
> No one has used that option for a long time and AFAICT its currently utterly
> useless.

Not so at all.  I certainly use it: not often, but from time to time,
in debugging.  If you're narrowing down on something, it's a worthwhile
tool to separate the different uses of a particular size.

And when studying slabinfo numbers, perhaps for a leak e.g. why doesn't
slabinfo doesn't show vm_area_struct, oh, it's sharing :0000088 with
cfq_queue, so we need slub_nomerge to see their actual numbers.
Perhaps I'm missing something: how does everyone else get the
right numbers without slub_nomerge?

Admittedly it's often too blunt an instrument for debugging: I'd be
happier with a debug flag which has no other side-effect than nomerge
(all the other SLUB_NEVER_MERGE flags seemed to have side-effects that
I wanted to avoid when trying to reproduce an elusive corruption),
that can be applied to a single cache as well as to the whole lot.

I could add that if you don't (or I could hack my mm/slub.c when
I need to, that's always an option: but I do think nomerge can be
useful out in the field).  If you go ahead and remove slub_nomerge,
please also remove it from Documentation/kernel-parameters.txt.

Hugh

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 05/10] slub: Remove slub_nomerge

[Christoph Lameter]

On Fri, 29 Feb 2008, Hugh Dickins wrote:

> And when studying slabinfo numbers, perhaps for a leak e.g. why doesn't
> slabinfo doesn't show vm_area_struct, oh, it's sharing :0000088 with
> cfq_queue, so we need slub_nomerge to see their actual numbers.
> Perhaps I'm missing something: how does everyone else get the
> right numbers without slub_nomerge?

I typically enable debugging in those cases.

> Admittedly it's often too blunt an instrument for debugging: I'd be
> happier with a debug flag which has no other side-effect than nomerge
> (all the other SLUB_NEVER_MERGE flags seemed to have side-effects that
> I wanted to avoid when trying to reproduce an elusive corruption),
> that can be applied to a single cache as well as to the whole lot.

Ohh..

> I could add that if you don't (or I could hack my mm/slub.c when
> I need to, that's always an option: but I do think nomerge can be
> useful out in the field).  If you go ahead and remove slub_nomerge,
> please also remove it from Documentation/kernel-parameters.txt.

Well then lets keep it. Patch dropped.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>