Re: [PATCH] writeback: keep cleanup_offline_cgwb()'s scan out of syncs

[Tejun Heo <tj@kernel.org>]

Hello, Liz.

On Thu, Oct 15, 2026 at 11:43:09AM +0000, Liz Fong-Jones wrote:
> Trigger: a wb has lazytime-dirty inodes on b_dirty_time when a sync
> starts, an inode on it gets a timestamp update after the sync's cutoff,
> and then its memcg (or, if the wb is already dying, any memcg) is
> removed, which runs cleanup_offline_cgwb() on it before the sync work
> queues inodes.

The late inode has to be one without a pending lazytime update. One
already on b_dirty_time keeps its position and dirtied_when.

cleanup_offline_cgwbs_workfn() skips wbs with anything on b_dirty, b_io
or b_more_io. So the scan has to land between wb_writeback() fixing
dirtied_before and taking list_lock for the first queue_io(), which is a
few instructions. The exception is a multi-pass drain that started
before the wb got dirty IO, as the loop doesn't recheck. Maybe say that
the window is narrow so that stable can weigh it?

> Once an isw fills before the scan reaches a newer inode, or a later pass
> walks past it, older inodes land in front of it on b_dirty_time.

A pass keeps the inodes it walks in order, so the only reordering is
between the walked inodes and the ones a filled pass didn't reach. "or a
later pass walks past it" doesn't seem to be a separate case.

> Tejun spotted this reordering racing with a sync on b_dirty in a patch
> under review and suggested holding wb_switch_rwsem for read across that
> scan [1]. cleanup_offline_cgwb()'s scan of b_dirty_time needs the same.

The description is pretty long for a six-line fix. Maybe drop this
paragraph, which Suggested-by and Link already cover, move the test
details below ---, and tighten the rest?

> +	/*
> +	 * The scan reorders b_dirty_time.  Keep it out of a sync's way, which
> +	 * could otherwise miss timestamps moved past its cutoff.
> +	 */

The inodes' dirtied_when doesn't change. What moves is their list
position. Maybe something like:

	/*
	 * The scan moves walked inodes to the newest end of b_dirty_time,
	 * past ones it didn't reach. If one of those was dirtied after a
	 * sync's cutoff, the sync's queue_io() stops there and misses the
	 * moved ones. Exclude syncs as process_inode_switch_wbs() does.
	 */

None of the above is blocking.

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun