[f2fs-dev] [PATCH v2] f2fs: fix data loss caused by fsync not writing any node folio

[f2fs-dev] [PATCH v2] f2fs: fix data loss caused by fsync not writing any node folio

[Yongpeng Yang]

From: Yongpeng Yang <yangyongpeng@xiaomi.com>

During fsync, the flow reaches f2fs_fsync_node_pages(), which scans all
dirty node folios of the node mapping. If there are no dirty node
folios, fsync will not write any node folio. The scenario is as follows:

create & write & fsync 'file A'                 writeback node folio
- f2fs_do_sync_file // inline inode
 - f2fs_write_inode // inode folio is dirty

                                                - f2fs_write_node_pages
                                                 - f2fs_sync_node_pages
 - f2fs_fsync_node_pages // no dirty node folios
 sudden poweroff and lost 'file A'

The root cause of the data loss is that although the inode folio is
written successfully, the corresponding node folio is not written with
the FSYNC_BIT_SHIFT mark. As a result, the recovery procedure ignores
this file.

This patch ensures that fsync writes at least one node folio with the
FSYNC_BIT_SHIFT mark for the inode, so that the recovery procedure can
properly detect and process it.

Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>
---
v2:
- Use f2fs_folio_put instead of folio_put to dec folio ref count.
---
 fs/f2fs/node.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 2fbfecaf3f7b..e14e5db1e8e6 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1982,6 +1982,22 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
 		folio_batch_release(&fbatch);
 		cond_resched();
 	}
+	/*
+	 * All dirty node folios may be written by other thread, but CP hasn't
+	 * been written yet. So, we need to flush one of inode's dnode to
+	 * recovery this inode when encounter sudden power off.
+	 */
+	if (!atomic && nwritten == 0) {
+		struct folio *ifolio;
+
+		ifolio = f2fs_get_inode_folio(sbi, inode->i_ino);
+		if (IS_ERR(ifolio))
+			return PTR_ERR(ifolio);
+		f2fs_folio_wait_writeback(ifolio, NODE, true, true);
+		folio_mark_dirty(ifolio);
+		f2fs_folio_put(ifolio, true);
+		goto retry;
+	}
 	if (atomic && !marked) {
 		f2fs_debug(sbi, "Retry to write fsync mark: ino=%u, idx=%lx",
 			   ino, last_folio->index);
-- 
2.43.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v2] f2fs: fix data loss caused by fsync not writing any node folio

[Chao Yu via Linux-f2fs-devel]

On 3/6/26 20:18, Yongpeng Yang wrote:
> From: Yongpeng Yang <yangyongpeng@xiaomi.com>
> 
> During fsync, the flow reaches f2fs_fsync_node_pages(), which scans all
> dirty node folios of the node mapping. If there are no dirty node
> folios, fsync will not write any node folio. The scenario is as follows:
> 
> create & write & fsync 'file A'                 writeback node folio
> - f2fs_do_sync_file // inline inode
>  - f2fs_write_inode // inode folio is dirty
> 
>                                                 - f2fs_write_node_pages
>                                                  - f2fs_sync_node_pages
>  - f2fs_fsync_node_pages // no dirty node folios
>  sudden poweroff and lost 'file A'
> 
> The root cause of the data loss is that although the inode folio is
> written successfully, the corresponding node folio is not written with
> the FSYNC_BIT_SHIFT mark. As a result, the recovery procedure ignores
> this file.
> 
> This patch ensures that fsync writes at least one node folio with the
> FSYNC_BIT_SHIFT mark for the inode, so that the recovery procedure can
> properly detect and process it.

I'm not sure, but I suspect that the problem should has been resolved by
commit 88bd02c9472a ("f2fs: fix conditions to remain recovery information
in f2fs_sync_file") has fixed this problem?

Thanks,

> 
> Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>
> ---
> v2:
> - Use f2fs_folio_put instead of folio_put to dec folio ref count.
> ---
>  fs/f2fs/node.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index 2fbfecaf3f7b..e14e5db1e8e6 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -1982,6 +1982,22 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
>  		folio_batch_release(&fbatch);
>  		cond_resched();
>  	}
> +	/*
> +	 * All dirty node folios may be written by other thread, but CP hasn't
> +	 * been written yet. So, we need to flush one of inode's dnode to
> +	 * recovery this inode when encounter sudden power off.
> +	 */
> +	if (!atomic && nwritten == 0) {
> +		struct folio *ifolio;
> +
> +		ifolio = f2fs_get_inode_folio(sbi, inode->i_ino);
> +		if (IS_ERR(ifolio))
> +			return PTR_ERR(ifolio);
> +		f2fs_folio_wait_writeback(ifolio, NODE, true, true);
> +		folio_mark_dirty(ifolio);
> +		f2fs_folio_put(ifolio, true);
> +		goto retry;
> +	}
>  	if (atomic && !marked) {
>  		f2fs_debug(sbi, "Retry to write fsync mark: ino=%u, idx=%lx",
>  			   ino, last_folio->index);



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v2] f2fs: fix data loss caused by fsync not writing any node folio

[Yongpeng Yang]

On 3/9/26 11:45, Chao Yu via Linux-f2fs-devel wrote:
> On 3/6/26 20:18, Yongpeng Yang wrote:
>> From: Yongpeng Yang <yangyongpeng@xiaomi.com>
>>
>> During fsync, the flow reaches f2fs_fsync_node_pages(), which scans all
>> dirty node folios of the node mapping. If there are no dirty node
>> folios, fsync will not write any node folio. The scenario is as follows:
>>
>> create & write & fsync 'file A'                 writeback node folio
>> - f2fs_do_sync_file // inline inode
>>  - f2fs_write_inode // inode folio is dirty
>>
>>                                                 - f2fs_write_node_pages
>>                                                  - f2fs_sync_node_pages
>>  - f2fs_fsync_node_pages // no dirty node folios
>>  sudden poweroff and lost 'file A'
>>
>> The root cause of the data loss is that although the inode folio is
>> written successfully, the corresponding node folio is not written with
>> the FSYNC_BIT_SHIFT mark. As a result, the recovery procedure ignores
>> this file.
>>
>> This patch ensures that fsync writes at least one node folio with the
>> FSYNC_BIT_SHIFT mark for the inode, so that the recovery procedure can
>> properly detect and process it.
> 
> I'm not sure, but I suspect that the problem should has been resolved by
> commit 88bd02c9472a ("f2fs: fix conditions to remain recovery information
> in f2fs_sync_file") has fixed this problem?

f2fs_do_sync_file() checks whether the inode needs to be marked dirty
again and written back, but there is an issue with how
f2fs_need_inode_block_update() accesses the nat_entry flag. My previous
root cause analysis was incorrect, the fix should instead address the
problem in f2fs_need_inode_block_update(). The updated change is
included in this patchset:
https://lore.kernel.org/all/20260310093611.2865092-2-monty_pavel@sina.com/

if (f2fs_need_inode_block_update(sbi, ino)) {
	f2fs_mark_inode_dirty_sync(inode, true);
	f2fs_write_inode(inode, NULL);
	goto sync_nodes;
}

Thanks
Yongpeng,

> 
> Thanks,
> 
>>
>> Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>
>> ---
>> v2:
>> - Use f2fs_folio_put instead of folio_put to dec folio ref count.
>> ---
>>  fs/f2fs/node.c | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>> index 2fbfecaf3f7b..e14e5db1e8e6 100644
>> --- a/fs/f2fs/node.c
>> +++ b/fs/f2fs/node.c
>> @@ -1982,6 +1982,22 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
>>  		folio_batch_release(&fbatch);
>>  		cond_resched();
>>  	}
>> +	/*
>> +	 * All dirty node folios may be written by other thread, but CP hasn't
>> +	 * been written yet. So, we need to flush one of inode's dnode to
>> +	 * recovery this inode when encounter sudden power off.
>> +	 */
>> +	if (!atomic && nwritten == 0) {
>> +		struct folio *ifolio;
>> +
>> +		ifolio = f2fs_get_inode_folio(sbi, inode->i_ino);
>> +		if (IS_ERR(ifolio))
>> +			return PTR_ERR(ifolio);
>> +		f2fs_folio_wait_writeback(ifolio, NODE, true, true);
>> +		folio_mark_dirty(ifolio);
>> +		f2fs_folio_put(ifolio, true);
>> +		goto retry;
>> +	}
>>  	if (atomic && !marked) {
>>  		f2fs_debug(sbi, "Retry to write fsync mark: ino=%u, idx=%lx",
>>  			   ino, last_folio->index);
> 
> 
> 
> _______________________________________________
> 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