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

From: Tejun Heo

Date: Fri Oct 16 2026 - 17:30:39 EST


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@xxxxxxxxxx>

Thanks.

--
tejun