Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757378AbYLPVld (ORCPT ); Tue, 16 Dec 2008 16:41:33 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753210AbYLPVlQ (ORCPT ); Tue, 16 Dec 2008 16:41:16 -0500 Received: from e8.ny.us.ibm.com ([32.97.182.138]:57981 "EHLO e8.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752902AbYLPVlO (ORCPT ); Tue, 16 Dec 2008 16:41:14 -0500 Date: Tue, 16 Dec 2008 13:41:11 -0800 From: "Paul E. McKenney" To: Eric Dumazet Cc: Andrew Morton , Ingo Molnar , Christoph Hellwig , David Miller , "Rafael J. Wysocki" , linux-kernel@vger.kernel.org, "kernel-testers@vger.kernel.org >> Kernel Testers List" , Mike Galbraith , Peter Zijlstra , Linux Netdev List , Christoph Lameter , linux-fsdevel@vger.kernel.org, Al Viro Subject: Re: [PATCH v3 5/7] fs: new_inode_single() and iput_single() Message-ID: <20081216214111.GP6681@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com References: <49267694.1030506@cosmosbay.com> <20081121.010508.40225532.davem@davemloft.net> <4926AEDB.10007@cosmosbay.com> <4926D022.5060008@cosmosbay.com> <20081121152148.GA20388@elte.hu> <4926D39D.9050603@cosmosbay.com> <20081121153453.GA23713@elte.hu> <492DDB6A.8090806@cosmosbay.com> <493100B0.6090104@cosmosbay.com> <494196C7.2050005@cosmosbay.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <494196C7.2050005@cosmosbay.com> User-Agent: Mutt/1.5.15+20070412 (2007-04-11) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Dec 11, 2008 at 11:40:07PM +0100, Eric Dumazet wrote: > Goal of this patch is to not touch inode_lock for socket/pipes/anonfd > inodes allocation/freeing. > > SINGLE dentries are attached to inodes that dont need to be linked > in a list of inodes, being "inode_in_use" or "sb->s_inodes" > As inode_lock was taken only to protect these lists, we avoid taking it > as well. > > Using iput_single() from dput_single() avoids taking inode_lock > at freeing time. > > This patch has a very noticeable effect, because we avoid dirtying of > three contended cache lines in new_inode(), and five cache lines in iput() > > ("socketallocbench -n 8" result : from 19.9s to 3.01s) Nice! Acked-by: Paul E. McKenney > Signed-off-by: Eric Dumazet > --- > fs/anon_inodes.c | 2 +- > fs/dcache.c | 2 +- > fs/inode.c | 29 ++++++++++++++++++++--------- > fs/pipe.c | 2 +- > include/linux/fs.h | 12 +++++++++++- > net/socket.c | 2 +- > 6 files changed, 35 insertions(+), 14 deletions(-) > > diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c > index 8bf83cb..89fd36d 100644 > --- a/fs/anon_inodes.c > +++ b/fs/anon_inodes.c > @@ -125,7 +125,7 @@ EXPORT_SYMBOL_GPL(anon_inode_getfd); > */ > static struct inode *anon_inode_mkinode(void) > { > - struct inode *inode = new_inode(anon_inode_mnt->mnt_sb); > + struct inode *inode = new_inode_single(anon_inode_mnt->mnt_sb); > > if (!inode) > return ERR_PTR(-ENOMEM); > diff --git a/fs/dcache.c b/fs/dcache.c > index af3bfb3..3363853 100644 > --- a/fs/dcache.c > +++ b/fs/dcache.c > @@ -231,7 +231,7 @@ static void dput_single(struct dentry *dentry) > return; > inode = dentry->d_inode; > if (inode) > - iput(inode); > + iput_single(inode); > d_free(dentry); > } > > diff --git a/fs/inode.c b/fs/inode.c > index dc8e72a..0fdfe1b 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -221,6 +221,13 @@ void destroy_inode(struct inode *inode) > kmem_cache_free(inode_cachep, (inode)); > } > > +void iput_single(struct inode *inode) > +{ > + if (atomic_dec_and_test(&inode->i_count)) { > + destroy_inode(inode); > + percpu_counter_dec(&nr_inodes); > + } > +} > > /* > * These are initializations that only need to be done > @@ -587,8 +594,9 @@ static int last_ino_get(void) > #endif > > /** > - * new_inode - obtain an inode > + * __new_inode - obtain an inode > * @sb: superblock > + * @single: if true, dont link new inode in a list > * > * Allocates a new inode for given superblock. The default gfp_mask > * for allocations related to inode->i_mapping is GFP_HIGHUSER_PAGECACHE. > @@ -598,7 +606,7 @@ static int last_ino_get(void) > * newly created inode's mapping > * > */ > -struct inode *new_inode(struct super_block *sb) > +struct inode *__new_inode(struct super_block *sb, int single) > { > /* > * On a 32bit, non LFS stat() call, glibc will generate an EOVERFLOW > @@ -607,22 +615,25 @@ struct inode *new_inode(struct super_block *sb) > */ > struct inode * inode; > > - spin_lock_prefetch(&inode_lock); > - > inode = alloc_inode(sb); > if (inode) { > percpu_counter_inc(&nr_inodes); > inode->i_state = 0; > inode->i_ino = last_ino_get(); > - spin_lock(&inode_lock); > - list_add(&inode->i_list, &inode_in_use); > - list_add(&inode->i_sb_list, &sb->s_inodes); > - spin_unlock(&inode_lock); > + if (single) { > + INIT_LIST_HEAD(&inode->i_list); > + INIT_LIST_HEAD(&inode->i_sb_list); > + } else { > + spin_lock(&inode_lock); > + list_add(&inode->i_list, &inode_in_use); > + list_add(&inode->i_sb_list, &sb->s_inodes); > + spin_unlock(&inode_lock); > + } > } > return inode; > } > > -EXPORT_SYMBOL(new_inode); > +EXPORT_SYMBOL(__new_inode); > > void unlock_new_inode(struct inode *inode) > { > diff --git a/fs/pipe.c b/fs/pipe.c > index 4de6dd5..8c51a0d 100644 > --- a/fs/pipe.c > +++ b/fs/pipe.c > @@ -865,7 +865,7 @@ static struct dentry_operations pipefs_dentry_operations = { > > static struct inode * get_pipe_inode(void) > { > - struct inode *inode = new_inode(pipe_mnt->mnt_sb); > + struct inode *inode = new_inode_single(pipe_mnt->mnt_sb); > struct pipe_inode_info *pipe; > > if (!inode) > diff --git a/include/linux/fs.h b/include/linux/fs.h > index a789346..a702d81 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -1899,7 +1899,17 @@ extern void __iget(struct inode * inode); > extern void iget_failed(struct inode *); > extern void clear_inode(struct inode *); > extern void destroy_inode(struct inode *); > -extern struct inode *new_inode(struct super_block *); > +extern struct inode *__new_inode(struct super_block *, int); > +static inline struct inode *new_inode(struct super_block *sb) > +{ > + return __new_inode(sb, 0); > +} > +static inline struct inode *new_inode_single(struct super_block *sb) > +{ > + return __new_inode(sb, 1); > +} > +extern void iput_single(struct inode *); > + > extern int should_remove_suid(struct dentry *); > extern int file_remove_suid(struct file *); > > diff --git a/net/socket.c b/net/socket.c > index 353c928..4017409 100644 > --- a/net/socket.c > +++ b/net/socket.c > @@ -464,7 +464,7 @@ static struct socket *sock_alloc(void) > struct inode *inode; > struct socket *sock; > > - inode = new_inode(sock_mnt->mnt_sb); > + inode = new_inode_single(sock_mnt->mnt_sb); > if (!inode) > return NULL; > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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/