diff mbox series

[v6,4/5] writeback, cgroup: support switching multiple inodes at once

Message ID 20210603005517.1403689-5-guro@fb.com (mailing list archive)
State New
Headers show
Series cgroup, blkcg: prevent dirty inodes to pin dying memory cgroups | expand

Commit Message

Roman Gushchin June 3, 2021, 12:55 a.m. UTC
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(-)

Comments

Jan Kara June 3, 2021, 10:10 a.m. UTC | #1
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 mbox series

Patch

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