All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josef Bacik <jbacik@fb.com>
To: <linux-fsdevel@vger.kernel.org>, <kernel-team@fb.com>,
	<viro@ZenIV.linux.org.uk>, <hch@infradead.org>,
	<david@fromorbit.com>, <jack@suse.cz>
Cc: Dave Chinner <dchinner@redhat.com>
Subject: [PATCH 4/8] sync: serialise per-superblock sync operations
Date: Tue, 23 Jun 2015 20:23:58 -0700	[thread overview]
Message-ID: <1435116242-27495-5-git-send-email-jbacik@fb.com> (raw)
In-Reply-To: <1435116242-27495-1-git-send-email-jbacik@fb.com>

From: Dave Chinner <dchinner@redhat.com>

When competing sync(2) calls walk the same filesystem, they need to
walk the list of inodes on the superblock to find all the inodes
that we need to wait for IO completion on. However, when multiple
wait_sb_inodes() calls do this at the same time, they contend on the
the inode_sb_list_lock and the contention causes system wide
slowdowns. In effect, concurrent sync(2) calls can take longer and
burn more CPU than if they were serialised.

Stop the worst of the contention by adding a per-sb mutex to wrap
around wait_sb_inodes() so that we only execute one sync(2) IO
completion walk per superblock superblock at a time and hence avoid
contention being triggered by concurrent sync(2) calls.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Josef Bacik <jbacik@fb.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Tested-by: Dave Chinner <dchinner@redhat.com>
---
 fs/fs-writeback.c  | 11 +++++++++++
 fs/super.c         |  1 +
 include/linux/fs.h |  2 ++
 3 files changed, 14 insertions(+)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 333afa3..04fbc8a 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -1361,6 +1361,15 @@ out_unlock_inode:
 }
 EXPORT_SYMBOL(__mark_inode_dirty);
 
+/*
+ * The @s_sync_lock is used to serialise concurrent sync operations
+ * to avoid lock contention problems with concurrent wait_sb_inodes() calls.
+ * Concurrent callers will block on the s_sync_lock rather than doing contending
+ * walks. The queueing maintains sync(2) required behaviour as all the IO that
+ * has been issued up to the time this function is enter is guaranteed to be
+ * completed by the time we have gained the lock and waited for all IO that is
+ * in progress regardless of the order callers are granted the lock.
+ */
 static void wait_sb_inodes(struct super_block *sb)
 {
 	struct inode *inode, *old_inode = NULL;
@@ -1371,6 +1380,7 @@ static void wait_sb_inodes(struct super_block *sb)
 	 */
 	WARN_ON(!rwsem_is_locked(&sb->s_umount));
 
+	mutex_lock(&sb->s_sync_lock);
 	spin_lock(&sb->s_inode_list_lock);
 
 	/*
@@ -1412,6 +1422,7 @@ static void wait_sb_inodes(struct super_block *sb)
 	}
 	spin_unlock(&sb->s_inode_list_lock);
 	iput(old_inode);
+	mutex_unlock(&sb->s_sync_lock);
 }
 
 /**
diff --git a/fs/super.c b/fs/super.c
index 15e57f5..ac2b64b 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -190,6 +190,7 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags)
 	s->s_flags = flags;
 	INIT_HLIST_NODE(&s->s_instances);
 	INIT_HLIST_BL_HEAD(&s->s_anon);
+	mutex_init(&s->s_sync_lock);
 	INIT_LIST_HEAD(&s->s_inodes);
 	spin_lock_init(&s->s_inode_list_lock);
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6a3f507..2d4d104 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1346,6 +1346,8 @@ struct super_block {
 	struct list_lru		s_inode_lru ____cacheline_aligned_in_smp;
 	struct rcu_head		rcu;
 
+	struct mutex		s_sync_lock;	/* sync serialisation lock */
+
 	/*
 	 * Indicates how deep in a filesystem stack this SB is
 	 */
-- 
2.1.0


  parent reply	other threads:[~2015-06-24  3:25 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-24  3:23 [PATCH 0/8] super block scalability patches V4 Josef Bacik
2015-06-24  3:23 ` [PATCH 1/8] writeback: plug writeback at a high level Josef Bacik
2015-06-24  3:23 ` [PATCH 2/8] inode: add hlist_fake to avoid the inode hash lock in evict Josef Bacik
2015-06-24  3:23 ` [PATCH 3/8] inode: convert inode_sb_list_lock to per-sb Josef Bacik
2015-06-24  3:23 ` Josef Bacik [this message]
2015-06-24  3:23 ` [PATCH 5/8] inode: rename i_wb_list to i_io_list Josef Bacik
2015-06-24  3:24 ` [PATCH 6/8] bdi: add a new writeback list for sync Josef Bacik
2015-12-09 18:40   ` Brian Foster
2015-12-10 10:08     ` Jan Kara
2015-12-11 14:37       ` Brian Foster
2015-06-24  3:24 ` [PATCH 7/8] writeback: periodically trim the writeback list Josef Bacik
2015-06-24  3:24 ` [PATCH 8/8] inode: don't softlockup when evicting inodes Josef Bacik
  -- strict thread matches above, loose matches on Subject: below --
2015-06-11 19:41 [PATCH 0/7] super block scalabilit patches V3 Josef Bacik
2015-06-11 19:41 ` [PATCH 4/8] sync: serialise per-superblock sync operations Josef Bacik
2015-06-17 12:06   ` Christoph Hellwig
2015-03-20 17:14 [PATCH 0/8] Sync and VFS scalability improvements V2 Josef Bacik
2015-03-20 17:14 ` [PATCH 4/8] sync: serialise per-superblock sync operations Josef Bacik

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=1435116242-27495-5-git-send-email-jbacik@fb.com \
    --to=jbacik@fb.com \
    --cc=david@fromorbit.com \
    --cc=dchinner@redhat.com \
    --cc=hch@infradead.org \
    --cc=jack@suse.cz \
    --cc=kernel-team@fb.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --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.