2007-05-05 16:08:33

by Pekka Enberg

[permalink] [raw]
Subject: [PATCH 1/4] ext2: remove inode constructor

From: Pekka Enberg <[email protected]>

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

Cc: Stephen C. Tweedie <[email protected]>
Cc: Andreas Dilger <[email protected]>
Cc: Christoph Lameter <[email protected]>
Signed-off-by: Pekka Enberg <[email protected]>
---
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;


2007-05-09 20:40:43

by Andrew Morton

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

On Sat, 5 May 2007 12:57:46 +0300 (EEST)
Pekka J Enberg <[email protected]> wrote:

> From: Pekka Enberg <[email protected]>
>
> As alloc_inode() touches the same cache line as init_once(), we gain
> nothing from using slab constructors.
>
> Cc: Stephen C. Tweedie <[email protected]>
> Cc: Andreas Dilger <[email protected]>
> Cc: Christoph Lameter <[email protected]>
> Signed-off-by: Pekka Enberg <[email protected]>
> ---
> 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.

2007-05-10 11:36:20

by Satyam Sharma

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

On 5/10/07, Andrew Morton <[email protected]> wrote:
> On Sat, 5 May 2007 12:57:46 +0300 (EEST)
> Pekka J Enberg <[email protected]> wrote:
>
> > From: Pekka Enberg <[email protected]>
> >
> > As alloc_inode() touches the same cache line as init_once(), we gain
> > nothing from using slab constructors.
> >
> > Cc: Stephen C. Tweedie <[email protected]>
> > Cc: Andreas Dilger <[email protected]>
> > Cc: Christoph Lameter <[email protected]>
> > Signed-off-by: Pekka Enberg <[email protected]>
> > ---
> > 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.

2007-05-10 12:56:56

by Pekka Enberg

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

On 5/9/07, Andrew Morton <[email protected]> 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 <[email protected]> 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. ;-)

2007-05-10 18:46:23

by Christoph Lameter

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

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).