Re: [PATCH v6 4/5] writeback, cgroup: support switching multiple inodes at once
From: Jan Kara <jack@suse.cz>
Date: 2021-06-03 10:11:00
Also in:
linux-fsdevel, linux-mm, lkml
On Wed 02-06-21 17:55:16, Roman Gushchin wrote:
Currently only a single inode can be switched to another writeback structure at once. That means to switch an inode a separate inode_switch_wbs_context structure must be allocated, and a separate rcu callback and work must be scheduled. It's fine for the existing ad-hoc switching, which is not happening that often, but sub-optimal for massive switching required in order to release a writeback structure. To prepare for it, let's add a support for switching multiple inodes at once. Instead of containing a single inode pointer, inode_switch_wbs_context will contain a NULL-terminated array of inode pointers. inode_do_switch_wbs() will be called for each inode. Signed-off-by: Roman Gushchin <redacted>
Two small comments below:
quoted hunk ↗ jump to hunk
@@ -473,10 +473,14 @@ static void inode_switch_wbs_work_fn(struct work_struct *work) { struct inode_switch_wbs_context *isw = container_of(to_rcu_work(work), struct inode_switch_wbs_context, work); + struct inode **inodep; + + for (inodep = &isw->inodes[0]; *inodep; inodep++) {
^^^^ why not just isw->inodes?
+ inode_do_switch_wbs(*inodep, isw->new_wb); + iput(*inodep); + }
I was kind of hoping that we would save the repeated locking of bdi->wb_switch_rwsem, old_wb->list_lock, and new_wb->list_lock for multiple inodes. Maybe we can have 'old_wb' as part of isw as well and assert that all inodes are still attached to the old_wb at this point to make this a bit simpler. Or we can fetch old_wb from the first inode and then just lock & assert using that one. Honza -- Jan Kara [off-list ref] SUSE Labs, CR