* [PATCH 2/7] f2fs: fix to return -EIO from f2fs_sync_fs() if cp_error is set
2026-10-22 12:52 [PATCH 1/7] f2fs: fix to persist i_size extended inside last block for fsync Chao Yu
@ 2026-10-22 12:52 ` Chao Yu
2026-10-22 12:52 ` [PATCH 3/7] f2fs: fix to return -EIO from f2fs_write_checkpoint() once " Chao Yu
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Chao Yu @ 2026-10-22 12:52 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu, stable
From: Chao Yu <chao@kernel.org>
f2fs_sync_fs() returns 0 w/o doing checkpoint once cp_error is set, so
its callers can not tell whether checkpoint was done or not.
For a file which needs a checkpoint to persist its fsynced data, e.g. a
compressed file (CP_COMPRESSED), f2fs_do_sync_file() calls
f2fs_sync_fs(). So if shutdown (e.g. F2FS_GOING_DOWN_NOSYNC) races w/
fsync, after f2fs_sync_file() passed its cp_error check, fsync() returns
0 while neither checkpoint nor fsync node blocks were written: after
recovery, the fsynced data of the compressed file is rolled back to an
old version or lost.
In addition, syncfs(2) and F2FS_IOC_WRITE_CHECKPOINT report false
success on a stopped filesystem as well, whereas other filesystems
return -EIO from ->sync_fs after shutdown.
Note that f2fs_write_checkpoint() already returns -EIO if cp_error is
set, so callers of f2fs_sync_fs() have to handle -EIO anyway.
So let's return -EIO from f2fs_sync_fs() if cp_error is set, and in
f2fs_do_sync_file(), bail out w/o clearing FI_APPEND_WRITE and
FI_UPDATE_WRITE if f2fs_sync_fs() fails, as the non-checkpoint path
already does after f2fs_fsync_node_caches().
For shutdown, F2FS_GOING_DOWN_METASYNC and F2FS_GOING_DOWN_NEED_FSCK
have already converted -EIO from f2fs_sync_fs() to 0, however,
F2FS_GOING_DOWN_FULLSYNC fails w/ -EIO once cp_error is set, e.g. when
the filesystem is shutdown again, because:
- f2fs_do_shutdown
- bdev_freeze
- fs_bdev_freeze
- freeze_super
- sync_filesystem
- f2fs_sync_fs_super
- f2fs_sync_fs
: return -EIO due to cp_error
- f2fs_freeze
: return -EIO due to cp_error as well
whereas other filesystems return 0 in this case. So let's do the
conversion for all shutdown modes at the end of f2fs_do_shutdown(), and
only if cp_error is set, so that the error is still returned to the
caller if the filesystem was not stopped, e.g. bdev_freeze() fails due
to other reasons.
Cc: stable@kernel.org
Fixes: 1f227a3e215d ("f2fs: stop all the operations by cp_error flag")
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/file.c | 16 ++++++++++------
fs/f2fs/super.c | 2 +-
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index b077e4eb790d..27e295bdce1c 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -417,6 +417,8 @@ static int f2fs_do_sync_file(struct file *file, loff_t start, loff_t end,
if (cp_reason) {
/* all the dirty node pages should be flushed for POR */
ret = f2fs_sync_fs(inode->i_sb, 1);
+ if (ret)
+ goto out;
/*
* We've secured consistency through sync_fs. Following pino
@@ -2564,11 +2566,8 @@ int f2fs_do_shutdown(struct f2fs_sb_info *sbi, unsigned int flag,
case F2FS_GOING_DOWN_METASYNC:
/* do checkpoint only */
ret = f2fs_sync_fs(sb, 1);
- if (ret) {
- if (ret == -EIO)
- ret = 0;
+ if (ret)
goto out;
- }
f2fs_stop_checkpoint(sbi, false, STOP_CP_REASON_SHUTDOWN);
break;
case F2FS_GOING_DOWN_NOSYNC:
@@ -2584,8 +2583,6 @@ int f2fs_do_shutdown(struct f2fs_sb_info *sbi, unsigned int flag,
set_sbi_flag(sbi, SBI_IS_DIRTY);
/* do checkpoint only */
ret = f2fs_sync_fs(sb, 1);
- if (ret == -EIO)
- ret = 0;
goto out;
default:
ret = -EINVAL;
@@ -2613,6 +2610,13 @@ int f2fs_do_shutdown(struct f2fs_sb_info *sbi, unsigned int flag,
f2fs_update_time(sbi, REQ_TIME);
out:
+ /*
+ * If filesystem has already stopped, e.g. it was shutdown before,
+ * bdev_freeze() or f2fs_sync_fs() will fail w/ -EIO, let's return
+ * 0 for such case, as other filesystems did.
+ */
+ if (ret == -EIO && f2fs_cp_error(sbi))
+ ret = 0;
trace_f2fs_shutdown(sbi, flag, ret);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 149ad6061097..ba2ec6123251 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2130,7 +2130,7 @@ int f2fs_sync_fs(struct super_block *sb, int sync)
int err = 0;
if (unlikely(f2fs_cp_error(sbi)))
- return 0;
+ return -EIO;
if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED)))
return 0;
--
2.49.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 3/7] f2fs: fix to return -EIO from f2fs_write_checkpoint() once cp_error is set
2026-10-22 12:52 [PATCH 1/7] f2fs: fix to persist i_size extended inside last block for fsync Chao Yu
2026-10-22 12:52 ` [PATCH 2/7] f2fs: fix to return -EIO from f2fs_sync_fs() if cp_error is set Chao Yu
@ 2026-10-22 12:52 ` Chao Yu
2026-10-22 12:52 ` [PATCH 4/7] f2fs: fix to truncate stale meta cache entry on in-place update Chao Yu
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Chao Yu @ 2026-10-22 12:52 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu, stable
From: Chao Yu <chao@kernel.org>
f2fs_write_checkpoint() skips a CP_SYNC/CP_FASTBOOT/CP_DISCARD request
w/ err = 0 if the superblock is not dirty, and this check is done before
the f2fs_cp_error() check.
However, do_checkpoint() clears SBI_IS_DIRTY unconditionally, even when
the checkpoint fails due to cp_error (e.g. shutdown), and only returns
-EIO at the end. So a checkpoint request which was queued before cp_error
was set, and waits on cp_global_sem or in the checkpoint merge queue, will
see a clean superblock once the previous checkpoint failed, and returns 0
w/o doing checkpoint.
For a file which relies on checkpoint to persist its fsynced data, e.g. a
compressed file (CP_COMPRESSED), f2fs_do_sync_file() -> f2fs_sync_fs()
then returns 0, and fsync() reports false success, whereas its data was
neither covered by a checkpoint nor by fsync node blocks: after recovery,
the fsynced data is rolled back to an old version.
Note that commit e41de21d78ec ("f2fs: fix to return -EIO from
f2fs_sync_fs() if cp_error is set") only covers the case cp_error is set
before f2fs_sync_fs() is called, not the case it is set while the request
is pending.
This was reproduced by the f2fs consistency test (verify_consistency.sh
--init zoned -S 64 --mode consistency, w/ compress_algorithm=lz4 and
default checkpoint_merge, under fault injection): fsync_verify.py randomly
overwrites 4K blocks w/ a generation number and fsyncs them, the test
injects "f2fs_io shutdown 2" in the middle, then verifies the fsynced
generation after fsck and remount:
FAIL: file 2 block 510: rolled back: gen 17 < durable gen 18
FAIL: file 2 block 561: rolled back: gen 22 < durable gen 23
FAIL: file 2 block 955: rolled back: gen 12 < durable gen 13
FAIL: file 2 block 3986: rolled back: gen 23 < durable gen 24
w/ debug logs (trace_printk) added in f2fs, the ftrace log of the
failing compressed file (ino 438) shows:
python3-346519 2267.558508: FSV SYNC-BEGIN f=2 fdatasync n=4
python3-346519 2267.558519: ino=438 FSYNC enter datasync=1 dirty=4 cpver=14c8edcb
f2fs_ckpt 2267.559876: CP end reason=4 err=0 cpver=14c8edcb
python3-346519 2267.570549: ino=438 DOWRITE OPU idx=510 old=a21bc1 new=ae4af3 hdr[f=2 b=510 g=18]
python3-346519 2267.571478: ino=438 WCACHE exit ret=0 nwritten=4 dirty_left=0
bash 2267.571810: iteration 4: injecting shutdown 2
f2fs_ckpt 2267.573403: CP start reason=4 cpver=14c8edcb dirty=1
python3-346519 2267.579576: ino=438 FSYNC wait_range ret=0 dirty_left=0 append=1
f2fs_ckpt 2267.589393: CP end reason=4 err=-5 cpver=14c8edcc cp_error=1
f2fs_ckpt 2267.589413: CP start reason=4 cpver=14c8edcc dirty=0
f2fs_ckpt 2267.589414: CP SKIPPED (sb not dirty) reason=4
f2fs_ckpt 2267.589415: CP end reason=4 err=0 cpver=14c8edcc cp_error=1
python3-346516 2267.589431: SYNC_FS sync=1 err=-5
python3-346519 2267.589503: SYNC_FS sync=1 err=0 <--- false success
python3-346519 2267.589507: ino=438 FSYNC cp_reason=2 sync_fs ret=0
python3-346519 2267.589510: ino=438 FSYNC exit ret=0
python3-346519 2267.589567: FSV SYNC-OK f=2 D=510:18,561:23,955:13,3986:24
[shutdown -> umount -> fsck -> remount, recovery from cpver 14c8edcb]
kworker 2370.028185: ino=438 READ-END idx=510 hdr[f=2 b=510 g=17]
- the checkpoint which should cover the data written at 2267.570549
failed w/ -EIO due to shutdown, and cleared SBI_IS_DIRTY.
- the next merged checkpoint request was skipped as the superblock looked
clean, and returned 0, so fdatasync() of ino 438 returned 0.
- after recovery from the last valid checkpoint (14c8edcb), block 510
has gen 17 instead of the fsynced gen 18.
Let's check cp_error before the "superblock is not dirty" check in
f2fs_write_checkpoint(), so that a pending checkpoint request returns
-EIO once cp_error is set.
Cc: stable@kernel.org
Fixes: c34f42e2cb2d ("f2fs: report error of do_checkpoint")
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/checkpoint.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 5c5b45d3c67b..819073671f3d 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1945,14 +1945,19 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
stat_cp_time(cpc, CP_TIME_LOCK);
- if (!is_sbi_flag_set(sbi, SBI_IS_DIRTY) &&
- ((cpc->reason & CP_FASTBOOT) || (cpc->reason & CP_SYNC) ||
- ((cpc->reason & CP_DISCARD) && !sbi->discard_blks)))
- goto out;
+ /*
+ * Check cp_error first: a failed checkpoint has cleared SBI_IS_DIRTY
+ * in do_checkpoint(), so a following request would be skipped below
+ * and report false success.
+ */
if (unlikely(f2fs_cp_error(sbi))) {
err = -EIO;
goto out;
}
+ if (!is_sbi_flag_set(sbi, SBI_IS_DIRTY) &&
+ ((cpc->reason & CP_FASTBOOT) || (cpc->reason & CP_SYNC) ||
+ ((cpc->reason & CP_DISCARD) && !sbi->discard_blks)))
+ goto out;
trace_f2fs_write_checkpoint(sbi->sb, cpc->reason, CP_PHASE_START_BLOCK_OPS);
--
2.49.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 4/7] f2fs: fix to truncate stale meta cache entry on in-place update
2026-10-22 12:52 [PATCH 1/7] f2fs: fix to persist i_size extended inside last block for fsync Chao Yu
2026-10-22 12:52 ` [PATCH 2/7] f2fs: fix to return -EIO from f2fs_sync_fs() if cp_error is set Chao Yu
2026-10-22 12:52 ` [PATCH 3/7] f2fs: fix to return -EIO from f2fs_write_checkpoint() once " Chao Yu
@ 2026-10-22 12:52 ` Chao Yu
2026-10-22 12:52 ` [PATCH 5/7] f2fs: fix to truncate meta cache entry in F2FS_IOC_SEC_TRIM_FILE Chao Yu
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Chao Yu @ 2026-10-22 12:52 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu, stable
From: Chao Yu <chao@kernel.org>
GC migrates data blocks of the inode which requires meta inode GC via
meta cache, and it trusts an uptodate entry w/o reading the block from
disk again, in both ra_data_block() and move_data_block().
However, in-place update paths only truncate the entry if the inode
requires meta inode GC at the time of writing:
- f2fs_inplace_write_data() checks fio->meta_gc
- f2fs_wait_on_block_writeback_range() for DIO checks
f2fs_meta_inode_gc_required()
f2fs_meta_inode_gc_required() returns true for atomic and COW files,
which is a temporary state, so an entry cached during atomic write, or
cached before the block was donated by F2FS_IOC_MOVE_RANGE, can survive
in-place update once the owner does not require meta inode GC, then GC
will migrate stale data from the entry once the owner requires meta
inode GC again, resulting in overwriting valid data.
Case #1: atomic write
- App (file F) - GC
- ioctl(F2FS_IOC_START_ATOMIC_WRITE)
: f2fs_meta_inode_gc_required(F) is true
- write F[k], k != j
- gc_data_segment() phase 3
- ra_data_block(F, j)
: cache F[j] in META[P]
- ioctl(F2FS_IOC_COMMIT_ATOMIC_WRITE)
- f2fs_commit_atomic_write()
- f2fs_down_write(i_gc_rwsem[WRITE])
- gc_data_segment() phase 4
- f2fs_down_write_trylock(i_gc_rwsem[WRITE])
: fail, skip F[j]
- __f2fs_commit_atomic_write()
: only F[k] is replaced, F[j] is still mapped to P
- f2fs_up_write(i_gc_rwsem[WRITE])
- f2fs_abort_atomic_write()
- clear_inode_flag(FI_ATOMIC_FILE)
- write F[j] w/ new data, fdatasync()
- f2fs_inplace_write_data()
: fio->meta_gc is false, META[P] becomes stale
- ioctl(F2FS_IOC_START_ATOMIC_WRITE)
- gc_data_segment() phase 3
- ra_data_block(F, j)
: META[P] is uptodate, skip reading
- gc_data_segment() phase 4
- move_data_block(F, j)
: META[P] is uptodate, skip reading
- memcpy(tentry, sentry)
- f2fs_update_data_blkaddr(newaddr)
: F[j] was rolled back to old data
Case #2: F2FS_IOC_MOVE_RANGE
- Thread A (src) - GC - Thread B (dst)
- write src[i], P is not checkpointed
- ioctl(F2FS_IOC_START_ATOMIC_WRITE)
- gc_data_segment() phase 3
- ra_data_block(src, i)
: cache src[i] in META[P]
- gc_data_segment() phase 4
: fail to lock i_gc_rwsem, skip src[i]
- ioctl(F2FS_IOC_ABORT_ATOMIC_WRITE)
- f2fs_abort_atomic_write()
- clear_inode_flag(FI_ATOMIC_FILE)
- ioctl(F2FS_IOC_MOVE_RANGE, src[i] -> dst[j])
- f2fs_move_file_range()
- __exchange_data_block()
- __read_out_blkaddrs()
: P is not checkpointed, do not invalidate it
- __clone_blkaddrs()
- f2fs_replace_block(dst[j], P)
- write dst[j] w/ new data, fdatasync()
- f2fs_inplace_write_data()
: fio->meta_gc is false, META[P] becomes stale
- ioctl(F2FS_IOC_START_ATOMIC_WRITE)
- gc_data_segment()
- ra_data_block(dst, j)
- move_data_block(dst, j)
: migrate stale data in META[P]
: dst[j] was overwritten w/ src[i]'s old data
DIO overwrite has the same issue, since f2fs_map_blocks() w/
F2FS_GET_BLOCK_DIO calls f2fs_wait_on_block_writeback_range() for the
block which will be overwritten in place.
Before commit 570e2ccc7cb3 ("f2fs: avoid reading already updated pages
during GC"), ra_data_block() always read the block from disk, then the
stale entry would be updated before move_data_block() used it.
Let's always truncate the meta cache entry in f2fs_inplace_write_data()
and f2fs_wait_on_block_writeback_range() regardless of current state of
the inode, and only wait for writeback of the cache entry when the inode
requires meta inode GC.
Cc: stable@kernel.org
Fixes: b40a2b003709 ("f2fs: use meta inode for GC of atomic file")
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/segment.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index daf6d9617d5b..c24910839583 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -4282,8 +4282,13 @@ int f2fs_inplace_write_data(struct f2fs_io_info *fio)
goto drop_bio;
}
- if (fio->meta_gc)
- f2fs_truncate_meta_caches(sbi, fio->new_blkaddr, 1);
+ /*
+ * GC may have cached this block in meta cache while its owner was in
+ * atomic write or before it was donated by F2FS_IOC_MOVE_RANGE, so
+ * truncate it regardless of fio->meta_gc to avoid that GC migrates
+ * stale data from meta cache later.
+ */
+ f2fs_truncate_meta_caches(sbi, fio->new_blkaddr, 1);
stat_inc_inplace_blocks(fio->sbi);
@@ -4471,12 +4476,16 @@ void f2fs_wait_on_block_writeback_range(struct inode *inode, block_t blkaddr,
struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
block_t i;
- if (!f2fs_meta_inode_gc_required(inode))
- return;
-
- for (i = 0; i < len; i++)
- f2fs_wait_on_block_writeback(inode, blkaddr + i);
+ if (f2fs_meta_inode_gc_required(inode)) {
+ for (i = 0; i < len; i++)
+ f2fs_wait_on_block_writeback(inode, blkaddr + i);
+ }
+ /*
+ * GC may have cached these blocks in meta cache while their owner
+ * required meta inode GC, truncate them regardless of current state
+ * of the inode, since DIO may overwrite them in place.
+ */
f2fs_truncate_meta_caches(sbi, blkaddr, len);
}
--
2.49.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 5/7] f2fs: fix to truncate meta cache entry in F2FS_IOC_SEC_TRIM_FILE
2026-10-22 12:52 [PATCH 1/7] f2fs: fix to persist i_size extended inside last block for fsync Chao Yu
` (2 preceding siblings ...)
2026-10-22 12:52 ` [PATCH 4/7] f2fs: fix to truncate stale meta cache entry on in-place update Chao Yu
@ 2026-10-22 12:52 ` Chao Yu
2026-10-22 12:52 ` [PATCH 6/7] f2fs: fix to serialize truncation of COW blocks w/ GC Chao Yu
2026-10-22 12:52 ` [PATCH 7/7] f2fs: cache: fix to initialize f2fs_writeback info at mount time Chao Yu
5 siblings, 0 replies; 7+ messages in thread
From: Chao Yu @ 2026-10-22 12:52 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu, stable
From: Chao Yu <chao@kernel.org>
F2FS_IOC_SEC_TRIM_FILE discards and/or zeroes out data blocks of the
file in place via f2fs_secure_erase(), but it never truncates the meta
cache entry of the blocks, which may be cached by GC if the inode
requires meta inode GC, e.g. encrypted inode.
GC trusts an uptodate meta cache entry w/o reading the block from disk
again, so it can migrate data which was erased to a new block address,
then the erased data will be resurrected in the file.
- App (file F) - GC
- gc_data_segment() phase 3
- ra_data_block(F, j)
: cache F[j] in META[P]
- ioctl(F2FS_IOC_SEC_TRIM_FILE)
- f2fs_down_write(i_gc_rwsem[WRITE])
- gc_data_segment() phase 4
- f2fs_down_write_trylock(i_gc_rwsem[WRITE])
: fail, skip F[j]
- f2fs_secure_erase(P)
: P was erased, META[P] becomes stale
- f2fs_up_write(i_gc_rwsem[WRITE])
- gc_data_segment() phase 3
- ra_data_block(F, j)
: META[P] is uptodate, skip reading
- gc_data_segment() phase 4
- move_data_block(F, j)
: META[P] is uptodate, skip reading
- memcpy(tentry, sentry)
- f2fs_update_data_blkaddr(newaddr)
: F[j] was resurrected w/ erased data
In addition, GC may be writing back the block w/ meta cache, we need to
wait for the writeback before erasing the block.
Let's call f2fs_wait_on_block_writeback_range() for each block before
erasing it, which waits for writeback of the cache entry, and truncates
the cache entry.
Cc: stable@kernel.org
Fixes: 9af846486d78 ("f2fs: add F2FS_IOC_SEC_TRIM_FILE ioctl")
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/file.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 27e295bdce1c..363039da3ab2 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -4661,6 +4661,13 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
goto out;
}
+ /*
+ * wait for GC's writeback and truncate the meta cache
+ * entry of the block, otherwise GC may migrate stale
+ * data in meta cache after the block was erased.
+ */
+ f2fs_wait_on_block_writeback_range(inode, blkaddr, 1);
+
cur_bdev = f2fs_target_device(sbi, blkaddr, NULL);
if (f2fs_is_multi_device(sbi)) {
int di = f2fs_target_device_index(sbi, blkaddr);
--
2.49.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 6/7] f2fs: fix to serialize truncation of COW blocks w/ GC
2026-10-22 12:52 [PATCH 1/7] f2fs: fix to persist i_size extended inside last block for fsync Chao Yu
` (3 preceding siblings ...)
2026-10-22 12:52 ` [PATCH 5/7] f2fs: fix to truncate meta cache entry in F2FS_IOC_SEC_TRIM_FILE Chao Yu
@ 2026-10-22 12:52 ` Chao Yu
2026-10-22 12:52 ` [PATCH 7/7] f2fs: cache: fix to initialize f2fs_writeback info at mount time Chao Yu
5 siblings, 0 replies; 7+ messages in thread
From: Chao Yu @ 2026-10-22 12:52 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu, stable
From: Chao Yu <chao@kernel.org>
GC migrates blocks of COW inode w/ the COW inode and its i_gc_rwsem,
however f2fs_ioc_start_atomic_write() truncates blocks of the reused
COW inode w/o holding COW inode's i_gc_rwsem, it only holds the
original inode's locks, so the truncation can race w/ GC.
Case #1: GC caches stale data of a freed block in meta cache
- GC (phase 3, cow inode C) - ioctl(F2FS_IOC_START_ATOMIC_WRITE)
- f2fs_down_write_trylock(C's i_gc_rwsem[WRITE])
- ra_data_block(C, j)
- f2fs_grab_cache_folio(orig mapping, j)
- f2fs_get_dnode_of_data(C, j)
: get P
- f2fs_put_dnode()
- f2fs_is_valid_blkaddr(P)
- f2fs_folio_wait_writeback()
- f2fs_wait_on_block_writeback()
- f2fs_ioc_start_atomic_write()
- f2fs_down_write(orig's i_gc_rwsem)
- f2fs_do_truncate_blocks(C, 0, true)
- f2fs_truncate_data_blocks_range()
- f2fs_set_data_blkaddr(NULL_ADDR)
- f2fs_invalidate_blocks(P)
- f2fs_invalidate_internal_cache(P)
: nothing is cached
- f2fs_grab_cache(META, P)
: cache P in META[P]
- f2fs_submit_cache_read(P)
: SIT of P is still valid
- update_sit_entry(P, -1)
: P was freed
- end_io: f2fs_cache_set_uptodate(META[P])
: META[P] is stale for a freed block
On superblock w/o encrypt, verity and compression features, checkpoint
does not truncate meta cache, so META[P] survives after checkpoint, then:
- checkpoint, P can be reused
- writeback of victim's atomic write
- do_write_block()
: allocate P, write victim's data to P, META[P] is not invalidated
- GC phase 3: ra_data_block(victim's cow inode, k)
: META[P] is uptodate, skip reading
- GC phase 4: move_data_block(victim's cow inode, k)
: META[P] is uptodate, skip reading
- memcpy(tentry, sentry)
- f2fs_update_data_blkaddr(newaddr)
: victim's block was overwritten w/ stale data
Case #2: GC reports a false inconsistency error
- GC (phase 3, cow inode C) - ioctl(F2FS_IOC_START_ATOMIC_WRITE)
- ra_data_block(C, j)
- f2fs_get_dnode_of_data(C, j)
: get P
- f2fs_put_dnode()
- f2fs_do_truncate_blocks(C, 0, true)
- f2fs_invalidate_blocks(P)
- update_sit_entry(P, -1)
- f2fs_is_valid_blkaddr(P, DATA_GENERIC_ENHANCE)
: or in f2fs_submit_cache_read()
- __is_bitmap_valid()
- f2fs_err("Inconsistent error blkaddr:...")
- set_sbi_flag(SBI_NEED_FSCK)
- dump_stack()
- f2fs_handle_error(ERROR_INVALID_BLKADDR)
All other truncation paths hold i_gc_rwsem[WRITE] of the truncated
inode, let's hold COW inode's i_gc_rwsem[WRITE] during truncation of
COW blocks as well, then GC will skip COW blocks due to failure of
f2fs_down_write_trylock().
Cc: stable@kernel.org
Fixes: f18d00769336 ("f2fs: use meta inode for GC of COW file")
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/file.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 363039da3ab2..3a7093a7c12e 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2445,11 +2445,20 @@ static int f2fs_ioc_start_atomic_write(struct file *filp, bool truncate)
F2FS_I(fi->cow_inode)->atomic_inode = inode;
} else {
/* Reuse the already created COW inode */
+ struct f2fs_inode_info *cfi = F2FS_I(fi->cow_inode);
+
f2fs_bug_on(sbi, get_dirty_pages(fi->cow_inode));
invalidate_mapping_pages(fi->cow_inode->i_mapping, 0, -1);
+ /*
+ * GC migrates COW blocks w/ cow inode and its i_gc_rwsem, so
+ * hold it to serialize truncation of COW blocks w/ GC.
+ */
+ f2fs_down_write_nested(&cfi->i_gc_rwsem[WRITE],
+ SINGLE_DEPTH_NESTING);
ret = f2fs_do_truncate_blocks(fi->cow_inode, 0, true);
+ f2fs_up_write(&cfi->i_gc_rwsem[WRITE]);
if (ret)
goto out_unlock;
}
--
2.49.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 7/7] f2fs: cache: fix to initialize f2fs_writeback info at mount time
2026-10-22 12:52 [PATCH 1/7] f2fs: fix to persist i_size extended inside last block for fsync Chao Yu
` (4 preceding siblings ...)
2026-10-22 12:52 ` [PATCH 6/7] f2fs: fix to serialize truncation of COW blocks w/ GC Chao Yu
@ 2026-10-22 12:52 ` Chao Yu
5 siblings, 0 replies; 7+ messages in thread
From: Chao Yu @ 2026-10-22 12:52 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu, stable
From: Chao Yu <chao@kernel.org>
f2fs_start_cache_wb_thread() initializes cache_wb_wq and f2fs_writeback
tunables every time f2fs_writeback thread is started, e.g. in remount,
which causes below race conditions:
1. cache_wb_interval, cache_wb_dirty_threshold and
cache_wb_total_threshold configured via sysfs will be reset to
default value after remount.
2. remount will reinitialize cache_wb_wq, which may be used by another
thread, e.g. f2fs_wake_up_cache_wb(), concurrently.
Let's introduce f2fs_init_cache_wb_info() to initialize them only once
in f2fs_fill_super(), then f2fs_start_cache_wb_thread() only needs to
reset cache_wb_throttled, and READ_ONCE()/WRITE_ONCE() is enough for
cache_wb_task.
Cc: stable@kernel.org
Fixes: f45afeabd5df ("f2fs: cache: introduce writeback thread")
Fixes: eeadc4807c95 ("f2fs: cache: wake up f2fs_writeback when exceeding threshold")
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/cache.c | 23 ++++++++++++++---------
fs/f2fs/cache.h | 1 +
fs/f2fs/super.c | 1 +
3 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/fs/f2fs/cache.c b/fs/f2fs/cache.c
index ddf94b5791e7..6cac7662fc1a 100644
--- a/fs/f2fs/cache.c
+++ b/fs/f2fs/cache.c
@@ -844,8 +844,7 @@ void f2fs_wake_up_cache_wb(struct f2fs_sb_info *sbi)
{
struct f2fs_cache_kthread *cache_thread = &sbi->cache_thread;
- /* pairs with smp_store_release() in f2fs_start_cache_wb_thread() */
- if (!smp_load_acquire(&cache_thread->cache_wb_task))
+ if (!READ_ONCE(cache_thread->cache_wb_task))
return;
if (!f2fs_should_wake_up_cache_wb(sbi))
@@ -855,6 +854,17 @@ void f2fs_wake_up_cache_wb(struct f2fs_sb_info *sbi)
wake_up(&cache_thread->cache_wb_wq);
}
+void f2fs_init_cache_wb_info(struct f2fs_sb_info *sbi)
+{
+ struct f2fs_cache_kthread *cache_thread = &sbi->cache_thread;
+
+ init_waitqueue_head(&cache_thread->cache_wb_wq);
+ cache_thread->cache_wb_interval = DEF_DIRTY_CACHE_TIMEOUT;
+ cache_thread->cache_wb_dirty_threshold = DEF_CACHE_WB_DIRTY_THRESH;
+ cache_thread->cache_wb_total_threshold = 0;
+ cache_thread->cache_wb_throttled = false;
+}
+
int f2fs_start_cache_wb_thread(struct f2fs_sb_info *sbi)
{
struct f2fs_cache_kthread *cache_thread = &sbi->cache_thread;
@@ -865,11 +875,7 @@ int f2fs_start_cache_wb_thread(struct f2fs_sb_info *sbi)
if (cache_thread->cache_wb_task)
return 0;
- init_waitqueue_head(&cache_thread->cache_wb_wq);
- cache_thread->cache_wb_interval = DEF_DIRTY_CACHE_TIMEOUT;
- cache_thread->cache_wb_dirty_threshold = DEF_CACHE_WB_DIRTY_THRESH;
- cache_thread->cache_wb_total_threshold = 0;
- cache_thread->cache_wb_throttled = false;
+ WRITE_ONCE(cache_thread->cache_wb_throttled, false);
snprintf(name, sizeof(name), "f2fs_writeback-%u:%u",
MAJOR(dev), MINOR(dev));
@@ -877,8 +883,7 @@ int f2fs_start_cache_wb_thread(struct f2fs_sb_info *sbi)
if (IS_ERR(task))
return PTR_ERR(task);
- /* pairs with smp_load_acquire() in f2fs_wake_up_cache_wb() */
- smp_store_release(&cache_thread->cache_wb_task, task);
+ WRITE_ONCE(cache_thread->cache_wb_task, task);
return 0;
}
diff --git a/fs/f2fs/cache.h b/fs/f2fs/cache.h
index 5408727dd321..cd017c1b188f 100644
--- a/fs/f2fs/cache.h
+++ b/fs/f2fs/cache.h
@@ -245,6 +245,7 @@ struct f2fs_cache_kthread {
bool cache_wb_throttled; /* last round made no progress */
};
+void f2fs_init_cache_wb_info(struct f2fs_sb_info *sbi);
int f2fs_start_cache_wb_thread(struct f2fs_sb_info *sbi);
void f2fs_stop_cache_wb_thread(struct f2fs_sb_info *sbi);
void f2fs_wake_up_cache_wb(struct f2fs_sb_info *sbi);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index ba2ec6123251..12a051ba51ac 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -5356,6 +5356,7 @@ static int f2fs_fill_super(struct super_block *sb, struct fs_context *fc)
f2fs_init_cache(sbi, META_CACHE(sbi), F2FS_META_CACHE);
f2fs_init_cache(sbi, NODE_CACHE(sbi), F2FS_NODE_CACHE);
f2fs_init_cache(sbi, COMPRESS_CACHE(sbi), F2FS_COMPRESS_CACHE);
+ f2fs_init_cache_wb_info(sbi);
err = f2fs_get_valid_checkpoint(sbi);
if (err) {
--
2.49.0
^ permalink raw reply [flat|nested] 7+ messages in thread