lkml.org 
[lkml]   [2005]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] remove redundant NULL pointer checks prior to calling kfree() in fs/nfsd/
Date
On Saturday 26 March 2005 10:34, Arjan van de Ven wrote:
> On Fri, 2005-03-25 at 17:34 -0500, linux-os wrote:
> > On Fri, 25 Mar 2005, Jesper Juhl wrote:
> >
> > > (please keep me on CC)
> > >
> > >
> > > checking for NULL before calling kfree() is redundant and needlessly
> > > enlarges the kernel image, let's get rid of those checks.
> > >
> >
> > Hardly. ORing a value with itself and jumping on condition is
> > real cheap compared with pushing a value into the stack
>
> which century are you from?
> "jumping on condition" can easily be 100+ cycles, depending on how
> effective the branch predictor is. Pushing a value onto the stack otoh
> is half a cycle.

linux-os is right because kfree does NULL check with exactly
the same code sequence, test and branch:

# objdump -d mm/slab.o
...
000012ef <kfree>:
12ef: 55 push %ebp
12f0: 89 e5 mov %esp,%ebp
12f2: 57 push %edi
12f3: 56 push %esi
12f4: 53 push %ebx
12f5: 51 push %ecx
12f6: 8b 7d 08 mov 0x8(%ebp),%edi
12f9: 85 ff test %edi,%edi
12fb: 74 46 je 1343 <kfree+0x54>
...
...
...
1343: 8d 65 f4 lea 0xfffffff4(%ebp),%esp
1346: 5b pop %ebx
1347: 5e pop %esi
1348: 5f pop %edi
1349: 5d pop %ebp
134a: c3 ret

So kfree(p) indeed will spend time for doing a call,
for test-and-branch, *and* finally for ret,
while if(p) kfree(p) will do test-and-branch first
and won't do call/ret if p==NULL.

However, if p is not NULL, if(p) kfree(p) does:
1) test-and-branch (not taken)
2) call
3) another test-and-branch (not taken)!

I conclude that if(p) kfree(p) makes sense only if:
a) p is more often NULL than not, and
b) it's in the hot path (you don't want to save on code size)

Since (a) is not typical, I think Jesper's cleanups are ok.
--
vda

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-04-06 13:30    [W:0.037 / U:5.676 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site