Re: [PATCH 2/2] writeback: allow for dirty metadata accounting
From: Tejun Heo <hidden>
Date: 2016-08-10 20:13:39
Also in:
linux-fsdevel
Hello, Josef. On Tue, Aug 09, 2016 at 03:08:27PM -0400, Josef Bacik wrote:
Provide a mechanism for file systems to indicate how much dirty metadata they are holding. This introduces a few things 1) Zone stats for dirty metadata, which is the same as the NR_FILE_DIRTY. 2) WB stat for dirty metadata. This way we know if we need to try and call into the file system to write out metadata. This could potentially be used in the future to make balancing of dirty pages smarter. 3) A super callback to handle writing back dirty metadata. A future patch will take advantage of this work in btrfs. Thanks, Signed-off-by: Josef Bacik <redacted>
...
+ if (!done && wb_stat(wb, WB_METADATA_DIRTY)) {
+ struct list_head list;
+
+ spin_unlock(&wb->list_lock);
+ spin_lock(&wb->bdi->sb_list_lock);
+ list_splice_init(&wb->bdi->sb_list, &list);
+ while (!list_empty(&list)) {
+ struct super_block *sb;
+
+ sb = list_first_entry(&list, struct super_block,
+ s_bdi_list);
+ list_move_tail(&wb->bdi->sb_list, &sb->s_bdi_list);It bothers me a bit that sb's can actually be off bdi->sb_list while sb_list_lock is released. Can we make this explicit? e.g. keep separate bdi sb list for sb's pending metadata writeout (like b_dirty) or by just walking the list in a smart way?
quoted hunk ↗ jump to hunk
+ if (!sb->s_op->write_metadata) + continue; + if (!trylock_super(sb)) + continue; + spin_unlock(&wb->bdi->sb_list_lock); + wrote += writeback_sb_metadata(sb, wb, work); + spin_lock(&wb->bdi->sb_list_lock); + up_read(&sb->s_umount); } + spin_unlock(&wb->bdi->sb_list_lock); + spin_lock(&wb->list_lock); } /* Leave any unwritten inodes on b_io */ return wrote;diff --git a/fs/super.c b/fs/super.c index c2ff475..940696d 100644 --- a/fs/super.c +++ b/fs/super.c@@ -215,6 +215,7 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags, spin_lock_init(&s->s_inode_list_lock); INIT_LIST_HEAD(&s->s_inodes_wb); spin_lock_init(&s->s_inode_wblist_lock); + INIT_LIST_HEAD(&s->s_bdi_list);
I can't find where sb's are actually added to the list. Is that supposed to happen from specific filesystems? Also, it might make sense to split up additions of sb_list and stat into separate patches. Thanks. -- tejun