lkml.org 
[lkml]   [2005]   [Mar]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] no need to check for NULL before calling kfree() -fs/ext2/
On Sat, 26 Mar 2005, linux-os wrote:

> On Sat, 26 Mar 2005, Arjan van de Ven wrote:
>
> > On Fri, 2005-03-25 at 17:29 -0500, linux-os wrote:
> > > Isn't it expensive of CPU time to call kfree() even though the
> > > pointer may have already been freed?
> >
> > nope
> >
> > a call instruction is effectively half a cycle or less, the branch
>
> Wrong!
>
> > predictor of the cpu can predict perfectly where the next instruction is
> > from. The extra if() you do in front is a different matter, that can
> > easily cost 100 cycles+. (And those are redundant cycles because kfree
> > will do the if again anyway). So what you propose is to spend 100+
> > cycles to save half a cycle. Not a good tradeoff ;)
> >
>
> Wrong!
>
[snip]
>
> Always, always, a call will be more expensive than a branch
> on condition. It's impossible to be otherwise. A call requires
> that the return address be written to memory (the stack),
> using register indirection (the stack-pointer).
>
> If somebody said; "I think that the code will look better
> and the few cycles lost will not be a consequence with modern
> CPUs...", then there is a point. But coming up with this
> disingenuous bullshit is something else.
>

I tried to create a test to see what the actual impact of this sort of
change is, the result I reached is below (as well as the code used to
obtain the numbers):


Each test is run 10000000 times, and the number of jiffies spent doing the
kfree();
or
if (p)
kfree(p);
is meassured. Total number of jiffies used for that for all 10000000 runs
is reported.

test 0:
Pointer is NULL half the time, value returned by kmalloc half the
time.
kfree() is called on the pointer without checking for NULL first.

test 1:
Pointer is NULL half the time, value returned by kmalloc half the
time.
The pointer is checked for NULL and kfree() is called on the
pointer only if it is != NULL.

test 2:
Pointer is NULL the majority of the time, only in 1 out of 50
cases is it assigned a real value by kmalloc().
kfree() is called on the pointer without checking for NULL first.

test 3:
Pointer is NULL the majority of the time, only in 1 out of 50
cases is it assigned a real value by kmalloc().
The pointer is checked for NULL and kfree() is called on the
pointer only if it is != NULL.

test 4:
Pointer is rarely NULL - only in 1 out of 50 cases.
kfree() is called on the pointer without checking for NULL first.

test 5:
Pointer is rarely NULL - only in 1 out of 50 cases.
The pointer is checked for NULL and kfree() is called on the
pointer only if it is != NULL.


Here are the numbers from 5 runs on my box - the numbers naturally
differ a bit between each run, but they are quite similar each time :

[ 1395.059375] test 0 used up 235 kfree related jiffies
[ 1395.059385] test 1 used up 195 kfree related jiffies
[ 1395.059389] test 2 used up 66 kfree related jiffies
[ 1395.059392] test 3 used up 20 kfree related jiffies
[ 1395.059395] test 4 used up 366 kfree related jiffies
[ 1395.059398] test 5 used up 428 kfree related jiffies

[ 1412.994705] test 0 used up 231 kfree related jiffies
[ 1412.994744] test 1 used up 209 kfree related jiffies
[ 1412.994748] test 2 used up 68 kfree related jiffies
[ 1412.994751] test 3 used up 12 kfree related jiffies
[ 1412.994754] test 4 used up 362 kfree related jiffies
[ 1412.994757] test 5 used up 392 kfree related jiffies

[ 1423.734356] test 0 used up 245 kfree related jiffies
[ 1423.734366] test 1 used up 179 kfree related jiffies
[ 1423.734370] test 2 used up 78 kfree related jiffies
[ 1423.734373] test 3 used up 30 kfree related jiffies
[ 1423.734376] test 4 used up 384 kfree related jiffies
[ 1423.734379] test 5 used up 385 kfree related jiffies

[ 1434.390194] test 0 used up 242 kfree related jiffies
[ 1434.390203] test 1 used up 179 kfree related jiffies
[ 1434.390207] test 2 used up 70 kfree related jiffies
[ 1434.390210] test 3 used up 16 kfree related jiffies
[ 1434.390214] test 4 used up 365 kfree related jiffies
[ 1434.390217] test 5 used up 397 kfree related jiffies

[ 1446.529856] test 0 used up 231 kfree related jiffies
[ 1446.530046] test 1 used up 232 kfree related jiffies
[ 1446.530117] test 2 used up 79 kfree related jiffies
[ 1446.530211] test 3 used up 16 kfree related jiffies
[ 1446.530278] test 4 used up 360 kfree related jiffies
[ 1446.530362] test 5 used up 412 kfree related jiffies

The conclusions I draw from those numbers are that when NULL pointers are
rare (tests 4 & 5) then it pays off to not have the if() check. When NULL
pointers are common, then there's a small bennefit to having the if()
check, but we are talking ~50 jiffies (or less) over 10 million runs pr
test, which is pretty insignificant unless the code is in a very hot path.
When pointers are NULL 50% of the time there's a bennefit to the if(), but
it's small.
So, unless the code is extremely performance critical *and* the pointer
is NULL more often than not, having the if(pointer != NULL) check before
calling kfree() is pointless and may even be degrading performance if the
pointer is most commonly != NULL. I'd say that the general rule should
be "don't check for NULL first unless you *know* the pointer will be NULL
>50% of the time"...
I ran these tests on a 1.4GHz AMD Athlon (T-bird), and with a HZ setting
of 1000.

Am I drawing flawed conclusions here?

If someone could check the sanity of my code used to obtain these numbers
(below), then I'd appreciate it - if the numbers are wrong, then any
conclusion is also wrong of course.


Here's the tiny module I wrote to get the numbers above :


#include <linux/init.h>
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/slab.h>

#define NR_TESTS 10000000

void do_work(void *data);

DECLARE_WORK(work, do_work, NULL);

static int test_time[] = {0, 0, 0, 0, 0, 0};

void do_work(void *data)
{
unsigned long j;
static int what_test = 0;
unsigned long start;
void *tmp;

switch (what_test) {
case 0:
for (j = 0; j < NR_TESTS; j++) {
if (j%2 == 0)
tmp = kmalloc(1, GFP_KERNEL);
else
tmp = NULL;
start = jiffies;
kfree(tmp);
test_time[0] += jiffies - start;
}
break;
case 1:
for (j = 0; j < NR_TESTS; j++) {
if (j%2 == 0)
tmp = kmalloc(1, GFP_KERNEL);
else
tmp = NULL;
start = jiffies;
if (tmp)
kfree(tmp);
test_time[1] += jiffies - start;
}
break;
case 2:
for (j = 0; j < NR_TESTS; j++) {
if (j%50 == 0)
tmp = kmalloc(1, GFP_KERNEL);
else
tmp = NULL;
start = jiffies;
kfree(tmp);
test_time[2] += jiffies - start;
}
break;
case 3:
for (j = 0; j < NR_TESTS; j++) {
if (j%50 == 0)
tmp = kmalloc(1, GFP_KERNEL);
else
tmp = NULL;
start = jiffies;
if (tmp)
kfree(tmp);
test_time[3] += jiffies - start;
}
break;
case 4:
for (j = 0; j < NR_TESTS; j++) {
if (j%50 == 0)
tmp = NULL;
else
tmp = kmalloc(1, GFP_KERNEL);
start = jiffies;
kfree(tmp);
test_time[4] += jiffies - start;
}
break;
case 5:
for (j = 0; j < NR_TESTS; j++) {
if (j%50 == 0)
tmp = NULL;
else
tmp = kmalloc(1, GFP_KERNEL);
start = jiffies;
if (tmp)
kfree(tmp);
test_time[5] += jiffies - start;
}
break;
default:
break;
}
printk(KERN_ALERT "test %d done.\n", what_test);

if (what_test < 5)
schedule_delayed_work(&work, 1);
else
printk(KERN_ALERT "All tests done...\n");

what_test++;
}


static int kfreetest_init(void)
{
schedule_work(&work);
return 0;
}

static void kfreetest_exit(void)
{
int i;

cancel_delayed_work(&work);
flush_scheduled_work();
for (i = 0; i < 6; i++)
printk(KERN_ALERT "test %d used up %d kfree related jiffies\n", i, test_time[i]);
}


module_init(kfreetest_init);
module_exit(kfreetest_exit);

MODULE_LICENSE("GPL");
MODULE_AUTHOR("Jesper Juhl");



--
Jesper Juhl <juhl-lkml@dif.dk>


-
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.118 / U:0.120 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site