All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>,
	Al Viro <viro@ZenIV.linux.org.uk>,
	David Miller <davem@davemloft.net>,
	Christoph Hellwig <hch@infradead.org>,
	Andi Kleen <andi@firstfloor.org>, Matthew Wilcox <matthew@wil.cx>,
	Anton Blanchard <anton@samba.org>,
	npiggin@kernel.dk, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, netdev <netdev@vger.kernel.org>
Subject: Re: [PATCH] vfs: dont chain pipe/anon/socket on superblock s_inodes list
Date: Tue, 26 Jul 2011 05:03:57 -0400	[thread overview]
Message-ID: <20110726090357.GA13013@infradead.org> (raw)
In-Reply-To: <1311668466.2355.12.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>

On Tue, Jul 26, 2011 at 10:21:06AM +0200, Eric Dumazet wrote:
> Well, not 'last' contention point, as we still hit remove_inode_hash(),

There should be no ned to put pipe or anon inodes on the inode hash.
Probably sockets don't need it either, but I'd need to look at it in
detail.

> inode_wb_list_del()

The should never be on the wb list either, doing an unlocked check for
actually beeing on the list before taking the lock should help you.

> inode_lru_list_del(),

No real need to keep inodes in the LRU if we only allocate them using
new_inode but never look them up either.  You might want to try setting
.drop_inode to generic_delete_inode for these.

> +struct inode *__new_inode(struct super_block *sb)
> +{
> +	struct inode *inode = alloc_inode(sb);
> +
> +	if (inode) {
> +		spin_lock(&inode->i_lock);
> +		inode->i_state = 0;
> +		spin_unlock(&inode->i_lock);
> +		INIT_LIST_HEAD(&inode->i_sb_list);
> +	}
> +	return inode;
> +}

This needs a much better name like new_inode_pseudo, and a kerneldoc 
comment explaining when it is safe to use, and the consequences, which
appear to me:

 - fs may never be unmount
 - quotas can't work on the filesystem
 - writeback can't work on the filesystem

> @@ -814,13 +829,9 @@ struct inode *new_inode(struct super_block *sb)
>  
>  	spin_lock_prefetch(&inode_sb_list_lock);
>  
> -	inode = alloc_inode(sb);
> -	if (inode) {
> -		spin_lock(&inode->i_lock);
> -		inode->i_state = 0;
> -		spin_unlock(&inode->i_lock);
> -		inode_sb_list_add(inode);
> -	}
> +	inode = __new_inode(sb);
> +	if (inode)
> +			inode_sb_list_add(inode);

bad indentation.


  reply	other threads:[~2011-07-26  9:04 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-17  0:50 vfsmount lock issues on very large ppc64 box Anton Blanchard
2011-07-17  1:04 ` Matthew Wilcox
2011-07-17  8:46   ` Andi Kleen
2011-07-18  8:17     ` Eric Dumazet
2011-07-18 15:40       ` Christoph Hellwig
2011-07-18 15:51         ` Al Viro
2011-07-19 16:32           ` [Patch] VFS : mount lock scalability for files systems without mount point (WAS vfsmount lock issues on very large ppc64 box) Tim Chen
2011-07-21 20:40             ` Al Viro
2011-07-22  0:27               ` Tim Chen
2011-07-23 13:24                 ` Christoph Hellwig
2011-07-25 22:39                   ` Tim Chen
2011-07-25 22:51                     ` Al Viro
2011-07-25 23:22                       ` Tim Chen
2011-07-26  6:00                         ` Eric Dumazet
2011-07-26  8:21                           ` [PATCH] vfs: dont chain pipe/anon/socket on superblock s_inodes list Eric Dumazet
2011-07-26  8:21                             ` Eric Dumazet
2011-07-26  9:03                             ` Christoph Hellwig [this message]
2011-07-26  9:36                               ` Eric Dumazet
2011-07-26  9:42                                 ` Christoph Hellwig
2011-07-26 10:43                                   ` Eric Dumazet
2011-07-26 10:43                                     ` Eric Dumazet
2011-07-26 11:49                                     ` Christoph Hellwig
2011-07-27 15:21                                 ` [PATCH] vfs: avoid taking locks if inode not in lists Eric Dumazet
2011-07-27 15:21                                   ` Eric Dumazet
2011-07-27 17:12                                   ` Andi Kleen
2011-07-27 20:44                                   ` Christoph Hellwig
2011-07-27 20:59                                     ` Andi Kleen
2011-07-27 21:01                                       ` Christoph Hellwig
2011-07-28  4:11                                         ` [PATCH] vfs: conditionally call inode_wb_list_del() Eric Dumazet
2011-07-28  4:11                                           ` Eric Dumazet
2011-07-28  4:41                                     ` [PATCH] vfs: avoid taking locks if inode not in lists Eric Dumazet
2011-07-28  4:41                                       ` Eric Dumazet
2011-07-28  4:55                                     ` [PATCH] vfs: avoid call to inode_lru_list_del() if possible Eric Dumazet
2011-07-28  4:55                                       ` Eric Dumazet
2011-07-18 16:41   ` vfsmount lock issues on very large ppc64 box Tim Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110726090357.GA13013@infradead.org \
    --to=hch@infradead.org \
    --cc=andi@firstfloor.org \
    --cc=anton@samba.org \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthew@wil.cx \
    --cc=netdev@vger.kernel.org \
    --cc=npiggin@kernel.dk \
    --cc=tim.c.chen@linux.intel.com \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.