[PATCH] replace kmem_cache_alloc with kmem_cache_zalloc to remove some following zero initializations.

[PATCH] replace kmem_cache_alloc with kmem_cache_zalloc to remove some following zero initializations.

[Denis Cheng]

>From 4d87e14b67890f06885a76b5792ca034de2e9d06 Mon Sep 17 00:00:00 2001
From: Denis Cheng <crquan@gmail.com>
Date: Thu, 12 Jul 2007 11:53:58 +0800
Subject: [PATCH] replace kmem_cache_alloc with kmem_cache_zalloc to
remove some following zero initializations.

Signed-off-by: Denis Cheng <crquan@gmail.com>
---
 fs/dcache.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 0e73aa0..8c559b2 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -898,7 +898,7 @@ struct dentry *d_alloc(struct dentry * parent, const
struct qstr *name)
 	struct dentry *dentry;
 	char *dname;
 
-	dentry = kmem_cache_alloc(dentry_cache, GFP_KERNEL); 
+	dentry = kmem_cache_zalloc(dentry_cache, GFP_KERNEL); 
 	if (!dentry)
 		return NULL;
 
@@ -921,15 +921,7 @@ struct dentry *d_alloc(struct dentry * parent,
const struct qstr *name)
 	atomic_set(&dentry->d_count, 1);
 	dentry->d_flags = DCACHE_UNHASHED;
 	spin_lock_init(&dentry->d_lock);
-	dentry->d_inode = NULL;
-	dentry->d_parent = NULL;
-	dentry->d_sb = NULL;
-	dentry->d_op = NULL;
-	dentry->d_fsdata = NULL;
-	dentry->d_mounted = 0;
-#ifdef CONFIG_PROFILING
-	dentry->d_cookie = NULL;
-#endif
+
 	INIT_HLIST_NODE(&dentry->d_hash);
 	INIT_LIST_HEAD(&dentry->d_lru);
 	INIT_LIST_HEAD(&dentry->d_subdirs);
-- 
1.5.2.2

Re: [PATCH] replace kmem_cache_alloc with kmem_cache_zalloc to remove some following zero initializations.

[Kirill Korotaev]

This doesn't look worth zeroing half of the struct
when it is initialized to non-zeros then.

Denis Cheng wrote:
>>From 4d87e14b67890f06885a76b5792ca034de2e9d06 Mon Sep 17 00:00:00 2001
> From: Denis Cheng <crquan@gmail.com>
> Date: Thu, 12 Jul 2007 11:53:58 +0800
> Subject: [PATCH] replace kmem_cache_alloc with kmem_cache_zalloc to
> remove some following zero initializations.
> 
> Signed-off-by: Denis Cheng <crquan@gmail.com>
> ---
>  fs/dcache.c |   12 ++----------
>  1 files changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 0e73aa0..8c559b2 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -898,7 +898,7 @@ struct dentry *d_alloc(struct dentry * parent, const
> struct qstr *name)
>  	struct dentry *dentry;
>  	char *dname;
>  
> -	dentry = kmem_cache_alloc(dentry_cache, GFP_KERNEL); 
> +	dentry = kmem_cache_zalloc(dentry_cache, GFP_KERNEL); 
>  	if (!dentry)
>  		return NULL;
>  
> @@ -921,15 +921,7 @@ struct dentry *d_alloc(struct dentry * parent,
> const struct qstr *name)
>  	atomic_set(&dentry->d_count, 1);
>  	dentry->d_flags = DCACHE_UNHASHED;
>  	spin_lock_init(&dentry->d_lock);
> -	dentry->d_inode = NULL;
> -	dentry->d_parent = NULL;
> -	dentry->d_sb = NULL;
> -	dentry->d_op = NULL;
> -	dentry->d_fsdata = NULL;
> -	dentry->d_mounted = 0;
> -#ifdef CONFIG_PROFILING
> -	dentry->d_cookie = NULL;
> -#endif
> +
>  	INIT_HLIST_NODE(&dentry->d_hash);
>  	INIT_LIST_HEAD(&dentry->d_lru);
>  	INIT_LIST_HEAD(&dentry->d_subdirs);

Re: [PATCH] replace kmem_cache_alloc with kmem_cache_zalloc to remove some following zero initializations.

[rae l]

On 7/13/07, Kirill Korotaev <dev@sw.ru> wrote:
> This doesn't look worth zeroing half of the struct
> when it is initialized to non-zeros then.
But why?

My reason to think it's better and faster is that:
1. the code will be shorter if it calls zalloc and then removes the
NULL and zero initilization;
2. in the assembly code objdumped, many mov operations reduced, such as:
    movl $0,0x40(%ebp)
    ...
    this style of zero initialization occupies 7 bytes per line
(i386), and then multiply 7 lines,

3. the only change is that calls to kmem_cache_zalloc other than
kmem_cache_alloc, it's just an extra memset is called, as we all know
the memset implimentation is string operation, that's rather fast.

>
> Denis Cheng wrote:
> >>From 4d87e14b67890f06885a76b5792ca034de2e9d06 Mon Sep 17 00:00:00 2001
> > From: Denis Cheng <crquan@gmail.com>
> > Date: Thu, 12 Jul 2007 11:53:58 +0800
> > Subject: [PATCH] replace kmem_cache_alloc with kmem_cache_zalloc to
> > remove some following zero initializations.
> >
> > Signed-off-by: Denis Cheng <crquan@gmail.com>
> > ---
> >  fs/dcache.c |   12 ++----------
> >  1 files changed, 2 insertions(+), 10 deletions(-)
> >
> > diff --git a/fs/dcache.c b/fs/dcache.c
> > index 0e73aa0..8c559b2 100644
> > --- a/fs/dcache.c
> > +++ b/fs/dcache.c
> > @@ -898,7 +898,7 @@ struct dentry *d_alloc(struct dentry * parent, const
> > struct qstr *name)
> >       struct dentry *dentry;
> >       char *dname;
> >
> > -     dentry = kmem_cache_alloc(dentry_cache, GFP_KERNEL);
> > +     dentry = kmem_cache_zalloc(dentry_cache, GFP_KERNEL);
> >       if (!dentry)
> >               return NULL;
> >
> > @@ -921,15 +921,7 @@ struct dentry *d_alloc(struct dentry * parent,
> > const struct qstr *name)
> >       atomic_set(&dentry->d_count, 1);
> >       dentry->d_flags = DCACHE_UNHASHED;
> >       spin_lock_init(&dentry->d_lock);
> > -     dentry->d_inode = NULL;
> > -     dentry->d_parent = NULL;
> > -     dentry->d_sb = NULL;
> > -     dentry->d_op = NULL;
> > -     dentry->d_fsdata = NULL;
> > -     dentry->d_mounted = 0;
> > -#ifdef CONFIG_PROFILING
> > -     dentry->d_cookie = NULL;
> > -#endif
> > +
> >       INIT_HLIST_NODE(&dentry->d_hash);
> >       INIT_LIST_HEAD(&dentry->d_lru);
> >       INIT_LIST_HEAD(&dentry->d_subdirs);
>
>


-- 
Denis Cheng
Linux Application Developer

"One of my most productive days was throwing away 1000 lines of code."
 - Ken Thompson.

Re: [PATCH] replace kmem_cache_alloc with kmem_cache_zalloc to remove some following zero initializations.

[Kirill Korotaev]

Look, until you have any numbers in hands it's impossible to say
which one is faster.

Please measure N d_alloc()'s on i686 and some other archs w/o string operations
and compare whether your patch improves something or not.

Kirill

rae l wrote:
> On 7/13/07, Kirill Korotaev <dev@sw.ru> wrote:
> 
>>This doesn't look worth zeroing half of the struct
>>when it is initialized to non-zeros then.
> 
> But why?
> 
> My reason to think it's better and faster is that:
> 1. the code will be shorter if it calls zalloc and then removes the
> NULL and zero initilization;
> 2. in the assembly code objdumped, many mov operations reduced, such as:
>     movl $0,0x40(%ebp)
>     ...
>     this style of zero initialization occupies 7 bytes per line
> (i386), and then multiply 7 lines,
> 
> 3. the only change is that calls to kmem_cache_zalloc other than
> kmem_cache_alloc, it's just an extra memset is called, as we all know
> the memset implimentation is string operation, that's rather fast.
> 
> 
>>Denis Cheng wrote:
>>
>>>>From 4d87e14b67890f06885a76b5792ca034de2e9d06 Mon Sep 17 00:00:00 2001
>>>From: Denis Cheng <crquan@gmail.com>
>>>Date: Thu, 12 Jul 2007 11:53:58 +0800
>>>Subject: [PATCH] replace kmem_cache_alloc with kmem_cache_zalloc to
>>>remove some following zero initializations.
>>>
>>>Signed-off-by: Denis Cheng <crquan@gmail.com>
>>>---
>>> fs/dcache.c |   12 ++----------
>>> 1 files changed, 2 insertions(+), 10 deletions(-)
>>>
>>>diff --git a/fs/dcache.c b/fs/dcache.c
>>>index 0e73aa0..8c559b2 100644
>>>--- a/fs/dcache.c
>>>+++ b/fs/dcache.c
>>>@@ -898,7 +898,7 @@ struct dentry *d_alloc(struct dentry * parent, const
>>>struct qstr *name)
>>>      struct dentry *dentry;
>>>      char *dname;
>>>
>>>-     dentry = kmem_cache_alloc(dentry_cache, GFP_KERNEL);
>>>+     dentry = kmem_cache_zalloc(dentry_cache, GFP_KERNEL);
>>>      if (!dentry)
>>>              return NULL;
>>>
>>>@@ -921,15 +921,7 @@ struct dentry *d_alloc(struct dentry * parent,
>>>const struct qstr *name)
>>>      atomic_set(&dentry->d_count, 1);
>>>      dentry->d_flags = DCACHE_UNHASHED;
>>>      spin_lock_init(&dentry->d_lock);
>>>-     dentry->d_inode = NULL;
>>>-     dentry->d_parent = NULL;
>>>-     dentry->d_sb = NULL;
>>>-     dentry->d_op = NULL;
>>>-     dentry->d_fsdata = NULL;
>>>-     dentry->d_mounted = 0;
>>>-#ifdef CONFIG_PROFILING
>>>-     dentry->d_cookie = NULL;
>>>-#endif
>>>+
>>>      INIT_HLIST_NODE(&dentry->d_hash);
>>>      INIT_LIST_HEAD(&dentry->d_lru);
>>>      INIT_LIST_HEAD(&dentry->d_subdirs);
>>
>>
> 
> 

Re: [PATCH] replace kmem_cache_alloc with kmem_cache_zalloc to remove some following zero initializations.

[Pekka Enberg]

Hi Kirill,

On 7/13/07, Kirill Korotaev <dev@sw.ru> wrote:
> Look, until you have any numbers in hands it's impossible to say
> which one is faster.

Well, it's not too hard to guess: struct dentry is 124 bytes on i386
so kmem_cache_zalloc over it is bound to be slower tha
kmem_cache_alloc as most members are initialized to non-NULL. And if
you're into micro-benchmarks, here's one:

d_alloc takes 182.75ns
d_alloc_zalloc takes 315.89ns

So definitely NAK for Denis' patch.

                                               Pekka

#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/time.h>
#include <string.h>
#include <time.h>

struct dentry {
	void *a;
	void *b;
	void *c;
	void *d;
	void *e;
	void *f;
	void *g;
	void *h;
	unsigned char d_iname[36];
};

#define NR_ITERS 1000000

#define MAX_DENTRIES (NR_ITERS*2)

static unsigned long offset;
static struct dentry dentries[MAX_DENTRIES];

static inline void *__kmalloc(void)
{
	void *p = &dentries[offset++];
	if (offset == MAX_DENTRIES)
		offset = 0;
	return p;
}

void *kmalloc(size_t sz)
{
	return __kmalloc();
}

struct dentry *d_alloc(const char *name)
{
	struct dentry *ret = kmalloc(sizeof *ret);
	if (ret) {
		ret->a = NULL;
		ret->b = NULL;
		ret->c = NULL;
		ret->d = NULL;
		ret->e = (void *) 0xdeadbeef;
		ret->f = (void *) 0xdeadbeef;
		ret->g = (void *) 0xdeadbeef;
		ret->h = (void *) 0xdeadbeef;
		strcpy(ret->d_iname, name);
	}
	return ret;
}

static inline void * __memset_generic(void * s, char c,size_t count)
{
	int d0, d1;
	__asm__ __volatile__(
	        "rep\n\t"
	        "stosb"
	        : "=&c" (d0), "=&D" (d1)
	        :"a" (c),"1" (s),"0" (count)
	        :"memory");
	return s;
}

void *kzalloc(size_t sz)
{
	void *p = __kmalloc();
	if (p)
		__memset_generic(p, 0, sz);
	return p;
}

struct dentry *d_alloc_zalloc(const char *name)
{
	struct dentry *ret = kzalloc(sizeof *ret);
	if (ret) {
		ret->e = (void *) 0xdeadbeef;
		ret->f = (void *) 0xdeadbeef;
		ret->g = (void *) 0xdeadbeef;
		ret->h = (void *) 0xdeadbeef;
		strcpy(ret->d_iname, name);
	}
	return ret;
}

int main(int argc, char *argv[])
{
	struct timeval start, end;
	unsigned long long usec;
	int i;

	gettimeofday(&start, NULL);
	for (i = 0; i < NR_ITERS; i++) {
		struct dentry *dentry = d_alloc("root");

		if (!dentry || dentry->a != NULL)
			abort();
	}
	gettimeofday(&end, NULL);
	usec = end.tv_usec + 1000000*(end.tv_sec - start.tv_sec) - start.tv_usec;
	printf("d_alloc takes %0.2lfns\n", (double)usec * 1000 / NR_ITERS);

	gettimeofday(&start, NULL);
	for (i = 0; i < NR_ITERS; i++) {
		struct dentry *dentry = d_alloc_zalloc("root");

		if (!dentry || dentry->a != NULL)
			abort();
	}
	gettimeofday(&end, NULL);
	usec = end.tv_usec + 1000000*(end.tv_sec - start.tv_sec) - start.tv_usec;
	printf("d_alloc_zalloc takes %0.2lfns\n", (double)usec * 1000 / NR_ITERS);

	return 0;
}