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