[PATCH 1/4] ext2: remove inode constructor

[PATCH 1/4] ext2: remove inode constructor

[Pekka J Enberg]

From: Pekka Enberg <penberg@cs.helsinki.fi>

As alloc_inode() touches the same cache line as init_once(), we gain
nothing from using slab constructors.

Cc: Stephen C. Tweedie <sct@redhat.com>
Cc: Andreas Dilger <adilger@clusterfs.com>
Cc: Christoph Lameter <clameter@sgi.com>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---
 fs/ext2/super.c |   32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

Index: 26-mm/fs/ext2/super.c
===================================================================
--- 26-mm.orig/fs/ext2/super.c	2007-05-05 12:26:15.000000000 +0300
+++ 26-mm/fs/ext2/super.c	2007-05-05 12:30:50.000000000 +0300
@@ -140,16 +140,24 @@ static struct kmem_cache * ext2_inode_ca
 static struct inode *ext2_alloc_inode(struct super_block *sb)
 {
 	struct ext2_inode_info *ei;
-	ei = (struct ext2_inode_info *)kmem_cache_alloc(ext2_inode_cachep,
-						GFP_KERNEL|__GFP_RECLAIMABLE);
+	struct inode *inode;
+
+	ei = kmem_cache_alloc(ext2_inode_cachep, GFP_KERNEL|__GFP_RECLAIMABLE);
 	if (!ei)
 		return NULL;
+	rwlock_init(&ei->i_meta_lock);
+#ifdef CONFIG_EXT2_FS_XATTR
+	init_rwsem(&ei->xattr_sem);
+#endif
+	mutex_init(&ei->truncate_mutex);
 #ifdef CONFIG_EXT2_FS_POSIX_ACL
 	ei->i_acl = EXT2_ACL_NOT_CACHED;
 	ei->i_default_acl = EXT2_ACL_NOT_CACHED;
 #endif
-	ei->vfs_inode.i_version = 1;
-	return &ei->vfs_inode;
+	inode = &ei->vfs_inode;
+	inode_init_once(inode);
+	inode->i_version = 1;
+	return inode;
 }
 
 static void ext2_destroy_inode(struct inode *inode)
@@ -157,27 +165,13 @@ static void ext2_destroy_inode(struct in
 	kmem_cache_free(ext2_inode_cachep, EXT2_I(inode));
 }
 
-static void init_once(void * foo, struct kmem_cache * cachep, unsigned long flags)
-{
-	struct ext2_inode_info *ei = (struct ext2_inode_info *) foo;
-
-	if (flags & SLAB_CTOR_CONSTRUCTOR) {
-		rwlock_init(&ei->i_meta_lock);
-#ifdef CONFIG_EXT2_FS_XATTR
-		init_rwsem(&ei->xattr_sem);
-#endif
-		mutex_init(&ei->truncate_mutex);
-		inode_init_once(&ei->vfs_inode);
-	}
-}
- 
 static int init_inodecache(void)
 {
 	ext2_inode_cachep = kmem_cache_create("ext2_inode_cache",
 					     sizeof(struct ext2_inode_info),
 					     0, (SLAB_RECLAIM_ACCOUNT|
 						SLAB_MEM_SPREAD),
-					     init_once, NULL);
+					     NULL, NULL);
 	if (ext2_inode_cachep == NULL)
 		return -ENOMEM;
 	return 0;

Re: [PATCH 1/4] ext2: remove inode constructor

[Andrew Morton]

On Sat, 5 May 2007 12:57:46 +0300 (EEST)
Pekka J Enberg <penberg@cs.helsinki.fi> wrote:

> From: Pekka Enberg <penberg@cs.helsinki.fi>
> 
> As alloc_inode() touches the same cache line as init_once(), we gain
> nothing from using slab constructors.
> 
> Cc: Stephen C. Tweedie <sct@redhat.com>
> Cc: Andreas Dilger <adilger@clusterfs.com>
> Cc: Christoph Lameter <clameter@sgi.com>
> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
> ---
>  fs/ext2/super.c |   32 +++++++++++++-------------------
>  1 file changed, 13 insertions(+), 19 deletions(-)
> 
> Index: 26-mm/fs/ext2/super.c
> ===================================================================
> --- 26-mm.orig/fs/ext2/super.c	2007-05-05 12:26:15.000000000 +0300
> +++ 26-mm/fs/ext2/super.c	2007-05-05 12:30:50.000000000 +0300
> @@ -140,16 +140,24 @@ static struct kmem_cache * ext2_inode_ca
>  static struct inode *ext2_alloc_inode(struct super_block *sb)
>  {
>  	struct ext2_inode_info *ei;
> -	ei = (struct ext2_inode_info *)kmem_cache_alloc(ext2_inode_cachep,
> -						GFP_KERNEL|__GFP_RECLAIMABLE);
> +	struct inode *inode;
> +
> +	ei = kmem_cache_alloc(ext2_inode_cachep, GFP_KERNEL|__GFP_RECLAIMABLE);
>  	if (!ei)
>  		return NULL;
> +	rwlock_init(&ei->i_meta_lock);
> +#ifdef CONFIG_EXT2_FS_XATTR
> +	init_rwsem(&ei->xattr_sem);
> +#endif
> +	mutex_init(&ei->truncate_mutex);

ext2 has no truncate_mutex.


These patches are rather tangled up with the unmerged __GFP_RECLAIMABLE
stuff.  I'll duck them for now.

Re: [PATCH 1/4] ext2: remove inode constructor

[Satyam Sharma]

On 5/10/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Sat, 5 May 2007 12:57:46 +0300 (EEST)
> Pekka J Enberg <penberg@cs.helsinki.fi> wrote:
>
> > From: Pekka Enberg <penberg@cs.helsinki.fi>
> >
> > As alloc_inode() touches the same cache line as init_once(), we gain
> > nothing from using slab constructors.
> >
> > Cc: Stephen C. Tweedie <sct@redhat.com>
> > Cc: Andreas Dilger <adilger@clusterfs.com>
> > Cc: Christoph Lameter <clameter@sgi.com>
> > Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
> > ---
> >  fs/ext2/super.c |   32 +++++++++++++-------------------
> >  1 file changed, 13 insertions(+), 19 deletions(-)
> >
> > Index: 26-mm/fs/ext2/super.c
> > ===================================================================
> > --- 26-mm.orig/fs/ext2/super.c        2007-05-05 12:26:15.000000000 +0300
> > +++ 26-mm/fs/ext2/super.c     2007-05-05 12:30:50.000000000 +0300
> > @@ -140,16 +140,24 @@ static struct kmem_cache * ext2_inode_ca
> >  static struct inode *ext2_alloc_inode(struct super_block *sb)
> >  {
> >       struct ext2_inode_info *ei;
> > -     ei = (struct ext2_inode_info *)kmem_cache_alloc(ext2_inode_cachep,
> > -                                             GFP_KERNEL|__GFP_RECLAIMABLE);
> > +     struct inode *inode;
> > +
> > +     ei = kmem_cache_alloc(ext2_inode_cachep, GFP_KERNEL|__GFP_RECLAIMABLE);
> >       if (!ei)
> >               return NULL;
> > +     rwlock_init(&ei->i_meta_lock);
> > +#ifdef CONFIG_EXT2_FS_XATTR
> > +     init_rwsem(&ei->xattr_sem);
> > +#endif
> > +     mutex_init(&ei->truncate_mutex);
>
> ext2 has no truncate_mutex.

2.6.21-mm2/broken-out/ext2-reservations.patch seems to have added one.

> These patches are rather tangled up with the unmerged __GFP_RECLAIMABLE
> stuff.

So _and_ the unmerged ext2-reservations stuff, you mean :-)

> I'll duck them for now.

Re: [PATCH 1/4] ext2: remove inode constructor

[Pekka Enberg]

On 5/9/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> ext2 has no truncate_mutex.

I think it does:

-               mutex_init(&ei->truncate_mutex);
-               inode_init_once(&ei->vfs_inode);
-       }
-}

And fs/ext2/super.c:init_once() from 2.6.21-mm2 says:

                mutex_init(&ei->truncate_mutex);
                inode_init_once(&ei->vfs_inode);
        }
}

Hmm?

On 5/9/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> These patches are rather tangled up with the unmerged __GFP_RECLAIMABLE
> stuff.  I'll duck them for now.

Ok. I would appreciate any kind of heads-up when you're ready to eat
these patches again. ;-)

Re: [PATCH 1/4] ext2: remove inode constructor

[Christoph Lameter]

On Thu, 10 May 2007, Pekka Enberg wrote:

> Ok. I would appreciate any kind of heads-up when you're ready to eat
> these patches again. ;-)

If you have some more spare cycles: Try to extend this 
to get rid of the inode constructors in the other fs as well?

(And if you have even more time and are touching the call sites anyways: 
Kill off SLAB_CONSTRUCTOR_CTOR. It is always set and has no purpose in life left).