Re: [RFC/PATCH] ext3: remove inode constructor
On Sat, 5 May 2007 11:58:45 +0300 (EEST) Pekka J Enberg [EMAIL PROTECTED] wrote: On Fri, 4 May 2007, Andrew Morton wrote: I got 100% rejects against this because Christoph has already had his paws all over the slab constructor code everywhere. Was going to fix it up but then decided that we ought to make changes like this to ext4 as well. Ideally beforehand, but simultaneously is OK as long as it's simple enough. I'll send you proper patches for them (and will convert other filesystems too). May as well. On Fri, 4 May 2007, Andrew Morton wrote: btw, for a benchmark I'd suggest just a silly create-1-files tight loop rather than something more complex like postmark. Do you want me to redo the benchmarks or are you happy enough with the postmark numbers? I doubt if this is measurable, really. It'll be something like the difference between an L1 hit and an L2 hit in amongst all the other stuff we do on a per-inode basis. - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] ext3: remove inode constructor
On Fri, 4 May 2007, Christoph Lameter wrote: Also could this be generalized to also apply to the generic inode allocation in fs/inode.c? I think we want to stick inode_init_ince() in alloc_inode() but we can't do that until all filesystems are converted over. Pekka - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] ext3: remove inode constructor
From: Pekka Enberg [EMAIL PROTECTED] As explained by Christoph Lameter, ext3_alloc_inode() touches the same cache line as init_once() so we gain nothing from using slab constructors. The SLUB allocator will be more effective without it (free pointer can be placed inside the free'd object), so move inode initialization to ext3_alloc_inode completely. [postmark: numbers = 1, transactions = 1, 2 GHz Pentium M] 2.6.21 vanilla: real 0m19.006s user 0m0.144s sys 0m7.424s real 0m9.040s user 0m0.156s sys 0m5.164s real 0m8.939s user 0m0.128s sys 0m5.180s 2.6.21 + ext3-remove-inode-constructor: real 0m19.176s user 0m0.176s sys 0m7.436s real 0m9.030s user 0m0.172s sys 0m5.120s real 0m8.966s user 0m0.168s sys 0m5.132s Cc: Stephen C. Tweedie [EMAIL PROTECTED] Cc: Andrew Morton [EMAIL PROTECTED] Cc: Andreas Dilger [EMAIL PROTECTED] Cc: Christoph Lameter [EMAIL PROTECTED] Signed-off-by: Pekka Enberg [EMAIL PROTECTED] --- fs/ext3/super.c | 30 -- 1 file changed, 12 insertions(+), 18 deletions(-) Index: 2.6/fs/ext3/super.c === --- 2.6.orig/fs/ext3/super.c2007-05-04 12:57:09.0 +0300 +++ 2.6/fs/ext3/super.c 2007-05-04 13:01:27.0 +0300 @@ -444,17 +444,26 @@ static struct kmem_cache *ext3_inode_cac static struct inode *ext3_alloc_inode(struct super_block *sb) { struct ext3_inode_info *ei; + struct inode *inode; ei = kmem_cache_alloc(ext3_inode_cachep, GFP_NOFS); if (!ei) return NULL; +INIT_LIST_HEAD(ei-i_orphan); +#ifdef CONFIG_EXT3_FS_XATTR +init_rwsem(ei-xattr_sem); +#endif +mutex_init(ei-truncate_mutex); #ifdef CONFIG_EXT3_FS_POSIX_ACL ei-i_acl = EXT3_ACL_NOT_CACHED; ei-i_default_acl = EXT3_ACL_NOT_CACHED; #endif ei-i_block_alloc_info = NULL; - 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 ext3_destroy_inode(struct inode *inode) @@ -462,28 +471,13 @@ static void ext3_destroy_inode(struct in kmem_cache_free(ext3_inode_cachep, EXT3_I(inode)); } -static void init_once(void * foo, struct kmem_cache * cachep, unsigned long flags) -{ - struct ext3_inode_info *ei = (struct ext3_inode_info *) foo; - - if ((flags (SLAB_CTOR_VERIFY|SLAB_CTOR_CONSTRUCTOR)) == - SLAB_CTOR_CONSTRUCTOR) { - INIT_LIST_HEAD(ei-i_orphan); -#ifdef CONFIG_EXT3_FS_XATTR - init_rwsem(ei-xattr_sem); -#endif - mutex_init(ei-truncate_mutex); - inode_init_once(ei-vfs_inode); - } -} - static int init_inodecache(void) { ext3_inode_cachep = kmem_cache_create(ext3_inode_cache, sizeof(struct ext3_inode_info), 0, (SLAB_RECLAIM_ACCOUNT| SLAB_MEM_SPREAD), -init_once, NULL); +NULL, NULL); if (ext3_inode_cachep == NULL) return -ENOMEM; return 0; - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] ext3: remove inode constructor
On Fri, 4 May 2007, Pekka J Enberg wrote: Index: 2.6/fs/ext3/super.c === --- 2.6.orig/fs/ext3/super.c 2007-05-04 12:57:09.0 +0300 +++ 2.6/fs/ext3/super.c 2007-05-04 13:01:27.0 +0300 @@ -444,17 +444,26 @@ static struct kmem_cache *ext3_inode_cac static struct inode *ext3_alloc_inode(struct super_block *sb) { struct ext3_inode_info *ei; + struct inode *inode; ei = kmem_cache_alloc(ext3_inode_cachep, GFP_NOFS); if (!ei) return NULL; +INIT_LIST_HEAD(ei-i_orphan); +#ifdef CONFIG_EXT3_FS_XATTR +init_rwsem(ei-xattr_sem); +#endif +mutex_init(ei-truncate_mutex); ^^^ whitespace issues Also could this be generalized to also apply to the generic inode allocation in fs/inode.c? - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] ext3: remove inode constructor
On Fri, 4 May 2007 13:14:35 +0300 (EEST) Pekka J Enberg [EMAIL PROTECTED] wrote: As explained by Christoph Lameter, ext3_alloc_inode() touches the same cache line as init_once() so we gain nothing from using slab constructors. The SLUB allocator will be more effective without it (free pointer can be placed inside the free'd object), so move inode initialization to ext3_alloc_inode completely. I got 100% rejects against this because Christoph has already had his paws all over the slab constructor code everywhere. Was going to fix it up but then decided that we ought to make changes like this to ext4 as well. Ideally beforehand, but simultaneously is OK as long as it's simple enough. btw, for a benchmark I'd suggest just a silly create-1-files tight loop rather than something more complex like postmark. - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html