Re: [PATCH] writeback: bound cleanup_offline_cgwb() rescans by rotating b_attached

["Patrick Lu (Anthropic)" <perf.patrick.lu@gmail.com>]

On Thu, Sep 10, 2026 at 01:24:52PM +0200, Jan Kara wrote:
> On Wed 09-09-26 18:50:27, Patrick Lu (Anthropic) wrote:
> > Move every scanned inode to the tail of b_attached, so the next pass
> > starts where the previous one stopped and the drain becomes linear.
> > b_attached is unordered and isw_prepare_wbs_switch() is its only
> > walker, so nobody else sees the reorder. b_dirty_time is ordered by
> > expiry for move_expired_inodes() and keeps its current scan.
>
> OK, but isn't there the very same quadratic behavior problem with
> b_dirty_time scan which you don't touch (and where your trick cannot work)?

Yes, the same thing happens there. We never saw it because none of our
filesystems are mounted with lazytime, so b_dirty_time was always empty
on the hosts we looked at.

We realized the rotation works for b_dirty_time too if the walk starts
from the oldest end instead of the newest. sync takes the whole list no
matter the order, and move_expired_inodes() picks from the oldest end,
so walking with list_for_each_entry_safe_reverse() and moving scanned
inodes to the newest end keeps the oldest unscanned inode right where
the expiry looks. Prepared inodes leave the list as soon as the switch
work runs and get a new dirtied_time_when on the new wb anyway
(9a6ebbdbd412), so the only inodes left out of order are the ones that
can never switch (DAX), and only on the dying wb.

I tried it in qemu with 100k lazytime inodes on a dying cgwb. With v1
the b_dirty_time scan under list_lock still grows from 11 to 115 ms per
pass across the drain, same as unpatched. Walking both lists from the
oldest end keeps b_attached and b_dirty_time flat at ~0.6 ms per pass,
with one loop and no flag. Does that make sense? Something like this,
which I can send as v2:

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index e744f9f9d43f..ea3eb40bf828 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -727,19 +727,34 @@ static bool isw_prepare_wbs_switch(struct bdi_writeback *new_wb,
 				   struct inode_switch_wbs_context *isw,
 				   struct list_head *list, int *nr)
 {
-	struct inode *inode;
+	struct inode *inode, *tmp;
+	LIST_HEAD(scanned);
+	bool full = false;
+
+	/*
+	 * Walk from the oldest end and move scanned inodes to the newest
+	 * end, so the next scan resumes at unscanned inodes instead of
+	 * re-walking an ever-growing run of prepared and skipped ones.
+	 * For b_dirty_time this keeps the oldest unscanned inode at the
+	 * end move_expired_inodes() picks from; b_attached is unordered.
+	 */
+	list_for_each_entry_safe_reverse(inode, tmp, list, i_io_list) {
+		list_move(&inode->i_io_list, &scanned);
 
-	list_for_each_entry(inode, list, i_io_list) {
 		if (!inode_prepare_wbs_switch(inode, new_wb))
 			continue;
 
 		isw->inodes[*nr] = inode;
 		(*nr)++;
 
-		if (*nr >= WB_MAX_INODES_PER_ISW - 1)
-			return true;
+		if (*nr >= WB_MAX_INODES_PER_ISW - 1) {
+			full = true;
+			break;
+		}
 	}
-	return false;
+	list_splice(&scanned, list);
+
+	return full;
 }
 
 /**

Thanks,
Patrick