| Message ID | 20210603005517.1403689-5-guro@fb.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cgroup, blkcg: prevent dirty inodes to pin dying memory cgroups | expand |
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 <guro@fb.com> Two small comments below: > @@ -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
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index 212494d89cc2..49d7b23a7cfe 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -335,10 +335,10 @@ static struct bdi_writeback *inode_to_wb_and_lock_list(struct inode *inode) } struct inode_switch_wbs_context { - struct inode *inode; - struct bdi_writeback *new_wb; - struct rcu_work work; + + struct bdi_writeback *new_wb; + struct inode *inodes[]; }; static void bdi_down_write_wb_switch_rwsem(struct backing_dev_info *bdi) @@ -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++) { + inode_do_switch_wbs(*inodep, isw->new_wb); + iput(*inodep); + } - inode_do_switch_wbs(isw->inode, isw->new_wb); wb_put(isw->new_wb); - iput(isw->inode); kfree(isw); atomic_dec(&isw_nr_in_flight); } @@ -503,7 +507,7 @@ static void inode_switch_wbs(struct inode *inode, int new_wb_id) if (atomic_read(&isw_nr_in_flight) > WB_FRN_MAX_IN_FLIGHT) return; - isw = kzalloc(sizeof(*isw), GFP_ATOMIC); + isw = kzalloc(sizeof(*isw) + 2 * sizeof(struct inode *), GFP_ATOMIC); if (!isw) return; @@ -528,7 +532,7 @@ static void inode_switch_wbs(struct inode *inode, int new_wb_id) __iget(inode); spin_unlock(&inode->i_lock); - isw->inode = inode; + isw->inodes[0] = inode; /* * In addition to synchronizing among switchers, I_WB_SWITCH tells
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 <guro@fb.com> --- fs/fs-writeback.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)