All of lore.kernel.org
 help / color / mirror / Atom feed
* [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

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

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