Fix kmem_cache_free performance regression in slab

PATCH In review Matthew Wilcox 3 participants 3 messages Nov 29, 2007 – Nov 30, 2007 Memory management

Share & tools

Matthew Wilcox

author

2007-11-29 19:05 UTC #0 lore raw

PATCH 1/1 Fix kmem_cache_free performance regression in slab

The database performance group have found that half the cycles spent in kmem_cache_free are spent in this one call to BUG_ON. Moving it into the CONFIG_SLAB_DEBUG-only function cache_free_debugcheck() is a performance win of almost 0.5% on their particular benchmark.

The call was added as part of commit ddc2e812d592457747c4367fb73edcaa8e1e49ff with the comment that "overhead should be minimal". It may have been minimal at the time, but it isn't now.

Signed-off-by Matthew Wilcox <[email protected]>

mm/slab.c +2 -2

index cfa6be4..6e16431 100644
@@ -2881,6 +2881,8 @@ static void *cache_free_debugcheck(struct kmem_cache *cachep, void *objp,
 	unsigned int objnr;
 	struct slab *slabp;
 
+	BUG_ON(virt_to_cache(objp) != cachep);
+
 	objp -= obj_offset(cachep);
 	kfree_debugcheck(objp);
 	page = virt_to_head_page(objp);

@@ -3759,8 +3761,6 @@ void kmem_cache_free(struct kmem_cache *cachep, void *objp)
 {
 	unsigned long flags;
 
-	BUG_ON(virt_to_cache(objp) != cachep);
-
 	local_irq_save(flags);
 	debug_check_no_locks_freed(objp, obj_size(cachep));
 	__cache_free(cachep, objp);

Signature · 10 lines

Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

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

Andrew Morton

maintainer

2007-11-30 02:45 UTC #1 lore raw

Quoted 9 lines · On Thu, 29 Nov 2007 12:05:13 -0700 Matthew Wilcox <[email protected]> wrote:

The database performance group have found that half the cycles spent in kmem_cache_free are spent in this one call to BUG_ON. Moving it into the CONFIG_SLAB_DEBUG-only function cache_free_debugcheck() is a performance win of almost 0.5% on their particular benchmark.
The call was added as part of commit ddc2e812d592457747c4367fb73edcaa8e1e49ff with the comment that "overhead should be minimal". It may have been minimal at the time, but it isn't now.

It is worth noting that the offending commit hit mainline in June 2006.

It takes a very long time for some performance regressions to be discovered. By which time it is effectively too late to fix it.

--
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 J Enberg

2007-11-30 09:14 UTC #2 lore raw

Hi,

Quoted 9 lines · On Thu, 29 Nov 2007 12:05:13 -0700 Matthew Wilcox <[email protected]> wrote:

Quoted 9 lines
The database performance group have found that half the cycles spent in kmem_cache_free are spent in this one call to BUG_ON. Moving it into the CONFIG_SLAB_DEBUG-only function cache_free_debugcheck() is a performance win of almost 0.5% on their particular benchmark.
The call was added as part of commit ddc2e812d592457747c4367fb73edcaa8e1e49ff with the comment that "overhead should be minimal". It may have been minimal at the time, but it isn't now.

Quoted 4 lines · Andrew Morton

It is worth noting that the offending commit hit mainline in June 2006.
It takes a very long time for some performance regressions to be discovered. By which time it is effectively too late to fix it.

What architecture is this? x86_64? I don't think the BUG_ON per se caused the performance regression but rather the virt_to_head_page() changes to virt_to_cache() that were added later. But reverting the BUG_ON is fine by me.

Thanks Matthew and others for tracking this down!

Pekka