Re: kfree(0) - ok?

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Satyam Sharma <satyam@...>
Cc: Arjan van de Ven <arjan@...>, Tim Bird <tim.bird@...>, linux kernel <linux-kernel@...>, Christoph Lameter <clameter@...>
Date: Friday, August 17, 2007 - 2:22 pm

On Wed, 15 Aug 2007 05:12:41 +0530 (IST)
Satyam Sharma <satyam@infradead.org> wrote:


I went through my current versions of slab/slub/slub and came up with this:

diff -puN mm/slob.c~slub-slob-use-unlikely-for-kfreezero_or_null_ptr-check mm/slob.c
--- a/mm/slob.c~slub-slob-use-unlikely-for-kfreezero_or_null_ptr-check
+++ a/mm/slob.c
@@ -360,7 +360,7 @@ static void slob_free(void *block, int s
 	slobidx_t units;
 	unsigned long flags;
 
-	if (ZERO_OR_NULL_PTR(block))
+	if (unlikely(ZERO_OR_NULL_PTR(block)))
 		return;
 	BUG_ON(!size);
 
@@ -466,7 +466,7 @@ void kfree(const void *block)
 {
 	struct slob_page *sp;
 
-	if (ZERO_OR_NULL_PTR(block))
+	if (unlikely(ZERO_OR_NULL_PTR(block)))
 		return;
 
 	sp = (struct slob_page *)virt_to_page(block);
@@ -484,7 +484,7 @@ size_t ksize(const void *block)
 {
 	struct slob_page *sp;
 
-	if (ZERO_OR_NULL_PTR(block))
+	if (unlikely(ZERO_OR_NULL_PTR(block)))
 		return 0;
 
 	sp = (struct slob_page *)virt_to_page(block);
diff -puN mm/slub.c~slub-slob-use-unlikely-for-kfreezero_or_null_ptr-check mm/slub.c
--- a/mm/slub.c~slub-slob-use-unlikely-for-kfreezero_or_null_ptr-check
+++ a/mm/slub.c
@@ -2434,7 +2434,7 @@ size_t ksize(const void *object)
 	struct page *page;
 	struct kmem_cache *s;
 
-	if (ZERO_OR_NULL_PTR(object))
+	if (unlikely(ZERO_OR_NULL_PTR(object)))
 		return 0;
 
 	page = get_object_page(object);
@@ -2468,7 +2468,7 @@ void kfree(const void *x)
 {
 	struct page *page;
 
-	if (ZERO_OR_NULL_PTR(x))
+	if (unlikely(ZERO_OR_NULL_PTR(x)))
 		return;
 
 	page = virt_to_head_page(x);
@@ -2785,7 +2785,7 @@ void *__kmalloc_track_caller(size_t size
 							get_order(size));
 	s = get_slab(size, gfpflags);
 
-	if (ZERO_OR_NULL_PTR(s))
+	if (unlikely(ZERO_OR_NULL_PTR(s)))
 		return s;
 
 	return slab_alloc(s, gfpflags, -1, caller);
@@ -2801,7 +2801,7 @@ void *__kmalloc_node_track_caller(size_t
 							get_order(size));
 	s = get_slab(size, gfpflags);
 
-	if (ZERO_OR_NULL_PTR(s))
+	if (unlikely(ZERO_OR_NULL_PTR(s)))
 		return s;
 
 	return slab_alloc(s, gfpflags, node, caller);
_

Which is getting pretty idiotic:

akpm:/usr/src/25> grep ZERO_OR_NULL_PTR */*.c
mm/slab.c:              BUG_ON(ZERO_OR_NULL_PTR(cachep->slabp_cache));
mm/slab.c:      if (unlikely(ZERO_OR_NULL_PTR(cachep)))
mm/slab.c:      if (unlikely(ZERO_OR_NULL_PTR(cachep)))
mm/slab.c:      if (unlikely(ZERO_OR_NULL_PTR(objp)))
mm/slab.c:      if (unlikely(ZERO_OR_NULL_PTR(objp)))
mm/slob.c:      if (unlikely(ZERO_OR_NULL_PTR(block)))
mm/slob.c:      if (unlikely(ZERO_OR_NULL_PTR(block)))
mm/slob.c:      if (unlikely(ZERO_OR_NULL_PTR(block)))
mm/slub.c:      if (unlikely(ZERO_OR_NULL_PTR(s)))
mm/slub.c:      if (unlikely(ZERO_OR_NULL_PTR(s)))
mm/slub.c:      if (unlikely(ZERO_OR_NULL_PTR(object)))
mm/slub.c:      if (unlikely(ZERO_OR_NULL_PTR(x)))
mm/slub.c:      if (unlikely(ZERO_OR_NULL_PTR(s)))
mm/slub.c:      if (unlikely(ZERO_OR_NULL_PTR(s)))

are we seeing a pattern here?  We could stick the unlikely inside
ZERO_OR_NULL_PTR() itself.  That's a little bit sleazy though - there might
be future callsites at which it is likely, who knows?

I guess we can stick with the idiotic patch ;)
-
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
kfree(0) - ok?, Tim Bird, (Tue Aug 14, 6:59 pm)
Re: kfree(0) - ok?, Satyam Sharma, (Tue Aug 14, 7:13 pm)
Re: kfree(0) - ok?, Arjan van de Ven, (Tue Aug 14, 6:55 pm)
Re: kfree(0) - ok?, Satyam Sharma, (Tue Aug 14, 7:42 pm)
Re: kfree(0) - ok?, Andrew Morton, (Fri Aug 17, 2:22 pm)
Re: kfree(0) - ok?, Satyam Sharma, (Fri Aug 17, 5:13 pm)
Re: kfree(0) - ok?, Christoph Lameter, (Fri Aug 17, 5:14 pm)
Re: kfree(0) - ok?, Satyam Sharma, (Fri Aug 17, 5:46 pm)
Re: kfree(0) - ok?, Pekka Enberg, (Fri Aug 17, 5:42 pm)
Re: kfree(0) - ok?, Thomas Gleixner, (Fri Aug 17, 7:40 pm)
Re: kfree(0) - ok?, Jan Engelhardt, (Sat Aug 18, 4:21 am)
Re: kfree(0) - ok?, Pekka Enberg, (Sat Aug 18, 4:10 am)
Re: kfree(0) - ok?, Christoph Lameter, (Fri Aug 17, 9:03 pm)
Re: kfree(0) - ok?, Satyam Sharma, (Fri Aug 17, 8:02 pm)
Re: kfree(0) - ok?, Christoph Lameter, (Fri Aug 17, 7:22 pm)
Re: kfree(0) - ok?, Christoph Lameter, (Fri Aug 17, 4:43 pm)
Re: kfree(0) - ok?, Satyam Sharma, (Fri Aug 17, 5:17 pm)
Re: kfree(0) - ok?, Satyam Sharma, (Fri Aug 17, 5:32 pm)
Re: kfree(0) - ok?, Jan Engelhardt, (Fri Aug 17, 2:37 pm)
Re: kfree(0) - ok?, Arjan van de Ven, (Fri Aug 17, 2:31 pm)
Re: kfree(0) - ok?, Satyam Sharma, (Fri Aug 17, 2:50 pm)
Re: kfree(0) - ok?, Christoph Lameter, (Tue Aug 14, 8:19 pm)
Re: kfree(0) - ok?, Jason Uhlenkott, (Tue Aug 14, 7:21 pm)
Re: kfree(0) - ok?, Jan Engelhardt, (Wed Aug 15, 3:28 am)
Re: kfree(0) - ok?, Giacomo A. Catenazzi, (Wed Aug 15, 5:32 am)
Re: kfree(0) - ok?, Andreas Schwab, (Wed Aug 15, 5:18 am)
Re: kfree(0) - ok?, Jason Uhlenkott, (Wed Aug 15, 4:52 am)
Re: kfree(0) - ok?, Rene Herman, (Wed Aug 15, 4:37 am)
Re: kfree(0) - ok?, Jan Engelhardt, (Wed Aug 15, 5:20 am)
Re: kfree(0) - ok?, Rene Herman, (Wed Aug 15, 5:58 am)
Re: kfree(0) - ok?, Jan Engelhardt, (Wed Aug 15, 6:20 am)
Re: kfree(0) - ok?, H. Peter Anvin, (Wed Aug 15, 12:01 pm)
Re: kfree(0) - ok?, Kyle Moffett, (Wed Aug 15, 9:58 am)
Re: kfree(0) - ok?, Jan Engelhardt, (Wed Aug 15, 10:06 am)
Re: kfree(0) - ok?, Kyle Moffett, (Wed Aug 15, 10:34 am)
Re: kfree(0) - ok?, Rene Herman, (Wed Aug 15, 6:27 am)
Re: kfree(0) - ok?, Jason Uhlenkott, (Wed Aug 15, 5:43 am)