* [PATCH v8 1/2] sb: add a new writeback list for sync
2016-06-22 11:23 [PATCH v8 0/2] improve sync efficiency with sb inode wb list Brian Foster
@ 2016-06-22 11:23 ` Brian Foster
2016-06-22 12:46 ` Holger Hoffstätte
2016-06-22 11:23 ` [PATCH v8 2/2] wb: inode writeback list tracking tracepoints Brian Foster
2016-06-22 13:12 ` [PATCH v8 0/2] improve sync efficiency with sb inode wb list Jan Kara
2 siblings, 1 reply; 5+ messages in thread
From: Brian Foster @ 2016-06-22 11:23 UTC (permalink / raw)
To: linux-fsdevel, linux-mm
Cc: Al Viro, Andrew Morton, Dave Chinner, Josef Bacik, Jan Kara,
Holger Hoffstatte
From: Dave Chinner <dchinner@redhat.com>
wait_sb_inodes() currently does a walk of all inodes in the
filesystem to find dirty one to wait on during sync. This is highly
inefficient and wastes a lot of CPU when there are lots of clean
cached inodes that we don't need to wait on.
To avoid this "all inode" walk, we need to track inodes that are
currently under writeback that we need to wait for. We do this by
adding inodes to a writeback list on the sb when the mapping is
first tagged as having pages under writeback. wait_sb_inodes() can
then walk this list of "inodes under IO" and wait specifically just
for the inodes that the current sync(2) needs to wait for.
Define a couple helpers to add/remove an inode from the writeback
list and call them when the overall mapping is tagged for or cleared
from writeback. Update wait_sb_inodes() to walk only the inodes
under writeback due to the sync.
With this change, filesystem sync times are significantly reduced for
fs' with largely populated inode caches and otherwise no other work to
do. For example, on a 16xcpu 2GHz x86-64 server, 10TB XFS filesystem
with a ~10m entry inode cache, sync times are reduced from ~7.3s to less
than 0.1s when the filesystem is fully clean.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Josef Bacik <jbacik@fb.com>
Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Tested-by: Holger HoffstA?tte <holger.hoffstaette@applied-asynchrony.com>
---
fs/fs-writeback.c | 106 +++++++++++++++++++++++++++++++++++-----------
fs/inode.c | 2 +
fs/super.c | 2 +
include/linux/fs.h | 4 ++
include/linux/writeback.h | 3 ++
mm/page-writeback.c | 18 ++++++++
6 files changed, 110 insertions(+), 25 deletions(-)
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 989a2ce..9dbf4d1 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -981,6 +981,37 @@ void inode_io_list_del(struct inode *inode)
}
/*
+ * mark an inode as under writeback on the sb
+ */
+void sb_mark_inode_writeback(struct inode *inode)
+{
+ struct super_block *sb = inode->i_sb;
+ unsigned long flags;
+
+ if (list_empty(&inode->i_wb_list)) {
+ spin_lock_irqsave(&sb->s_inode_wblist_lock, flags);
+ if (list_empty(&inode->i_wb_list))
+ list_add_tail(&inode->i_wb_list, &sb->s_inodes_wb);
+ spin_unlock_irqrestore(&sb->s_inode_wblist_lock, flags);
+ }
+}
+
+/*
+ * clear an inode as under writeback on the sb
+ */
+void sb_clear_inode_writeback(struct inode *inode)
+{
+ struct super_block *sb = inode->i_sb;
+ unsigned long flags;
+
+ if (!list_empty(&inode->i_wb_list)) {
+ spin_lock_irqsave(&sb->s_inode_wblist_lock, flags);
+ list_del_init(&inode->i_wb_list);
+ spin_unlock_irqrestore(&sb->s_inode_wblist_lock, flags);
+ }
+}
+
+/*
* Redirty an inode: set its when-it-was dirtied timestamp and move it to the
* furthest end of its superblock's dirty-inode list.
*
@@ -2154,7 +2185,7 @@ EXPORT_SYMBOL(__mark_inode_dirty);
*/
static void wait_sb_inodes(struct super_block *sb)
{
- struct inode *inode, *old_inode = NULL;
+ LIST_HEAD(sync_list);
/*
* We need to be protected against the filesystem going from
@@ -2163,38 +2194,60 @@ 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);
/*
- * Data integrity sync. Must wait for all pages under writeback,
- * because there may have been pages dirtied before our sync
- * call, but which had writeout started before we write it out.
- * In which case, the inode may not be on the dirty list, but
- * we still have to wait for that writeout.
+ * Splice the writeback list onto a temporary list to avoid waiting on
+ * inodes that have started writeback after this point.
+ *
+ * Use rcu_read_lock() to keep the inodes around until we have a
+ * reference. s_inode_wblist_lock protects sb->s_inodes_wb as well as
+ * the local list because inodes can be dropped from either by writeback
+ * completion.
+ */
+ rcu_read_lock();
+ spin_lock_irq(&sb->s_inode_wblist_lock);
+ list_splice_init(&sb->s_inodes_wb, &sync_list);
+
+ /*
+ * Data integrity sync. Must wait for all pages under writeback, because
+ * there may have been pages dirtied before our sync call, but which had
+ * writeout started before we write it out. In which case, the inode
+ * may not be on the dirty list, but we still have to wait for that
+ * writeout.
*/
- list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
+ while (!list_empty(&sync_list)) {
+ struct inode *inode = list_first_entry(&sync_list, struct inode,
+ i_wb_list);
struct address_space *mapping = inode->i_mapping;
+ /*
+ * Move each inode back to the wb list before we drop the lock
+ * to preserve consistency between i_wb_list and the mapping
+ * writeback tag. Writeback completion is responsible to remove
+ * the inode from either list once the writeback tag is cleared.
+ */
+ list_move_tail(&inode->i_wb_list, &sb->s_inodes_wb);
+
+ /*
+ * The mapping can appear untagged while still on-list since we
+ * do not have the mapping lock. Skip it here, wb completion
+ * will remove it.
+ */
+ if (!mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK))
+ continue;
+
+ spin_unlock_irq(&sb->s_inode_wblist_lock);
+
spin_lock(&inode->i_lock);
- if ((inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) ||
- (mapping->nrpages == 0)) {
+ if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) {
spin_unlock(&inode->i_lock);
+
+ spin_lock_irq(&sb->s_inode_wblist_lock);
continue;
}
__iget(inode);
spin_unlock(&inode->i_lock);
- spin_unlock(&sb->s_inode_list_lock);
-
- /*
- * We hold a reference to 'inode' so it couldn't have been
- * removed from s_inodes list while we dropped the
- * s_inode_list_lock. We cannot iput the inode now as we can
- * be holding the last reference and we cannot iput it under
- * s_inode_list_lock. So we keep the reference and iput it
- * later.
- */
- iput(old_inode);
- old_inode = inode;
+ rcu_read_unlock();
/*
* We keep the error status of individual mapping so that
@@ -2205,10 +2258,13 @@ static void wait_sb_inodes(struct super_block *sb)
cond_resched();
- spin_lock(&sb->s_inode_list_lock);
+ iput(inode);
+
+ rcu_read_lock();
+ spin_lock_irq(&sb->s_inode_wblist_lock);
}
- spin_unlock(&sb->s_inode_list_lock);
- iput(old_inode);
+ spin_unlock_irq(&sb->s_inode_wblist_lock);
+ rcu_read_unlock();
mutex_unlock(&sb->s_sync_lock);
}
diff --git a/fs/inode.c b/fs/inode.c
index 4ccbc21..e171f7b 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -365,6 +365,7 @@ void inode_init_once(struct inode *inode)
INIT_HLIST_NODE(&inode->i_hash);
INIT_LIST_HEAD(&inode->i_devices);
INIT_LIST_HEAD(&inode->i_io_list);
+ INIT_LIST_HEAD(&inode->i_wb_list);
INIT_LIST_HEAD(&inode->i_lru);
address_space_init_once(&inode->i_data);
i_size_ordered_init(inode);
@@ -507,6 +508,7 @@ void clear_inode(struct inode *inode)
BUG_ON(!list_empty(&inode->i_data.private_list));
BUG_ON(!(inode->i_state & I_FREEING));
BUG_ON(inode->i_state & I_CLEAR);
+ BUG_ON(!list_empty(&inode->i_wb_list));
/* don't need i_lock here, no concurrent mods to i_state */
inode->i_state = I_FREEING | I_CLEAR;
}
diff --git a/fs/super.c b/fs/super.c
index d78b984..5806ffd 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -206,6 +206,8 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags)
mutex_init(&s->s_sync_lock);
INIT_LIST_HEAD(&s->s_inodes);
spin_lock_init(&s->s_inode_list_lock);
+ INIT_LIST_HEAD(&s->s_inodes_wb);
+ spin_lock_init(&s->s_inode_wblist_lock);
if (list_lru_init_memcg(&s->s_dentry_lru))
goto fail;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index dd28814..0c9ebf5 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -665,6 +665,7 @@ struct inode {
#endif
struct list_head i_lru; /* inode LRU list */
struct list_head i_sb_list;
+ struct list_head i_wb_list; /* backing dev writeback list */
union {
struct hlist_head i_dentry;
struct rcu_head i_rcu;
@@ -1448,6 +1449,9 @@ struct super_block {
/* s_inode_list_lock protects s_inodes */
spinlock_t s_inode_list_lock ____cacheline_aligned_in_smp;
struct list_head s_inodes; /* all inodes */
+
+ spinlock_t s_inode_wblist_lock;
+ struct list_head s_inodes_wb; /* writeback inodes */
};
extern struct timespec current_fs_time(struct super_block *sb);
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index d0b5ca5..717e614 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -384,4 +384,7 @@ void tag_pages_for_writeback(struct address_space *mapping,
void account_page_redirty(struct page *page);
+void sb_mark_inode_writeback(struct inode *inode);
+void sb_clear_inode_writeback(struct inode *inode);
+
#endif /* WRITEBACK_H */
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index e248194..8195eb4 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2747,6 +2747,11 @@ int test_clear_page_writeback(struct page *page)
__wb_writeout_inc(wb);
}
}
+
+ if (mapping->host && !mapping_tagged(mapping,
+ PAGECACHE_TAG_WRITEBACK))
+ sb_clear_inode_writeback(mapping->host);
+
spin_unlock_irqrestore(&mapping->tree_lock, flags);
} else {
ret = TestClearPageWriteback(page);
@@ -2774,11 +2779,24 @@ int __test_set_page_writeback(struct page *page, bool keep_write)
spin_lock_irqsave(&mapping->tree_lock, flags);
ret = TestSetPageWriteback(page);
if (!ret) {
+ bool on_wblist;
+
+ on_wblist = mapping_tagged(mapping,
+ PAGECACHE_TAG_WRITEBACK);
+
radix_tree_tag_set(&mapping->page_tree,
page_index(page),
PAGECACHE_TAG_WRITEBACK);
if (bdi_cap_account_writeback(bdi))
__inc_wb_stat(inode_to_wb(inode), WB_WRITEBACK);
+
+ /*
+ * We can come through here when swapping anonymous
+ * pages, so we don't necessarily have an inode to track
+ * for sync.
+ */
+ if (mapping->host && !on_wblist)
+ sb_mark_inode_writeback(mapping->host);
}
if (!PageDirty(page))
radix_tree_tag_clear(&mapping->page_tree,
--
2.5.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v8 2/2] wb: inode writeback list tracking tracepoints
2016-06-22 11:23 [PATCH v8 0/2] improve sync efficiency with sb inode wb list Brian Foster
2016-06-22 11:23 ` [PATCH v8 1/2] sb: add a new writeback list for sync Brian Foster
@ 2016-06-22 11:23 ` Brian Foster
2016-06-22 13:12 ` [PATCH v8 0/2] improve sync efficiency with sb inode wb list Jan Kara
2 siblings, 0 replies; 5+ messages in thread
From: Brian Foster @ 2016-06-22 11:23 UTC (permalink / raw)
To: linux-fsdevel, linux-mm
Cc: Al Viro, Andrew Morton, Dave Chinner, Josef Bacik, Jan Kara,
Holger Hoffstatte
The per-sb inode writeback list tracks inodes currently under writeback
to facilitate efficient sync processing. In particular, it ensures that
sync only needs to walk through a list of inodes that were cleaned by
the sync.
Add a couple tracepoints to help identify when inodes are added/removed
to and from the writeback lists. Piggyback off of the writeback lazytime
tracepoint template as it already tracks the relevant inode information.
Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
fs/fs-writeback.c | 9 +++++++--
include/trace/events/writeback.h | 22 ++++++++++++++++++----
2 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 9dbf4d1..e21d20b 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -990,8 +990,10 @@ void sb_mark_inode_writeback(struct inode *inode)
if (list_empty(&inode->i_wb_list)) {
spin_lock_irqsave(&sb->s_inode_wblist_lock, flags);
- if (list_empty(&inode->i_wb_list))
+ if (list_empty(&inode->i_wb_list)) {
list_add_tail(&inode->i_wb_list, &sb->s_inodes_wb);
+ trace_sb_mark_inode_writeback(inode);
+ }
spin_unlock_irqrestore(&sb->s_inode_wblist_lock, flags);
}
}
@@ -1006,7 +1008,10 @@ void sb_clear_inode_writeback(struct inode *inode)
if (!list_empty(&inode->i_wb_list)) {
spin_lock_irqsave(&sb->s_inode_wblist_lock, flags);
- list_del_init(&inode->i_wb_list);
+ if (!list_empty(&inode->i_wb_list)) {
+ list_del_init(&inode->i_wb_list);
+ trace_sb_clear_inode_writeback(inode);
+ }
spin_unlock_irqrestore(&sb->s_inode_wblist_lock, flags);
}
}
diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
index 73614ce..531f581 100644
--- a/include/trace/events/writeback.h
+++ b/include/trace/events/writeback.h
@@ -696,7 +696,7 @@ DEFINE_EVENT(writeback_single_inode_template, writeback_single_inode,
TP_ARGS(inode, wbc, nr_to_write)
);
-DECLARE_EVENT_CLASS(writeback_lazytime_template,
+DECLARE_EVENT_CLASS(writeback_inode_template,
TP_PROTO(struct inode *inode),
TP_ARGS(inode),
@@ -723,25 +723,39 @@ DECLARE_EVENT_CLASS(writeback_lazytime_template,
show_inode_state(__entry->state), __entry->mode)
);
-DEFINE_EVENT(writeback_lazytime_template, writeback_lazytime,
+DEFINE_EVENT(writeback_inode_template, writeback_lazytime,
TP_PROTO(struct inode *inode),
TP_ARGS(inode)
);
-DEFINE_EVENT(writeback_lazytime_template, writeback_lazytime_iput,
+DEFINE_EVENT(writeback_inode_template, writeback_lazytime_iput,
TP_PROTO(struct inode *inode),
TP_ARGS(inode)
);
-DEFINE_EVENT(writeback_lazytime_template, writeback_dirty_inode_enqueue,
+DEFINE_EVENT(writeback_inode_template, writeback_dirty_inode_enqueue,
TP_PROTO(struct inode *inode),
TP_ARGS(inode)
);
+/*
+ * Inode writeback list tracking.
+ */
+
+DEFINE_EVENT(writeback_inode_template, sb_mark_inode_writeback,
+ TP_PROTO(struct inode *inode),
+ TP_ARGS(inode)
+);
+
+DEFINE_EVENT(writeback_inode_template, sb_clear_inode_writeback,
+ TP_PROTO(struct inode *inode),
+ TP_ARGS(inode)
+);
+
#endif /* _TRACE_WRITEBACK_H */
/* This part must be outside protection */
--
2.5.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v8 0/2] improve sync efficiency with sb inode wb list
2016-06-22 11:23 [PATCH v8 0/2] improve sync efficiency with sb inode wb list Brian Foster
2016-06-22 11:23 ` [PATCH v8 1/2] sb: add a new writeback list for sync Brian Foster
2016-06-22 11:23 ` [PATCH v8 2/2] wb: inode writeback list tracking tracepoints Brian Foster
@ 2016-06-22 13:12 ` Jan Kara
2 siblings, 0 replies; 5+ messages in thread
From: Jan Kara @ 2016-06-22 13:12 UTC (permalink / raw)
To: Brian Foster
Cc: linux-fsdevel, linux-mm, Al Viro, Andrew Morton, Dave Chinner,
Josef Bacik, Jan Kara, Holger Hoffstatte
On Wed 22-06-16 07:23:11, Brian Foster wrote:
> This is just a rebase to linus' latest master. I haven't heard any
> feedback on this one so Jan suggested I send to a wider audience.
Yeah. Brian is sending these patches for a few months and they get ignored.
Andrew, can you please pick them up if Al doesn't? Thanks!
Honza
> v8:
> - Rebased to latest master.
> - Added Holger's Tested-by.
> v7: http://marc.info/?l=linux-fsdevel&m=145349651407631&w=2
> - Updated patch 1/2 commit log description to reference performance
> impact.
> v6: http://marc.info/?l=linux-fsdevel&m=145322635828644&w=2
> - Use rcu locking instead of s_inode_list_lock spinlock in
> wait_sb_inodes().
> - Refactor wait_sb_inodes() to keep inode on wb list.
> - Drop remaining, unnecessary lazy list removal bits and relocate inode
> list check to clear_inode().
> - Fix up some comments, etc.
> v5: http://marc.info/?l=linux-fsdevel&m=145262374402798&w=2
> - Converted from per-bdi list to per-sb list. Also marked as RFC and
> dropped testing/review tags.
> - Updated to use new irq-safe lock for wb list.
> - Dropped lazy list removal. Inodes are removed when the mapping is
> cleared of the writeback tag.
> - Tweaked wait_sb_inodes() to remove deferred iput(), other cleanups.
> - Added wb list tracepoint patch.
> v4: http://marc.info/?l=linux-fsdevel&m=143511628828000&w=2
>
> Brian Foster (1):
> wb: inode writeback list tracking tracepoints
>
> Dave Chinner (1):
> sb: add a new writeback list for sync
>
> fs/fs-writeback.c | 111 ++++++++++++++++++++++++++++++---------
> fs/inode.c | 2 +
> fs/super.c | 2 +
> include/linux/fs.h | 4 ++
> include/linux/writeback.h | 3 ++
> include/trace/events/writeback.h | 22 ++++++--
> mm/page-writeback.c | 18 +++++++
> 7 files changed, 133 insertions(+), 29 deletions(-)
>
> --
> 2.5.5
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 5+ messages in thread