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

Pekka Enberg

author Reviewed-by

2008-02-29 07:21 UTC #2 lore raw

Quoted 4 lines · Christoph Lameter

No one has used that option for a long time and AFAICT its currently utterly useless.
Signed-off-by Christoph Lameter <[email protected]>

Reviewed-by Pekka Enberg <[email protected]>

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

Hugh Dickins

2008-02-29 14:01 UTC #3 lore raw

In reply to a message not in the archive

Quoted 2 lines · Christoph Lameter

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

2008-02-29 19:48 UTC #4 lore raw

Quoted 5 lines · Hugh Dickins

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.

Quoted 5 lines

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..

Quoted 4 lines

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Pekka Enberg

author Reviewed-by

2008-02-29 07:29 UTC #6 lore raw

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

Quoted 3 lines · Christoph Lameter

Group SLUB_DEBUG code together to reduce the number of #ifdefs.
Signed-off-by Christoph Lameter <[email protected]>

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 <[email protected]>

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

Christoph Lameter

2008-02-29 19:31 UTC #7 lore raw

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

Quoted 7 lines · Pekka Enberg

Quoted 3 lines · Christoph Lameter
Group SLUB_DEBUG code together to reduce the number of #ifdefs.
Signed-off-by Christoph Lameter <[email protected]>
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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Pekka Enberg

author Reviewed-by

2008-02-29 07:37 UTC #9 lore raw

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

Quoted 4 lines · Christoph Lameter

No need to access the kmem_cache structure. We have the same value in kmem_cache_cpu.
Signed-off-by Christoph Lameter <[email protected]>

Reviewed-by Pekka Enberg <[email protected]>

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

Pekka Enberg

author

2008-02-29 07:45 UTC #11 lore raw

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

Quoted 11 lines · Christoph Lameter

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 <[email protected]>
(quoted diff) +1 -0
+#ifdef CONFIG_SLUB_DEBUG
 	/*
 	 * Debugging requires use of the padding between object
 	 * and whatever may come after it.

Review size_t ksize(const void *object) · line 2635

Quoted 5 lines

(quoted diff) +1 -1

@@ -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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

2008-02-29 19:34 UTC #12 lore raw

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

Quoted 1 line · Pekka Enberg

Why are you wrapping the SLAB_DESTORY_BY_RCU case with CONFIG_SLUB_DEBUG too?

Mistake on my part. Corrected patch follows:

Pekka J Enberg

author Reviewed-by

2008-03-01 09:59 UTC #13 lore raw

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

Quoted 1 line · Pekka Enberg

Quoted 1 line
Why are you wrapping the SLAB_DESTORY_BY_RCU case with CONFIG_SLUB_DEBUG too?

Quoted 7 lines · Christoph Lameter

Mistake on my part. Corrected patch follows:
From 74d3c465f217f3103e7a6d21cb090497386de8be Mon Sep 17 00:00:00 2001
From: Christoph Lameter <[email protected]>
Date: Fri, 15 Feb 2008 23:45:25 -0800
Subject: [PATCH] slub: Remove BUG_ON() from ksize and omit checks for !SLUB_DEBUG

Quoted 5 lines

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 <[email protected]>

Reviewed-by Pekka Enberg <[email protected]>

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

Pekka Enberg

author Reviewed-by

2008-02-29 07:46 UTC #15 lore raw

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

Quoted 3 lines · Christoph Lameter

Slub is missing some NUMA support for large kmallocs. Provide that.
Signed-off-by Christoph Lameter <[email protected]>

Reviewed-by Pekka Enberg <[email protected]>

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

Pekka Enberg

author Reviewed-by

2008-02-29 07:54 UTC #17 lore raw

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

Quoted 4 lines · Christoph Lameter

Alloc debug processing is never called with a NULL object pointer. No reason to check for NULL.
Signed-off-by Christoph Lameter <[email protected]>

Reviewed-by Pekka Enberg <[email protected]>

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

Pekka Enberg

author Reviewed-by

2008-02-29 08:00 UTC #19 lore raw

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

Quoted 6 lines · Christoph Lameter

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 <[email protected]>

Reviewed-by Pekka Enberg <[email protected]>

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

Pekka Enberg

author Reviewed-by

2008-02-29 08:05 UTC #21 lore raw

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

Quoted 7 lines · Christoph Lameter

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 <[email protected]>

Reviewed-by Pekka Enberg <[email protected]>

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