Le mardi 26 juillet 2011 à 11:36 +0200, Eric Dumazet a écrit :

Le mardi 26 juillet 2011 à 05:03 -0400, Christoph Hellwig a écrit :

quoted

On Tue, Jul 26, 2011 at 10:21:06AM +0200, Eric Dumazet wrote:

quoted

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.

quoted

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.

Yes, it might even help regular inodes ;)

quoted

quoted

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.

Yes, I'll take a look, thanks.

If I am not mistaken, we can add unlocked checks on the three hot spots.

After following patch, a close(socket(PF_INET, SOCK_DGRAM, 0)) pair on
my dev machine takes ~3us instead of ~9us.

Maybe its better to split it in three patches, just let me know.

22us -> 3us, thats a nice patch series ;)

Thanks

[PATCH] vfs: avoid taking locks if inode not in lists

sockets and pipes inodes destruction hits three possibly contended
locks :

system-wide inode_hash_lock in remove_inode_hash()
superblock s_inode_lru_lock in inode_lru_list_del()
bdi wb.list_lock in inode_wb_list_del()

Before even taking locks, we can perform an unlocked test to check if
inode can possibly be in the lists.

On a 2x4x2 machine, a close(socket()) pair can be 200% faster with these
changes.

Signed-off-by: Eric Dumazet <redacted>
---
 fs/fs-writeback.c |   10 ++++++----
 fs/inode.c        |    6 ++++++
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 1599aa9..8b90bdb 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c

@@ -182,11 +182,13 @@ void bdi_start_background_writeback(struct backing_dev_info *bdi)
  */
 void inode_wb_list_del(struct inode *inode)
 {
-	struct backing_dev_info *bdi = inode_to_bdi(inode);
+	if (!list_empty(&inode->i_wb_list)) {
+		struct backing_dev_info *bdi = inode_to_bdi(inode);
 
-	spin_lock(&bdi->wb.list_lock);
-	list_del_init(&inode->i_wb_list);
-	spin_unlock(&bdi->wb.list_lock);
+		spin_lock(&bdi->wb.list_lock);
+		list_del_init(&inode->i_wb_list);
+		spin_unlock(&bdi->wb.list_lock);
+	}
 }
 
 /*

diff --git a/fs/inode.c b/fs/inode.c
index d0c72ff..796a420 100644
--- a/fs/inode.c
+++ b/fs/inode.c

@@ -338,6 +338,9 @@ static void inode_lru_list_add(struct inode *inode)
 
 static void inode_lru_list_del(struct inode *inode)
 {
+	if (list_empty(&inode->i_lru))
+		return;
+
 	spin_lock(&inode->i_sb->s_inode_lru_lock);
 	if (!list_empty(&inode->i_lru)) {
 		list_del_init(&inode->i_lru);

@@ -406,6 +409,9 @@ EXPORT_SYMBOL(__insert_inode_hash);
  */
 void remove_inode_hash(struct inode *inode)
 {
+	if (inode_unhashed(inode))
+		return;
+
 	spin_lock(&inode_hash_lock);
 	spin_lock(&inode->i_lock);
 	hlist_del_init(&inode->i_hash);


--

To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html