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

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

[Liz Fong-Jones]

On a filesystem mounted with lazytime, sync() and syncfs() can return
without persisting timestamp updates (atime, mtime, ctime) of inodes
attached to a cgroup writeback that is being cleaned up after its
memory cgroup was removed. A crash after the sync loses them, contrary
to the lazytime guarantee that sync persists them. File data and sizes
are not affected.

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.

Since commit f6988c90671e ("writeback: bound cleanup_offline_cgwb()
rescans by rotating scanned inodes"), isw_prepare_wbs_switch() moves
every inode it walks, prepared or not, to the newest end of the list.
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
for_sync queue_io() expires b_dirty_time with the sync's fixed cutoff
and stops at the first newer inode, so it never reaches them. The
switches themselves already wait for the sync on wb_switch_rwsem; the
scan does not.

Take wb_switch_rwsem for read across the scan, as
process_inode_switch_wbs() does for the switches.

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.

Test, with a debug-only delay holding a syncfs's work on the wb between
its cutoff and queue_io(): 300 files on a cgroup's wb with lazytime
atime updates, a syncfs held after its cutoff, one more read, then the
cgroup removed so the cleanup ran. Without this patch, syncfs() queued
48 of the 300 and after a crash 252 atime updates were lost, on ext4
(-I 4096, so neighbours' timestamps are not written along) in 3 of 3
runs and on XFS in 1 of 1. With it, the scan waited for the sync, which
queued all 300, and none were lost.

Fixes: f6988c90671e ("writeback: bound cleanup_offline_cgwb() rescans by rotating scanned inodes")
Cc: stable@vger.kernel.org
Suggested-by: Tejun Heo <tj@kernel.org>
Link: https://lore.kernel.org/all/0bc5d58533f10b4841807be3025daa11@kernel.org/ [1]
Assisted-by: Claude:claude-opus-5-5 checkpatch
Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Liz Fong-Jones <lizf@honeycomb.io>
---
This came up while answering Tejun's review of my series "writeback: let
foreign flushes reach dying cgwbs" [1], whose patch 3 had the same
problem on b_dirty in its own scan; v7 of that series [2] takes the same
read lock there. This patch is independent of the series and applies to
7.3-rc7 on its own; both together apply cleanly.

Impact is limited to lazytime timestamps on inodes attached to a cgroup
wb while it is cleaned up after its memcg is removed. On ext4 with the
default inode size, a write to any inode in the same inode-table block
also writes the neighbours' timestamps, so fewer are lost there; the
test used -I 4096 to rule that out. XFS has no such rescue.

The debug knob used for the test, applied to wb_writeback() and not part
of this patch: when /sys/module/kernel/parameters/wb_debug_sync_delay_ms
is set, a WB_SYNC_ALL work on a non-root wb sleeps that long once, after
dirtied_before is fixed and before queue_io(); it also skips sync's
best-effort WB_SYNC_NONE pass on cgroup wbs, which had nothing to write
here. Steps: mount -o lazytime,strictatime; a cgroup writes 300 small
files and one more, syncfs twice; read the 300 in order (each goes to
b_dirty_time); set the knob to 10s and start syncfs; 1s later read the
extra file; 1s later remove the cgroup; after syncfs returns, crash with
sysrq-b and compare atimes on disk in a second boot. The scripts are
available if useful.

Tested on arm64 KVM guests (virtme-ng) with ext4 and XFS on virtio, on
7.3-rc7. Not tested: x86, production.

Developed with Claude Opus 5.5 and Claude Fable 5.1. Fable spotted that
cleanup_offline_cgwb()'s scan of b_dirty_time was exposed the same way
while checking Tejun's analysis, and reviewed the fix and the claims
here; Opus wrote the fix, the reproducer and this text, and ran the
tests. I directed the investigation and reviewed the analysis, code and
results.

[1] https://lore.kernel.org/all/0bc5d58533f10b4841807be3025daa11@kernel.org/
[2] https://lore.kernel.org/all/20261015-wb-dying-cgwb-flush-v7-0-9278d9ebaf5d@honeycomb.io/

 fs/fs-writeback.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index ea3eb40bf828..99570aacdd18 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -789,6 +789,11 @@ bool cleanup_offline_cgwb(struct bdi_writeback *wb)
 		new_wb = &wb->bdi->wb; /* wb_get() is noop for bdi's wb */
 
 	nr = 0;
+	/*
+	 * The scan reorders b_dirty_time.  Keep it out of a sync's way, which
+	 * could otherwise miss timestamps moved past its cutoff.
+	 */
+	down_read(&wb->bdi->wb_switch_rwsem);
 	spin_lock(&wb->list_lock);
 	/*
 	 * In addition to the inodes that have completed writeback, also switch
@@ -803,6 +808,7 @@ bool cleanup_offline_cgwb(struct bdi_writeback *wb)
 		restart = isw_prepare_wbs_switch(new_wb, isw, &wb->b_dirty_time,
 						 &nr);
 	spin_unlock(&wb->list_lock);
+	up_read(&wb->bdi->wb_switch_rwsem);
 
 	/* no attached inodes? bail out */
 	if (nr == 0) {

base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
-- 
2.53.0

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

[Tejun Heo]

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

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

[Liz Fong-Jones]

On a filesystem mounted with lazytime, sync() and syncfs() can return
without persisting timestamp updates of inodes attached to a dying
cgroup wb, so a crash after the sync loses them. File data and sizes are
not affected.

Since commit f6988c90671e ("writeback: bound cleanup_offline_cgwb()
rescans by rotating scanned inodes"), isw_prepare_wbs_switch() moves the
inodes it walks to the newest end of the list. When a pass fills its isw
before reaching an inode that got its lazytime update after a sync's
cutoff, the walked inodes land past it on b_dirty_time, and the sync's
queue_io(), which stops at the first inode dirtied after the cutoff,
misses them. The late inode has to be one without a pending lazytime
update; one already on b_dirty_time keeps its position.

The window is narrow. 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 its first
queue_io(), unless a multi-pass drain started before the wb got dirty
IO.

Take wb_switch_rwsem for read across the scan, as
process_inode_switch_wbs() does for the switches.

Fixes: f6988c90671e ("writeback: bound cleanup_offline_cgwb() rescans by rotating scanned inodes")
Cc: stable@vger.kernel.org
Suggested-by: Tejun Heo <tj@kernel.org>
Link: https://lore.kernel.org/all/0bc5d58533f10b4841807be3025daa11@kernel.org/
Assisted-by: Claude:claude-opus-5-5 checkpatch
Assisted-by: Claude:claude-fable-5-1
Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Liz Fong-Jones <lizf@honeycomb.io>
---
Changes in v2, all from Tejun's review:
- Correct the trigger and the reordering description, and note how
  narrow the window is
- Reword the code comment
- Move the test details here and trim the description

Test, with a debug-only delay holding a syncfs's work on the wb between
its cutoff and queue_io() to widen the window: 300 files on a cgroup's
wb with lazytime atime updates, one more read after the cutoff, then
the cgroup removed so the cleanup ran. Without this patch, syncfs()
queued 48 of the 300 and after a crash 252 atime updates were lost, on
ext4 (-I 4096, so neighbours' timestamps are not written along) in 3 of
3 runs and on XFS in 1 of 1; with it, none were lost. arm64 KVM guests,
7.3-rc7.

v1: https://lore.kernel.org/all/20261015114307.1041273-1-lizf@honeycomb.io/

 fs/fs-writeback.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index ea3eb40bf828..372ac9785e03 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -789,6 +789,13 @@ bool cleanup_offline_cgwb(struct bdi_writeback *wb)
 		new_wb = &wb->bdi->wb; /* wb_get() is noop for bdi's wb */
 
 	nr = 0;
+	/*
+	 * 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.
+	 */
+	down_read(&wb->bdi->wb_switch_rwsem);
 	spin_lock(&wb->list_lock);
 	/*
 	 * In addition to the inodes that have completed writeback, also switch
@@ -803,6 +810,7 @@ bool cleanup_offline_cgwb(struct bdi_writeback *wb)
 		restart = isw_prepare_wbs_switch(new_wb, isw, &wb->b_dirty_time,
 						 &nr);
 	spin_unlock(&wb->list_lock);
+	up_read(&wb->bdi->wb_switch_rwsem);
 
 	/* no attached inodes? bail out */
 	if (nr == 0) {

base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
-- 
2.53.0

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

[Tejun Heo]

On Sat, Oct 17, 2026 at 04:07:11AM +0000, Liz Fong-Jones wrote:
> On a filesystem mounted with lazytime, sync() and syncfs() can return
> without persisting timestamp updates of inodes attached to a dying
> cgroup wb, so a crash after the sync loses them. File data and sizes are
> not affected.
>
> Since commit f6988c90671e ("writeback: bound cleanup_offline_cgwb()
> rescans by rotating scanned inodes"), isw_prepare_wbs_switch() moves the
> inodes it walks to the newest end of the list. When a pass fills its isw
> before reaching an inode that got its lazytime update after a sync's
> cutoff, the walked inodes land past it on b_dirty_time, and the sync's
> queue_io(), which stops at the first inode dirtied after the cutoff,
> misses them. The late inode has to be one without a pending lazytime
> update; one already on b_dirty_time keeps its position.
>
> The window is narrow. 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 its first
> queue_io(), unless a multi-pass drain started before the wb got dirty
> IO.
>
> Take wb_switch_rwsem for read across the scan, as
> process_inode_switch_wbs() does for the switches.
>
> Fixes: f6988c90671e ("writeback: bound cleanup_offline_cgwb() rescans by rotating scanned inodes")
> Cc: stable@vger.kernel.org
> Suggested-by: Tejun Heo <tj@kernel.org>
> Link: https://lore.kernel.org/all/0bc5d58533f10b4841807be3025daa11@kernel.org/
> Assisted-by: Claude:claude-opus-5-5 checkpatch
> Assisted-by: Claude:claude-fable-5-1
> Acked-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Liz Fong-Jones <lizf@honeycomb.io>

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

Thanks.

--
tejun

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

[Liz Fong-Jones]

Christian,

This fixes f6988c90671e, which went into 7.3-rc5, so the regression is
new in this cycle, and f6988c90671e is already tagged for stable. Could
it go into vfs.fixes before 7.3 is released? Tejun has acked it:

  b4 shazam 20261017040710.1174009-1-lizf@honeycomb.io

Separately, the wb-dying-cgwb-flush series now has Tejun's ack on all
three patches and Jan's Reviewed-by, for whichever cycle suits:
https://lore.kernel.org/all/20261015-wb-dying-cgwb-flush-v7-0-9278d9ebaf5d@honeycomb.io/

Thanks,
Liz

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

[Jan Kara]

On Sat 17-10-26 04:07:11, Liz Fong-Jones wrote:
> On a filesystem mounted with lazytime, sync() and syncfs() can return
> without persisting timestamp updates of inodes attached to a dying
> cgroup wb, so a crash after the sync loses them. File data and sizes are
> not affected.
> 
> Since commit f6988c90671e ("writeback: bound cleanup_offline_cgwb()
> rescans by rotating scanned inodes"), isw_prepare_wbs_switch() moves the
> inodes it walks to the newest end of the list. When a pass fills its isw
> before reaching an inode that got its lazytime update after a sync's
> cutoff, the walked inodes land past it on b_dirty_time, and the sync's
> queue_io(), which stops at the first inode dirtied after the cutoff,
> misses them. The late inode has to be one without a pending lazytime
> update; one already on b_dirty_time keeps its position.
> 
> The window is narrow. 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 its first
> queue_io(), unless a multi-pass drain started before the wb got dirty
> IO.
> 
> Take wb_switch_rwsem for read across the scan, as
> process_inode_switch_wbs() does for the switches.
> 
> Fixes: f6988c90671e ("writeback: bound cleanup_offline_cgwb() rescans by rotating scanned inodes")
> Cc: stable@vger.kernel.org
> Suggested-by: Tejun Heo <tj@kernel.org>
> Link: https://lore.kernel.org/all/0bc5d58533f10b4841807be3025daa11@kernel.org/
> Assisted-by: Claude:claude-opus-5-5 checkpatch
> Assisted-by: Claude:claude-fable-5-1
> Acked-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Liz Fong-Jones <lizf@honeycomb.io>

Thanks! The fix looks good to me. Feel free to add:

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

								Honza

> ---
> Changes in v2, all from Tejun's review:
> - Correct the trigger and the reordering description, and note how
>   narrow the window is
> - Reword the code comment
> - Move the test details here and trim the description
> 
> Test, with a debug-only delay holding a syncfs's work on the wb between
> its cutoff and queue_io() to widen the window: 300 files on a cgroup's
> wb with lazytime atime updates, one more read after the cutoff, then
> the cgroup removed so the cleanup ran. Without this patch, syncfs()
> queued 48 of the 300 and after a crash 252 atime updates were lost, on
> ext4 (-I 4096, so neighbours' timestamps are not written along) in 3 of
> 3 runs and on XFS in 1 of 1; with it, none were lost. arm64 KVM guests,
> 7.3-rc7.
> 
> v1: https://lore.kernel.org/all/20261015114307.1041273-1-lizf@honeycomb.io/
> 
>  fs/fs-writeback.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index ea3eb40bf828..372ac9785e03 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -789,6 +789,13 @@ bool cleanup_offline_cgwb(struct bdi_writeback *wb)
>  		new_wb = &wb->bdi->wb; /* wb_get() is noop for bdi's wb */
>  
>  	nr = 0;
> +	/*
> +	 * 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.
> +	 */
> +	down_read(&wb->bdi->wb_switch_rwsem);
>  	spin_lock(&wb->list_lock);
>  	/*
>  	 * In addition to the inodes that have completed writeback, also switch
> @@ -803,6 +810,7 @@ bool cleanup_offline_cgwb(struct bdi_writeback *wb)
>  		restart = isw_prepare_wbs_switch(new_wb, isw, &wb->b_dirty_time,
>  						 &nr);
>  	spin_unlock(&wb->list_lock);
> +	up_read(&wb->bdi->wb_switch_rwsem);
>  
>  	/* no attached inodes? bail out */
>  	if (nr == 0) {
> 
> base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
> -- 
> 2.53.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

[Christian Brauner]

On Sat, 17 Oct 2026 04:07:11 +0000, Liz Fong-Jones wrote:
> writeback: keep cleanup_offline_cgwb()'s scan out of syncs

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] writeback: keep cleanup_offline_cgwb()'s scan out of syncs
      https://git.kernel.org/vfs/vfs/c/056b35228fad