linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] writeback: bound cleanup_offline_cgwb() rescans by rotating scanned inodes
@ 2026-09-11 18:49 Patrick Lu (Anthropic)
  2026-09-11 19:32 ` Andrew Morton
  2026-09-14  8:03 ` Jan Kara
  0 siblings, 2 replies; 3+ messages in thread
From: Patrick Lu (Anthropic) @ 2026-09-11 18:49 UTC (permalink / raw)
  To: Alexander Viro, Christian Brauner, Jan Kara, Andrew Morton,
	Dennis Zhou, Roman Gushchin, Tejun Heo, Matthew Wilcox (Oracle)
  Cc: linux-fsdevel, linux-kernel, stable, Patrick Lu (Anthropic)

cleanup_offline_cgwb() prepares at most WB_MAX_INODES_PER_ISW inodes
per call and is called again until the dying wb is drained, but every
call walks wb->b_attached and then wb->b_dirty_time from the same end.
Inodes already prepared (they stay on the list with I_WB_SWITCH set
until the switch worker runs) and inodes that cannot be switched
(I_FREEING, I_WILL_FREE, !SB_ACTIVE, DAX, already on the target wb)
stay where they are, so each pass rescans a growing run of them under
wb->list_lock and a full drain is quadratic in the number of inodes on
the list. With ~17M inodes attached to one dying cgwb we saw this end
in soft lockups, with CPUs reported stuck for 21-48s.

Walk both lists from the oldest end and move every scanned inode to
the newest end, so the next pass starts where the previous one stopped
and the drain becomes linear. b_attached is unordered, so nobody sees
the reorder there. b_dirty_time is ordered by dirtied_when, but the
oldest unscanned inode stays at the end move_expired_inodes() picks
from, sync takes the whole list regardless of order, and prepared
inodes leave the list as soon as the switch work runs and get a new
dirtied_time_when on the new wb anyway, so the only inodes left out of
order are the ones that can never switch (DAX), and only on the dying
wb.

Fixes: c22d70a162d3 ("writeback, cgroup: release dying cgwbs by switching attached inodes")
Cc: stable@vger.kernel.org
Acked-by: Tejun Heo <tj@kernel.org>
Acked-by: Roman Gushchin <roman.gushchin@linux.dev>
Assisted-by: LLM
Signed-off-by: Patrick Lu (Anthropic) <perf.patrick.lu@gmail.com>
---
Seen in production on a 6.18-based kernel: with ~17M inodes attached
to one dying cgwb, a node spent 36 minutes in back-to-back
wb->list_lock holds by the cleanup scanner (~6ms each, ~46% of wall
time, starving writeback on that wb); with v1 of this patch the same
workload drains in ~30 seconds. Also seen on stock Amazon Linux 2023
6.12.68 as isw workers spinning on the list_lock in
inode_switch_wbs_work_fn() while cleanup_offline_cgwbs_workfn() runs.

Tested v2 with a QEMU A/B at 100k inodes on b_attached and 100k
lazytime inodes on b_dirty_time: the per-pass scan under list_lock is
flat on both lists where unpatched (and v1 on b_dirty_time) grows
across the drain, all inodes switch, and on-disk timestamps match after
sync. v1 was also run patched vs unpatched on production-class hardware
at ~17M attached inodes.

Josef Bacik's patch making the drain loop report a Tasks-RCU quiescent
state [1] fixes BPF/ftrace detach stalls behind the same drain; this
patch bounds the walk itself. The two are independent.

[1] https://lore.kernel.org/linux-mm/20260909-cgwb-tasks-rcu-qs-v1-1-967a7754771f@toxicpanda.com/
---
Changes in v2:
- Rotate b_dirty_time as well, walking both lists from the oldest end
  so the expiry still sees the oldest unscanned inode first (Jan)
- Drop the unrelated comment updates
- Kept acks from Tejun and Roman since the b_attached side did not
  change
- Link to v1: https://patch.msgid.link/20260909-wb-cgwb-rotate-v1-1-f2eb994d2a46@gmail.com
---
 fs/fs-writeback.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

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;
 }
 
 /**

---
base-commit: e14d4302cbd0de773960bec33c2281508c8d8855
change-id: 20260909-wb-cgwb-rotate-f17a75facfdc

Best regards,
--  
Patrick Lu (Anthropic) <perf.patrick.lu@gmail.com>


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] writeback: bound cleanup_offline_cgwb() rescans by rotating scanned inodes
  2026-09-11 18:49 [PATCH v2] writeback: bound cleanup_offline_cgwb() rescans by rotating scanned inodes Patrick Lu (Anthropic)
@ 2026-09-11 19:32 ` Andrew Morton
  2026-09-14  8:03 ` Jan Kara
  1 sibling, 0 replies; 3+ messages in thread
From: Andrew Morton @ 2026-09-11 19:32 UTC (permalink / raw)
  To: Patrick Lu (Anthropic)
  Cc: Alexander Viro, Christian Brauner, Jan Kara, Dennis Zhou,
	Roman Gushchin, Tejun Heo, Matthew Wilcox (Oracle), linux-fsdevel,
	linux-kernel, stable

On Fri, 11 Sep 2026 18:49:49 +0000 "Patrick Lu (Anthropic)" <perf.patrick.lu@gmail.com> wrote:

> cleanup_offline_cgwb() prepares at most WB_MAX_INODES_PER_ISW inodes
> per call and is called again until the dying wb is drained, but every
> call walks wb->b_attached and then wb->b_dirty_time from the same end.
> Inodes already prepared (they stay on the list with I_WB_SWITCH set
> until the switch worker runs) and inodes that cannot be switched
> (I_FREEING, I_WILL_FREE, !SB_ACTIVE, DAX, already on the target wb)
> stay where they are, so each pass rescans a growing run of them under
> wb->list_lock and a full drain is quadratic in the number of inodes on
> the list. With ~17M inodes attached to one dying cgwb we saw this end
> in soft lockups, with CPUs reported stuck for 21-48s.

Well.  "quadratic" is a trigger word around here.  Even if it's O(n),
someone will hit it.  Thanks for working on this.

> Walk both lists from the oldest end and move every scanned inode to
> the newest end, so the next pass starts where the previous one stopped
> and the drain becomes linear. b_attached is unordered, so nobody sees
> the reorder there. b_dirty_time is ordered by dirtied_when, but the
> oldest unscanned inode stays at the end move_expired_inodes() picks
> from, sync takes the whole list regardless of order, and prepared
> inodes leave the list as soon as the switch work runs and get a new
> dirtied_time_when on the new wb anyway, so the only inodes left out of
> order are the ones that can never switch (DAX), and only on the dying
> wb.
> 
> Fixes: c22d70a162d3 ("writeback, cgroup: release dying cgwbs by switching attached inodes")
> Cc: stable@vger.kernel.org

It sounds like this.  Maintainers, please lmk if you feel backporting is not
justified.  It's a five-year-old thing.

> ---
> Seen in production on a 6.18-based kernel: with ~17M inodes attached
> to one dying cgwb, a node spent 36 minutes in back-to-back
> wb->list_lock holds by the cleanup scanner (~6ms each, ~46% of wall
> time, starving writeback on that wb); with v1 of this patch the same
> workload drains in ~30 seconds. Also seen on stock Amazon Linux 2023
> 6.12.68 as isw workers spinning on the list_lock in
> inode_switch_wbs_work_fn() while cleanup_offline_cgwbs_workfn() runs.

This is super-important info and it deserves to be above the ---.  In
fact it deserves to become the first paragraph.


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] writeback: bound cleanup_offline_cgwb() rescans by rotating scanned inodes
  2026-09-11 18:49 [PATCH v2] writeback: bound cleanup_offline_cgwb() rescans by rotating scanned inodes Patrick Lu (Anthropic)
  2026-09-11 19:32 ` Andrew Morton
@ 2026-09-14  8:03 ` Jan Kara
  1 sibling, 0 replies; 3+ messages in thread
From: Jan Kara @ 2026-09-14  8:03 UTC (permalink / raw)
  To: Patrick Lu (Anthropic)
  Cc: Alexander Viro, Christian Brauner, Jan Kara, Andrew Morton,
	Dennis Zhou, Roman Gushchin, Tejun Heo, Matthew Wilcox (Oracle),
	linux-fsdevel, linux-kernel, stable

On Fri 11-09-26 18:49:49, Patrick Lu (Anthropic) wrote:
> cleanup_offline_cgwb() prepares at most WB_MAX_INODES_PER_ISW inodes
> per call and is called again until the dying wb is drained, but every
> call walks wb->b_attached and then wb->b_dirty_time from the same end.
> Inodes already prepared (they stay on the list with I_WB_SWITCH set
> until the switch worker runs) and inodes that cannot be switched
> (I_FREEING, I_WILL_FREE, !SB_ACTIVE, DAX, already on the target wb)
> stay where they are, so each pass rescans a growing run of them under
> wb->list_lock and a full drain is quadratic in the number of inodes on
> the list. With ~17M inodes attached to one dying cgwb we saw this end
> in soft lockups, with CPUs reported stuck for 21-48s.
> 
> Walk both lists from the oldest end and move every scanned inode to
> the newest end, so the next pass starts where the previous one stopped
> and the drain becomes linear. b_attached is unordered, so nobody sees
> the reorder there. b_dirty_time is ordered by dirtied_when, but the
> oldest unscanned inode stays at the end move_expired_inodes() picks
> from, sync takes the whole list regardless of order, and prepared
> inodes leave the list as soon as the switch work runs and get a new
> dirtied_time_when on the new wb anyway, so the only inodes left out of
> order are the ones that can never switch (DAX), and only on the dying
> wb.
> 
> Fixes: c22d70a162d3 ("writeback, cgroup: release dying cgwbs by switching attached inodes")
> Cc: stable@vger.kernel.org
> Acked-by: Tejun Heo <tj@kernel.org>
> Acked-by: Roman Gushchin <roman.gushchin@linux.dev>
> Assisted-by: LLM
> Signed-off-by: Patrick Lu (Anthropic) <perf.patrick.lu@gmail.com>

Looks good to me now. Thanks! Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
> Seen in production on a 6.18-based kernel: with ~17M inodes attached
> to one dying cgwb, a node spent 36 minutes in back-to-back
> wb->list_lock holds by the cleanup scanner (~6ms each, ~46% of wall
> time, starving writeback on that wb); with v1 of this patch the same
> workload drains in ~30 seconds. Also seen on stock Amazon Linux 2023
> 6.12.68 as isw workers spinning on the list_lock in
> inode_switch_wbs_work_fn() while cleanup_offline_cgwbs_workfn() runs.
> 
> Tested v2 with a QEMU A/B at 100k inodes on b_attached and 100k
> lazytime inodes on b_dirty_time: the per-pass scan under list_lock is
> flat on both lists where unpatched (and v1 on b_dirty_time) grows
> across the drain, all inodes switch, and on-disk timestamps match after
> sync. v1 was also run patched vs unpatched on production-class hardware
> at ~17M attached inodes.
> 
> Josef Bacik's patch making the drain loop report a Tasks-RCU quiescent
> state [1] fixes BPF/ftrace detach stalls behind the same drain; this
> patch bounds the walk itself. The two are independent.
> 
> [1] https://lore.kernel.org/linux-mm/20260909-cgwb-tasks-rcu-qs-v1-1-967a7754771f@toxicpanda.com/
> ---
> Changes in v2:
> - Rotate b_dirty_time as well, walking both lists from the oldest end
>   so the expiry still sees the oldest unscanned inode first (Jan)
> - Drop the unrelated comment updates
> - Kept acks from Tejun and Roman since the b_attached side did not
>   change
> - Link to v1: https://patch.msgid.link/20260909-wb-cgwb-rotate-v1-1-f2eb994d2a46@gmail.com
> ---
>  fs/fs-writeback.c | 25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> 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;
>  }
>  
>  /**
> 
> ---
> base-commit: e14d4302cbd0de773960bec33c2281508c8d8855
> change-id: 20260909-wb-cgwb-rotate-f17a75facfdc
> 
> Best regards,
> --  
> Patrick Lu (Anthropic) <perf.patrick.lu@gmail.com>
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-14  8:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-11 18:49 [PATCH v2] writeback: bound cleanup_offline_cgwb() rescans by rotating scanned inodes Patrick Lu (Anthropic)
2026-09-11 19:32 ` Andrew Morton
2026-09-14  8:03 ` Jan Kara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).