* [f2fs-dev] [PATCH 0/2] f2fs: fix data consistency issue caused by nat_entry flag
@ 2026-03-10 9:36 Yongpeng Yang
2026-03-10 9:36 ` [f2fs-dev] [PATCH 1/2] f2fs: fix fsck inconsistency caused by incorrect nat_entry flag usage Yongpeng Yang
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Yongpeng Yang @ 2026-03-10 9:36 UTC (permalink / raw)
To: Chao Yu, Jaegeuk Kim; +Cc: Yongpeng Yang, Yongpeng Yang, linux-f2fs-devel
From: Yongpeng Yang <yangyongpeng@xiaomi.com>
This patchset addresses a data consistency issue caused by the lack of
mutual exclusion between checks of the HAS_FSYNCED_INODE,
IS_CHECKPOINTED, and HAS_LAST_FSYNC flags and the checkpoint write path.
In f2fs_flush_nat_entries(), after writing back the NAT block, the code
sets HAS_LAST_FSYNC and IS_CHECKPOINTED, and clears HAS_FSYNCED_INODE.
Although accesses and updates to these flags are protected by
nm_i->nat_tree_lock, observing this state in the fsync context does not
guarantee that the corresponding nat_entry state has already been
persisted to the device.
The root cause is that the semantics of these three flags are only
guaranteed after the checkpoint write completes, while the fsync path
does not follow this constraint.
This patchset ensures that accesses and updates to nat_entry flags in
the fsync path are protected by sbi->node_write.
There are still call paths not protected by sbi->node_write:
need_do_checkpoint()->f2fs_is_checkpointed_node() and
need_do_checkpoint()->f2fs_need_dentry_mark().
The flags obtained in these paths may be imprecise, but they only affect
whether a checkpoint is triggered and do not impact consistency. Adding
lock protection here would increase lock contention, so these paths are
left unchanged.
Yongpeng Yang (2):
f2fs: fix fsck inconsistency caused by incorrect nat_entry flag usage
f2fs: fix data loss caused by incorrect use of nat_entry flags
fs/f2fs/node.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
--
2.43.0
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 9+ messages in thread* [f2fs-dev] [PATCH 1/2] f2fs: fix fsck inconsistency caused by incorrect nat_entry flag usage 2026-03-10 9:36 [f2fs-dev] [PATCH 0/2] f2fs: fix data consistency issue caused by nat_entry flag Yongpeng Yang @ 2026-03-10 9:36 ` Yongpeng Yang 2026-03-11 8:39 ` Chao Yu via Linux-f2fs-devel 2026-03-11 18:16 ` Jaegeuk Kim via Linux-f2fs-devel 2026-03-10 9:36 ` [f2fs-dev] [PATCH 2/2] f2fs: fix data loss caused by incorrect use of nat_entry flag Yongpeng Yang 2026-03-24 17:32 ` [f2fs-dev] [PATCH 0/2] f2fs: fix data consistency issue caused by " patchwork-bot+f2fs--- via Linux-f2fs-devel 2 siblings, 2 replies; 9+ messages in thread From: Yongpeng Yang @ 2026-03-10 9:36 UTC (permalink / raw) To: Chao Yu, Jaegeuk Kim; +Cc: Yongpeng Yang, Yongpeng Yang, linux-f2fs-devel From: Yongpeng Yang <yangyongpeng@xiaomi.com> f2fs_need_dentry_mark() reads nat_entry flags without mutual exclusion with the checkpoint path, which can result in an incorrect inode block marking state. The scenario is as follows: create & write & fsync 'file A' write checkpoint - f2fs_do_sync_file // inline inode - f2fs_write_inode // inode folio is dirty - f2fs_write_checkpoint - f2fs_flush_merged_writes - f2fs_sync_node_pages - f2fs_fsync_node_pages // no dirty node - f2fs_need_inode_block_update // return true - f2fs_fsync_node_pages // inode dirtied - f2fs_need_dentry_mark //return true - f2fs_flush_nat_entries - f2fs_write_checkpoint end - __write_node_folio // inode with DENT_BIT_SHIFT set SPO, "fsck --dry-run" find inode has already checkpointed but still with DENT_BIT_SHIFT set The state observed by f2fs_need_dentry_mark() can differ from the state observed in __write_node_folio() after acquiring sbi->node_write. The root cause is that the semantics of IS_CHECKPOINTED and HAS_FSYNCED_INODE are only guaranteed after the checkpoint write has fully completed. This patch moves set_dentry_mark() into __write_node_folio() and protects it with the sbi->node_write lock. Fixes: 88bd02c9472a ("f2fs: fix conditions to remain recovery information in f2fs_sync_file") Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com> --- fs/f2fs/node.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 2fbfecaf3f7b..7d3b377cbc17 100644 --- a/fs/f2fs/node.c +++ b/fs/f2fs/node.c @@ -1807,7 +1807,9 @@ static bool __write_node_folio(struct folio *folio, bool atomic, bool *submitted if (IS_INODE(folio)) set_dentry_mark(folio, f2fs_need_dentry_mark(sbi, ino_of_node(folio))); - } + } else if (IS_INODE(folio) && is_fsync_dnode(folio)) + set_dentry_mark(folio, + f2fs_need_dentry_mark(sbi, ino_of_node(folio))); /* should add to global list before clearing PAGECACHE status */ if (f2fs_in_warm_node_list(folio)) { @@ -1948,9 +1950,6 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode, if (is_inode_flag_set(inode, FI_DIRTY_INODE)) f2fs_update_inode(inode, folio); - if (!atomic) - set_dentry_mark(folio, - f2fs_need_dentry_mark(sbi, ino)); } /* may be written by other thread */ if (!folio_test_dirty(folio)) -- 2.43.0 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: fix fsck inconsistency caused by incorrect nat_entry flag usage 2026-03-10 9:36 ` [f2fs-dev] [PATCH 1/2] f2fs: fix fsck inconsistency caused by incorrect nat_entry flag usage Yongpeng Yang @ 2026-03-11 8:39 ` Chao Yu via Linux-f2fs-devel 2026-03-11 18:16 ` Jaegeuk Kim via Linux-f2fs-devel 1 sibling, 0 replies; 9+ messages in thread From: Chao Yu via Linux-f2fs-devel @ 2026-03-11 8:39 UTC (permalink / raw) To: Yongpeng Yang, Jaegeuk Kim; +Cc: Yongpeng Yang, linux-f2fs-devel On 3/10/26 17:36, Yongpeng Yang wrote: > From: Yongpeng Yang <yangyongpeng@xiaomi.com> > > f2fs_need_dentry_mark() reads nat_entry flags without mutual exclusion > with the checkpoint path, which can result in an incorrect inode block > marking state. The scenario is as follows: > > create & write & fsync 'file A' write checkpoint > - f2fs_do_sync_file // inline inode > - f2fs_write_inode // inode folio is dirty > - f2fs_write_checkpoint > - f2fs_flush_merged_writes > - f2fs_sync_node_pages > - f2fs_fsync_node_pages // no dirty node > - f2fs_need_inode_block_update // return true > - f2fs_fsync_node_pages // inode dirtied > - f2fs_need_dentry_mark //return true > - f2fs_flush_nat_entries > - f2fs_write_checkpoint end > - __write_node_folio // inode with DENT_BIT_SHIFT set > SPO, "fsck --dry-run" find inode has already checkpointed but still > with DENT_BIT_SHIFT set > > The state observed by f2fs_need_dentry_mark() can differ from the state > observed in __write_node_folio() after acquiring sbi->node_write. The > root cause is that the semantics of IS_CHECKPOINTED and > HAS_FSYNCED_INODE are only guaranteed after the checkpoint write has > fully completed. > > This patch moves set_dentry_mark() into __write_node_folio() and > protects it with the sbi->node_write lock. > Cc: stable@kernel.org > Fixes: 88bd02c9472a ("f2fs: fix conditions to remain recovery information in f2fs_sync_file") > Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com> Reviewed-by: Chao Yu <chao@kernel.org> Thanks, _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: fix fsck inconsistency caused by incorrect nat_entry flag usage 2026-03-10 9:36 ` [f2fs-dev] [PATCH 1/2] f2fs: fix fsck inconsistency caused by incorrect nat_entry flag usage Yongpeng Yang 2026-03-11 8:39 ` Chao Yu via Linux-f2fs-devel @ 2026-03-11 18:16 ` Jaegeuk Kim via Linux-f2fs-devel 2026-03-12 1:13 ` Chao Yu via Linux-f2fs-devel 2026-03-12 4:01 ` Yongpeng Yang 1 sibling, 2 replies; 9+ messages in thread From: Jaegeuk Kim via Linux-f2fs-devel @ 2026-03-11 18:16 UTC (permalink / raw) To: Yongpeng Yang; +Cc: Yongpeng Yang, linux-f2fs-devel On 03/10, Yongpeng Yang wrote: > From: Yongpeng Yang <yangyongpeng@xiaomi.com> > > f2fs_need_dentry_mark() reads nat_entry flags without mutual exclusion > with the checkpoint path, which can result in an incorrect inode block > marking state. The scenario is as follows: > > create & write & fsync 'file A' write checkpoint > - f2fs_do_sync_file // inline inode > - f2fs_write_inode // inode folio is dirty > - f2fs_write_checkpoint > - f2fs_flush_merged_writes > - f2fs_sync_node_pages > - f2fs_fsync_node_pages // no dirty node > - f2fs_need_inode_block_update // return true > - f2fs_fsync_node_pages // inode dirtied > - f2fs_need_dentry_mark //return true > - f2fs_flush_nat_entries > - f2fs_write_checkpoint end > - __write_node_folio // inode with DENT_BIT_SHIFT set > SPO, "fsck --dry-run" find inode has already checkpointed but still > with DENT_BIT_SHIFT set > > The state observed by f2fs_need_dentry_mark() can differ from the state > observed in __write_node_folio() after acquiring sbi->node_write. The > root cause is that the semantics of IS_CHECKPOINTED and > HAS_FSYNCED_INODE are only guaranteed after the checkpoint write has > fully completed. > > This patch moves set_dentry_mark() into __write_node_folio() and > protects it with the sbi->node_write lock. > > Fixes: 88bd02c9472a ("f2fs: fix conditions to remain recovery information in f2fs_sync_file") > Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com> > --- > fs/f2fs/node.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c > index 2fbfecaf3f7b..7d3b377cbc17 100644 > --- a/fs/f2fs/node.c > +++ b/fs/f2fs/node.c > @@ -1807,7 +1807,9 @@ static bool __write_node_folio(struct folio *folio, bool atomic, bool *submitted > if (IS_INODE(folio)) > set_dentry_mark(folio, > f2fs_need_dentry_mark(sbi, ino_of_node(folio))); > - } > + } else if (IS_INODE(folio) && is_fsync_dnode(folio)) > + set_dentry_mark(folio, > + f2fs_need_dentry_mark(sbi, ino_of_node(folio))); Thanks, I applied with some clean-up as below. Could you please review this? - if (atomic) { - if (!test_opt(sbi, NOBARRIER)) - fio.op_flags |= REQ_PREFLUSH | REQ_FUA; - if (IS_INODE(folio)) - set_dentry_mark(folio, + if (atomic && !test_opt(sbi, NOBARRIER)) + fio.op_flags |= REQ_PREFLUSH | REQ_FUA; + + if (IS_INODE(folio) && (atomic || is_fsync_dnode(folio))) + set_dentry_mark(folio, f2fs_need_dentry_mark(sbi, ino_of_node(folio))); - } > > /* should add to global list before clearing PAGECACHE status */ > if (f2fs_in_warm_node_list(folio)) { > @@ -1948,9 +1950,6 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode, > if (is_inode_flag_set(inode, > FI_DIRTY_INODE)) > f2fs_update_inode(inode, folio); > - if (!atomic) > - set_dentry_mark(folio, > - f2fs_need_dentry_mark(sbi, ino)); > } > /* may be written by other thread */ > if (!folio_test_dirty(folio)) > -- > 2.43.0 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: fix fsck inconsistency caused by incorrect nat_entry flag usage 2026-03-11 18:16 ` Jaegeuk Kim via Linux-f2fs-devel @ 2026-03-12 1:13 ` Chao Yu via Linux-f2fs-devel 2026-03-12 4:01 ` Yongpeng Yang 1 sibling, 0 replies; 9+ messages in thread From: Chao Yu via Linux-f2fs-devel @ 2026-03-12 1:13 UTC (permalink / raw) To: Jaegeuk Kim, Yongpeng Yang; +Cc: Yongpeng Yang, linux-f2fs-devel On 2026/3/12 02:16, Jaegeuk Kim wrote: > On 03/10, Yongpeng Yang wrote: >> From: Yongpeng Yang <yangyongpeng@xiaomi.com> >> >> f2fs_need_dentry_mark() reads nat_entry flags without mutual exclusion >> with the checkpoint path, which can result in an incorrect inode block >> marking state. The scenario is as follows: >> >> create & write & fsync 'file A' write checkpoint >> - f2fs_do_sync_file // inline inode >> - f2fs_write_inode // inode folio is dirty >> - f2fs_write_checkpoint >> - f2fs_flush_merged_writes >> - f2fs_sync_node_pages >> - f2fs_fsync_node_pages // no dirty node >> - f2fs_need_inode_block_update // return true >> - f2fs_fsync_node_pages // inode dirtied >> - f2fs_need_dentry_mark //return true >> - f2fs_flush_nat_entries >> - f2fs_write_checkpoint end >> - __write_node_folio // inode with DENT_BIT_SHIFT set >> SPO, "fsck --dry-run" find inode has already checkpointed but still >> with DENT_BIT_SHIFT set >> >> The state observed by f2fs_need_dentry_mark() can differ from the state >> observed in __write_node_folio() after acquiring sbi->node_write. The >> root cause is that the semantics of IS_CHECKPOINTED and >> HAS_FSYNCED_INODE are only guaranteed after the checkpoint write has >> fully completed. >> >> This patch moves set_dentry_mark() into __write_node_folio() and >> protects it with the sbi->node_write lock. >> >> Fixes: 88bd02c9472a ("f2fs: fix conditions to remain recovery information in f2fs_sync_file") >> Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com> >> --- >> fs/f2fs/node.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c >> index 2fbfecaf3f7b..7d3b377cbc17 100644 >> --- a/fs/f2fs/node.c >> +++ b/fs/f2fs/node.c >> @@ -1807,7 +1807,9 @@ static bool __write_node_folio(struct folio *folio, bool atomic, bool *submitted >> if (IS_INODE(folio)) >> set_dentry_mark(folio, >> f2fs_need_dentry_mark(sbi, ino_of_node(folio))); >> - } >> + } else if (IS_INODE(folio) && is_fsync_dnode(folio)) >> + set_dentry_mark(folio, >> + f2fs_need_dentry_mark(sbi, ino_of_node(folio))); > > Thanks, I applied with some clean-up as below. Could you please review this? > > - if (atomic) { > - if (!test_opt(sbi, NOBARRIER)) > - fio.op_flags |= REQ_PREFLUSH | REQ_FUA; > - if (IS_INODE(folio)) > - set_dentry_mark(folio, > + if (atomic && !test_opt(sbi, NOBARRIER)) > + fio.op_flags |= REQ_PREFLUSH | REQ_FUA; > + > + if (IS_INODE(folio) && (atomic || is_fsync_dnode(folio))) > + set_dentry_mark(folio, > f2fs_need_dentry_mark(sbi, ino_of_node(folio))); > - } Looks good. Thanks, > > >> >> /* should add to global list before clearing PAGECACHE status */ >> if (f2fs_in_warm_node_list(folio)) { >> @@ -1948,9 +1950,6 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode, >> if (is_inode_flag_set(inode, >> FI_DIRTY_INODE)) >> f2fs_update_inode(inode, folio); >> - if (!atomic) >> - set_dentry_mark(folio, >> - f2fs_need_dentry_mark(sbi, ino)); >> } >> /* may be written by other thread */ >> if (!folio_test_dirty(folio)) >> -- >> 2.43.0 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: fix fsck inconsistency caused by incorrect nat_entry flag usage 2026-03-11 18:16 ` Jaegeuk Kim via Linux-f2fs-devel 2026-03-12 1:13 ` Chao Yu via Linux-f2fs-devel @ 2026-03-12 4:01 ` Yongpeng Yang 1 sibling, 0 replies; 9+ messages in thread From: Yongpeng Yang @ 2026-03-12 4:01 UTC (permalink / raw) To: Jaegeuk Kim; +Cc: Yongpeng Yang, linux-f2fs-devel On 3/12/26 02:16, Jaegeuk Kim via Linux-f2fs-devel wrote: > On 03/10, Yongpeng Yang wrote: >> From: Yongpeng Yang <yangyongpeng@xiaomi.com> >> >> f2fs_need_dentry_mark() reads nat_entry flags without mutual exclusion >> with the checkpoint path, which can result in an incorrect inode block >> marking state. The scenario is as follows: >> >> create & write & fsync 'file A' write checkpoint >> - f2fs_do_sync_file // inline inode >> - f2fs_write_inode // inode folio is dirty >> - f2fs_write_checkpoint >> - f2fs_flush_merged_writes >> - f2fs_sync_node_pages >> - f2fs_fsync_node_pages // no dirty node >> - f2fs_need_inode_block_update // return true >> - f2fs_fsync_node_pages // inode dirtied >> - f2fs_need_dentry_mark //return true >> - f2fs_flush_nat_entries >> - f2fs_write_checkpoint end >> - __write_node_folio // inode with DENT_BIT_SHIFT set >> SPO, "fsck --dry-run" find inode has already checkpointed but still >> with DENT_BIT_SHIFT set >> >> The state observed by f2fs_need_dentry_mark() can differ from the state >> observed in __write_node_folio() after acquiring sbi->node_write. The >> root cause is that the semantics of IS_CHECKPOINTED and >> HAS_FSYNCED_INODE are only guaranteed after the checkpoint write has >> fully completed. >> >> This patch moves set_dentry_mark() into __write_node_folio() and >> protects it with the sbi->node_write lock. >> >> Fixes: 88bd02c9472a ("f2fs: fix conditions to remain recovery information in f2fs_sync_file") >> Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com> >> --- >> fs/f2fs/node.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c >> index 2fbfecaf3f7b..7d3b377cbc17 100644 >> --- a/fs/f2fs/node.c >> +++ b/fs/f2fs/node.c >> @@ -1807,7 +1807,9 @@ static bool __write_node_folio(struct folio *folio, bool atomic, bool *submitted >> if (IS_INODE(folio)) >> set_dentry_mark(folio, >> f2fs_need_dentry_mark(sbi, ino_of_node(folio))); >> - } >> + } else if (IS_INODE(folio) && is_fsync_dnode(folio)) >> + set_dentry_mark(folio, >> + f2fs_need_dentry_mark(sbi, ino_of_node(folio))); > > Thanks, I applied with some clean-up as below. Could you please review this? > > - if (atomic) { > - if (!test_opt(sbi, NOBARRIER)) > - fio.op_flags |= REQ_PREFLUSH | REQ_FUA; > - if (IS_INODE(folio)) > - set_dentry_mark(folio, > + if (atomic && !test_opt(sbi, NOBARRIER)) > + fio.op_flags |= REQ_PREFLUSH | REQ_FUA; > + > + if (IS_INODE(folio) && (atomic || is_fsync_dnode(folio))) > + set_dentry_mark(folio, > f2fs_need_dentry_mark(sbi, ino_of_node(folio))); > - } > > This looks good to me. Thanks Yongpeng, >> >> /* should add to global list before clearing PAGECACHE status */ >> if (f2fs_in_warm_node_list(folio)) { >> @@ -1948,9 +1950,6 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode, >> if (is_inode_flag_set(inode, >> FI_DIRTY_INODE)) >> f2fs_update_inode(inode, folio); >> - if (!atomic) >> - set_dentry_mark(folio, >> - f2fs_need_dentry_mark(sbi, ino)); >> } >> /* may be written by other thread */ >> if (!folio_test_dirty(folio)) >> -- >> 2.43.0 > > > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* [f2fs-dev] [PATCH 2/2] f2fs: fix data loss caused by incorrect use of nat_entry flag 2026-03-10 9:36 [f2fs-dev] [PATCH 0/2] f2fs: fix data consistency issue caused by nat_entry flag Yongpeng Yang 2026-03-10 9:36 ` [f2fs-dev] [PATCH 1/2] f2fs: fix fsck inconsistency caused by incorrect nat_entry flag usage Yongpeng Yang @ 2026-03-10 9:36 ` Yongpeng Yang 2026-03-11 8:48 ` Chao Yu via Linux-f2fs-devel 2026-03-24 17:32 ` [f2fs-dev] [PATCH 0/2] f2fs: fix data consistency issue caused by " patchwork-bot+f2fs--- via Linux-f2fs-devel 2 siblings, 1 reply; 9+ messages in thread From: Yongpeng Yang @ 2026-03-10 9:36 UTC (permalink / raw) To: Chao Yu, Jaegeuk Kim; +Cc: Yongpeng Yang, Yongpeng Yang, linux-f2fs-devel From: Yongpeng Yang <yangyongpeng@xiaomi.com> Data loss can occur when fsync is performed on a newly created file (before any checkpoint has been written) concurrently with a checkpoint operation. The scenario is as follows: create & write & fsync 'file A' write checkpoint - f2fs_do_sync_file // inline inode - f2fs_write_inode // inode folio is dirty - f2fs_write_checkpoint - f2fs_flush_merged_writes - f2fs_sync_node_pages - f2fs_flush_nat_entries - f2fs_fsync_node_pages // no dirty node - f2fs_need_inode_block_update // return false SPO and lost 'file A' f2fs_flush_nat_entries() sets the IS_CHECKPOINTED and HAS_LAST_FSYNC flags for the nat_entry, but this does not mean that the checkpoint has actually completed successfully. However, f2fs_need_inode_block_update() checks these flags and incorrectly assumes that the checkpoint has finished. The root cause is that the semantics of IS_CHECKPOINTED and HAS_LAST_FSYNC are only guaranteed after the checkpoint write fully completes. This patch modifies f2fs_need_inode_block_update() to acquire the sbi->node_write lock before reading the nat_entry flags, ensuring that once IS_CHECKPOINTED and HAS_LAST_FSYNC are observed to be set, the checkpoint operation has already completed. Fixes: e05df3b115e7 ("f2fs: add node operations") Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com> --- fs/f2fs/node.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 7d3b377cbc17..7153d780293b 100644 --- a/fs/f2fs/node.c +++ b/fs/f2fs/node.c @@ -427,7 +427,9 @@ bool f2fs_need_inode_block_update(struct f2fs_sb_info *sbi, nid_t ino) struct f2fs_nm_info *nm_i = NM_I(sbi); struct nat_entry *e; bool need_update = true; + struct f2fs_lock_context lc; + f2fs_down_read_trace(&sbi->node_write, &lc); f2fs_down_read(&nm_i->nat_tree_lock); e = __lookup_nat_cache(nm_i, ino, false); if (e && get_nat_flag(e, HAS_LAST_FSYNC) && @@ -435,6 +437,7 @@ bool f2fs_need_inode_block_update(struct f2fs_sb_info *sbi, nid_t ino) get_nat_flag(e, HAS_FSYNCED_INODE))) need_update = false; f2fs_up_read(&nm_i->nat_tree_lock); + f2fs_up_read_trace(&sbi->node_write, &lc); return need_update; } -- 2.43.0 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [f2fs-dev] [PATCH 2/2] f2fs: fix data loss caused by incorrect use of nat_entry flag 2026-03-10 9:36 ` [f2fs-dev] [PATCH 2/2] f2fs: fix data loss caused by incorrect use of nat_entry flag Yongpeng Yang @ 2026-03-11 8:48 ` Chao Yu via Linux-f2fs-devel 0 siblings, 0 replies; 9+ messages in thread From: Chao Yu via Linux-f2fs-devel @ 2026-03-11 8:48 UTC (permalink / raw) To: Yongpeng Yang, Jaegeuk Kim; +Cc: Yongpeng Yang, linux-f2fs-devel On 3/10/26 17:36, Yongpeng Yang wrote: > From: Yongpeng Yang <yangyongpeng@xiaomi.com> > > Data loss can occur when fsync is performed on a newly created file > (before any checkpoint has been written) concurrently with a checkpoint > operation. The scenario is as follows: > > create & write & fsync 'file A' write checkpoint > - f2fs_do_sync_file // inline inode > - f2fs_write_inode // inode folio is dirty > - f2fs_write_checkpoint > - f2fs_flush_merged_writes > - f2fs_sync_node_pages > - f2fs_flush_nat_entries > - f2fs_fsync_node_pages // no dirty node > - f2fs_need_inode_block_update // return false > SPO and lost 'file A' > > f2fs_flush_nat_entries() sets the IS_CHECKPOINTED and HAS_LAST_FSYNC > flags for the nat_entry, but this does not mean that the checkpoint has > actually completed successfully. However, f2fs_need_inode_block_update() > checks these flags and incorrectly assumes that the checkpoint has > finished. > > The root cause is that the semantics of IS_CHECKPOINTED and > HAS_LAST_FSYNC are only guaranteed after the checkpoint write fully > completes. > > This patch modifies f2fs_need_inode_block_update() to acquire the > sbi->node_write lock before reading the nat_entry flags, ensuring that > once IS_CHECKPOINTED and HAS_LAST_FSYNC are observed to be set, the > checkpoint operation has already completed. > > Fixes: e05df3b115e7 ("f2fs: add node operations") > Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com> Reviewed-by: Chao Yu <chao@kernel.org> Thanks, _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [f2fs-dev] [PATCH 0/2] f2fs: fix data consistency issue caused by nat_entry flag 2026-03-10 9:36 [f2fs-dev] [PATCH 0/2] f2fs: fix data consistency issue caused by nat_entry flag Yongpeng Yang 2026-03-10 9:36 ` [f2fs-dev] [PATCH 1/2] f2fs: fix fsck inconsistency caused by incorrect nat_entry flag usage Yongpeng Yang 2026-03-10 9:36 ` [f2fs-dev] [PATCH 2/2] f2fs: fix data loss caused by incorrect use of nat_entry flag Yongpeng Yang @ 2026-03-24 17:32 ` patchwork-bot+f2fs--- via Linux-f2fs-devel 2 siblings, 0 replies; 9+ messages in thread From: patchwork-bot+f2fs--- via Linux-f2fs-devel @ 2026-03-24 17:32 UTC (permalink / raw) To: Yongpeng Yang; +Cc: jaegeuk, yangyongpeng, linux-f2fs-devel Hello: This series was applied to jaegeuk/f2fs.git (dev) by Jaegeuk Kim <jaegeuk@kernel.org>: On Tue, 10 Mar 2026 17:36:10 +0800 you wrote: > From: Yongpeng Yang <yangyongpeng@xiaomi.com> > > This patchset addresses a data consistency issue caused by the lack of > mutual exclusion between checks of the HAS_FSYNCED_INODE, > IS_CHECKPOINTED, and HAS_LAST_FSYNC flags and the checkpoint write path. > > In f2fs_flush_nat_entries(), after writing back the NAT block, the code > sets HAS_LAST_FSYNC and IS_CHECKPOINTED, and clears HAS_FSYNCED_INODE. > Although accesses and updates to these flags are protected by > nm_i->nat_tree_lock, observing this state in the fsync context does not > guarantee that the corresponding nat_entry state has already been > persisted to the device. > > [...] Here is the summary with links: - [f2fs-dev,1/2] f2fs: fix fsck inconsistency caused by incorrect nat_entry flag usage (no matching commit) - [f2fs-dev,2/2] f2fs: fix data loss caused by incorrect use of nat_entry flag https://git.kernel.org/jaegeuk/f2fs/c/06d5ce7683fe You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-03-24 17:32 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-10 9:36 [f2fs-dev] [PATCH 0/2] f2fs: fix data consistency issue caused by nat_entry flag Yongpeng Yang 2026-03-10 9:36 ` [f2fs-dev] [PATCH 1/2] f2fs: fix fsck inconsistency caused by incorrect nat_entry flag usage Yongpeng Yang 2026-03-11 8:39 ` Chao Yu via Linux-f2fs-devel 2026-03-11 18:16 ` Jaegeuk Kim via Linux-f2fs-devel 2026-03-12 1:13 ` Chao Yu via Linux-f2fs-devel 2026-03-12 4:01 ` Yongpeng Yang 2026-03-10 9:36 ` [f2fs-dev] [PATCH 2/2] f2fs: fix data loss caused by incorrect use of nat_entry flag Yongpeng Yang 2026-03-11 8:48 ` Chao Yu via Linux-f2fs-devel 2026-03-24 17:32 ` [f2fs-dev] [PATCH 0/2] f2fs: fix data consistency issue caused by " patchwork-bot+f2fs--- via Linux-f2fs-devel
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.