[f2fs-dev] [PATCH v3] f2fs:fix missing space reclamation during the recovery process

[f2fs-dev] [PATCH v3] f2fs:fix missing space reclamation during the recovery process

[Wang Xiaojun via Linux-f2fs-devel]

This patch fixes missing space reclamation during the recovery process.
In the following scenarios, F2FS cannot reclaim truncated space.
case 1:
write file A, size is 1G | CP | truncate A to 1M | fsync A | SPO

case 2:
CP | write file A, size is 1G | fsync A | truncate A to 1M | fsync A |SPO

During the recovery process, F2FS will recover file A,
but the 1M-1G space cannot be reclaimed.

Fixes: d624c96fb3249 ("f2fs: add recovery routines for roll-forward")

Signed-off-by: Wang Xiaojun <wangxiaojun@vivo.com>
---
v3: Add a Fixes line.
v2: Apply Chao's suggestion from v1. No logical changes.
v1: Fix missing space reclamation during the recovery process.  
---
 fs/f2fs/f2fs.h     |  1 +
 fs/f2fs/recovery.c | 18 +++++++++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 46be7560548c..28fce59198ce 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -459,6 +459,7 @@ struct fsync_inode_entry {
 	struct inode *inode;	/* vfs inode pointer */
 	block_t blkaddr;	/* block address locating the last fsync */
 	block_t last_dentry;	/* block address locating the last dentry */
+	loff_t max_i_size;	/* previous max file size for truncate */
 };
 
 #define nats_in_cursum(jnl)		(le16_to_cpu((jnl)->n_nats))
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 4cb3a91801b4..68b62c8a74d3 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -95,6 +95,7 @@ static struct fsync_inode_entry *add_fsync_inode(struct f2fs_sb_info *sbi,
 	entry = f2fs_kmem_cache_alloc(fsync_entry_slab,
 					GFP_F2FS_ZERO, true, NULL);
 	entry->inode = inode;
+	entry->max_i_size = i_size_read(inode);
 	list_add_tail(&entry->list, head);
 
 	return entry;
@@ -796,6 +797,7 @@ static int recover_data(struct f2fs_sb_info *sbi, struct list_head *inode_list,
 	while (1) {
 		struct fsync_inode_entry *entry;
 		struct folio *folio;
+		loff_t i_size;
 
 		if (!f2fs_is_valid_blkaddr(sbi, blkaddr, META_POR))
 			break;
@@ -828,6 +830,9 @@ static int recover_data(struct f2fs_sb_info *sbi, struct list_head *inode_list,
 				break;
 			}
 			recovered_inode++;
+			i_size = i_size_read(entry->inode);
+			if (entry->max_i_size < i_size)
+				entry->max_i_size = i_size;
 		}
 		if (entry->last_dentry == blkaddr) {
 			err = recover_dentry(entry->inode, folio, dir_list);
@@ -844,8 +849,19 @@ static int recover_data(struct f2fs_sb_info *sbi, struct list_head *inode_list,
 		}
 		recovered_dnode++;
 
-		if (entry->blkaddr == blkaddr)
+		if (entry->blkaddr == blkaddr) {
+			i_size = i_size_read(entry->inode);
+			if (entry->max_i_size > i_size) {
+				err = f2fs_truncate_blocks(entry->inode,
+							i_size, false);
+				if (err) {
+					f2fs_folio_put(folio, true);
+					break;
+				}
+				f2fs_mark_inode_dirty_sync(entry->inode, true);
+			}
 			list_move_tail(&entry->list, tmp_inode_list);
+		}
 next:
 		ra_blocks = adjust_por_ra_blocks(sbi, ra_blocks, blkaddr,
 					next_blkaddr_of_node(folio));
-- 
2.34.1



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

Re: [f2fs-dev] [PATCH v3] f2fs:fix missing space reclamation during the recovery process

[Chao Yu via Linux-f2fs-devel]

On 2025/8/7 16:44, Wang Xiaojun wrote:
> This patch fixes missing space reclamation during the recovery process.
> In the following scenarios, F2FS cannot reclaim truncated space.
> case 1:
> write file A, size is 1G | CP | truncate A to 1M | fsync A | SPO
> 
> case 2:
> CP | write file A, size is 1G | fsync A | truncate A to 1M | fsync A |SPO
> 
> During the recovery process, F2FS will recover file A,
> but the 1M-1G space cannot be reclaimed.
> 
> Fixes: d624c96fb3249 ("f2fs: add recovery routines for roll-forward")
> 
> Signed-off-by: Wang Xiaojun <wangxiaojun@vivo.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

Re: [f2fs-dev] [PATCH v3] f2fs:fix missing space reclamation during the recovery process

[Chao Yu via Linux-f2fs-devel]

Xiaojun,

I just notice generic/483 will fail w/ this change, can you please take a
look?

Thanks,

On 8/7/2025 4:44 PM, Wang Xiaojun wrote:
> This patch fixes missing space reclamation during the recovery process.
> In the following scenarios, F2FS cannot reclaim truncated space.
> case 1:
> write file A, size is 1G | CP | truncate A to 1M | fsync A | SPO
> 
> case 2:
> CP | write file A, size is 1G | fsync A | truncate A to 1M | fsync A |SPO
> 
> During the recovery process, F2FS will recover file A,
> but the 1M-1G space cannot be reclaimed.
> 
> Fixes: d624c96fb3249 ("f2fs: add recovery routines for roll-forward")
> 
> Signed-off-by: Wang Xiaojun <wangxiaojun@vivo.com>
> ---
> v3: Add a Fixes line.
> v2: Apply Chao's suggestion from v1. No logical changes.
> v1: Fix missing space reclamation during the recovery process.
> ---
>   fs/f2fs/f2fs.h     |  1 +
>   fs/f2fs/recovery.c | 18 +++++++++++++++++-
>   2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 46be7560548c..28fce59198ce 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -459,6 +459,7 @@ struct fsync_inode_entry {
>   	struct inode *inode;	/* vfs inode pointer */
>   	block_t blkaddr;	/* block address locating the last fsync */
>   	block_t last_dentry;	/* block address locating the last dentry */
> +	loff_t max_i_size;	/* previous max file size for truncate */
>   };
>   
>   #define nats_in_cursum(jnl)		(le16_to_cpu((jnl)->n_nats))
> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> index 4cb3a91801b4..68b62c8a74d3 100644
> --- a/fs/f2fs/recovery.c
> +++ b/fs/f2fs/recovery.c
> @@ -95,6 +95,7 @@ static struct fsync_inode_entry *add_fsync_inode(struct f2fs_sb_info *sbi,
>   	entry = f2fs_kmem_cache_alloc(fsync_entry_slab,
>   					GFP_F2FS_ZERO, true, NULL);
>   	entry->inode = inode;
> +	entry->max_i_size = i_size_read(inode);
>   	list_add_tail(&entry->list, head);
>   
>   	return entry;
> @@ -796,6 +797,7 @@ static int recover_data(struct f2fs_sb_info *sbi, struct list_head *inode_list,
>   	while (1) {
>   		struct fsync_inode_entry *entry;
>   		struct folio *folio;
> +		loff_t i_size;
>   
>   		if (!f2fs_is_valid_blkaddr(sbi, blkaddr, META_POR))
>   			break;
> @@ -828,6 +830,9 @@ static int recover_data(struct f2fs_sb_info *sbi, struct list_head *inode_list,
>   				break;
>   			}
>   			recovered_inode++;
> +			i_size = i_size_read(entry->inode);
> +			if (entry->max_i_size < i_size)
> +				entry->max_i_size = i_size;
>   		}
>   		if (entry->last_dentry == blkaddr) {
>   			err = recover_dentry(entry->inode, folio, dir_list);
> @@ -844,8 +849,19 @@ static int recover_data(struct f2fs_sb_info *sbi, struct list_head *inode_list,
>   		}
>   		recovered_dnode++;
>   
> -		if (entry->blkaddr == blkaddr)
> +		if (entry->blkaddr == blkaddr) {
> +			i_size = i_size_read(entry->inode);
> +			if (entry->max_i_size > i_size) {
> +				err = f2fs_truncate_blocks(entry->inode,
> +							i_size, false);
> +				if (err) {
> +					f2fs_folio_put(folio, true);
> +					break;
> +				}
> +				f2fs_mark_inode_dirty_sync(entry->inode, true);
> +			}
>   			list_move_tail(&entry->list, tmp_inode_list);
> +		}
>   next:
>   		ra_blocks = adjust_por_ra_blocks(sbi, ra_blocks, blkaddr,
>   					next_blkaddr_of_node(folio));



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

Re: [f2fs-dev] [PATCH v3] f2fs:fix missing space reclamation during the recovery process

[王晓珺 via Linux-f2fs-devel]

在 2025/8/8 12:16, Chao Yu 写道:
> Xiaojun,
>
> I just notice generic/483 will fail w/ this change, can you please take a
> look?
>
> Thanks,

Hi Chao,

OK, Thanks for pointing out the issue.

>
>
> On 8/7/2025 4:44 PM, Wang Xiaojun wrote:
>> This patch fixes missing space reclamation during the recovery process.
>> In the following scenarios, F2FS cannot reclaim truncated space.
>> case 1:
>> write file A, size is 1G | CP | truncate A to 1M | fsync A | SPO
>>
>> case 2:
>> CP | write file A, size is 1G | fsync A | truncate A to 1M | fsync A 
>> |SPO
>>
>> During the recovery process, F2FS will recover file A,
>> but the 1M-1G space cannot be reclaimed.
>>
>> Fixes: d624c96fb3249 ("f2fs: add recovery routines for roll-forward")
>>
>> Signed-off-by: Wang Xiaojun <wangxiaojun@vivo.com>
>> ---
>> v3: Add a Fixes line.
>> v2: Apply Chao's suggestion from v1. No logical changes.
>> v1: Fix missing space reclamation during the recovery process.
>> ---
>>   fs/f2fs/f2fs.h     |  1 +
>>   fs/f2fs/recovery.c | 18 +++++++++++++++++-
>>   2 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 46be7560548c..28fce59198ce 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -459,6 +459,7 @@ struct fsync_inode_entry {
>>       struct inode *inode;    /* vfs inode pointer */
>>       block_t blkaddr;    /* block address locating the last fsync */
>>       block_t last_dentry;    /* block address locating the last 
>> dentry */
>> +    loff_t max_i_size;    /* previous max file size for truncate */
>>   };
>>     #define nats_in_cursum(jnl) (le16_to_cpu((jnl)->n_nats))
>> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
>> index 4cb3a91801b4..68b62c8a74d3 100644
>> --- a/fs/f2fs/recovery.c
>> +++ b/fs/f2fs/recovery.c
>> @@ -95,6 +95,7 @@ static struct fsync_inode_entry 
>> *add_fsync_inode(struct f2fs_sb_info *sbi,
>>       entry = f2fs_kmem_cache_alloc(fsync_entry_slab,
>>                       GFP_F2FS_ZERO, true, NULL);
>>       entry->inode = inode;
>> +    entry->max_i_size = i_size_read(inode);
>>       list_add_tail(&entry->list, head);
>>         return entry;
>> @@ -796,6 +797,7 @@ static int recover_data(struct f2fs_sb_info *sbi, 
>> struct list_head *inode_list,
>>       while (1) {
>>           struct fsync_inode_entry *entry;
>>           struct folio *folio;
>> +        loff_t i_size;
>>             if (!f2fs_is_valid_blkaddr(sbi, blkaddr, META_POR))
>>               break;
>> @@ -828,6 +830,9 @@ static int recover_data(struct f2fs_sb_info *sbi, 
>> struct list_head *inode_list,
>>                   break;
>>               }
>>               recovered_inode++;
>> +            i_size = i_size_read(entry->inode);
>> +            if (entry->max_i_size < i_size)
>> +                entry->max_i_size = i_size;
>>           }
>>           if (entry->last_dentry == blkaddr) {
>>               err = recover_dentry(entry->inode, folio, dir_list);
>> @@ -844,8 +849,19 @@ static int recover_data(struct f2fs_sb_info 
>> *sbi, struct list_head *inode_list,
>>           }
>>           recovered_dnode++;
>>   -        if (entry->blkaddr == blkaddr)
>> +        if (entry->blkaddr == blkaddr) {
>> +            i_size = i_size_read(entry->inode);
>> +            if (entry->max_i_size > i_size) {
>> +                err = f2fs_truncate_blocks(entry->inode,
>> +                            i_size, false);
>> +                if (err) {
>> +                    f2fs_folio_put(folio, true);
>> +                    break;
>> +                }
>> +                f2fs_mark_inode_dirty_sync(entry->inode, true);
>> +            }
>>               list_move_tail(&entry->list, tmp_inode_list);
>> +        }
>>   next:
>>           ra_blocks = adjust_por_ra_blocks(sbi, ra_blocks, blkaddr,
>>                       next_blkaddr_of_node(folio));
>


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

Re: [f2fs-dev] [PATCH v3] f2fs:fix missing space reclamation during the recovery process

[Jaegeuk Kim via Linux-f2fs-devel]

On 08/08, 王晓珺 wrote:
> 在 2025/8/8 12:16, Chao Yu 写道:
> > Xiaojun,
> >
> > I just notice generic/483 will fail w/ this change, can you please take a
> > look?
> >
> > Thanks,
> 
> Hi Chao,
> 
> OK, Thanks for pointing out the issue.

Yeah, I'm also hitting several failures in xfstests. Hence, I dropped it in
the dev-test.

> 
> >
> >
> > On 8/7/2025 4:44 PM, Wang Xiaojun wrote:
> >> This patch fixes missing space reclamation during the recovery process.
> >> In the following scenarios, F2FS cannot reclaim truncated space.
> >> case 1:
> >> write file A, size is 1G | CP | truncate A to 1M | fsync A | SPO
> >>
> >> case 2:
> >> CP | write file A, size is 1G | fsync A | truncate A to 1M | fsync A 
> >> |SPO
> >>
> >> During the recovery process, F2FS will recover file A,
> >> but the 1M-1G space cannot be reclaimed.
> >>
> >> Fixes: d624c96fb3249 ("f2fs: add recovery routines for roll-forward")
> >>
> >> Signed-off-by: Wang Xiaojun <wangxiaojun@vivo.com>
> >> ---
> >> v3: Add a Fixes line.
> >> v2: Apply Chao's suggestion from v1. No logical changes.
> >> v1: Fix missing space reclamation during the recovery process.
> >> ---
> >>   fs/f2fs/f2fs.h     |  1 +
> >>   fs/f2fs/recovery.c | 18 +++++++++++++++++-
> >>   2 files changed, 18 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> >> index 46be7560548c..28fce59198ce 100644
> >> --- a/fs/f2fs/f2fs.h
> >> +++ b/fs/f2fs/f2fs.h
> >> @@ -459,6 +459,7 @@ struct fsync_inode_entry {
> >>       struct inode *inode;    /* vfs inode pointer */
> >>       block_t blkaddr;    /* block address locating the last fsync */
> >>       block_t last_dentry;    /* block address locating the last 
> >> dentry */
> >> +    loff_t max_i_size;    /* previous max file size for truncate */
> >>   };
> >>     #define nats_in_cursum(jnl) (le16_to_cpu((jnl)->n_nats))
> >> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> >> index 4cb3a91801b4..68b62c8a74d3 100644
> >> --- a/fs/f2fs/recovery.c
> >> +++ b/fs/f2fs/recovery.c
> >> @@ -95,6 +95,7 @@ static struct fsync_inode_entry 
> >> *add_fsync_inode(struct f2fs_sb_info *sbi,
> >>       entry = f2fs_kmem_cache_alloc(fsync_entry_slab,
> >>                       GFP_F2FS_ZERO, true, NULL);
> >>       entry->inode = inode;
> >> +    entry->max_i_size = i_size_read(inode);
> >>       list_add_tail(&entry->list, head);
> >>         return entry;
> >> @@ -796,6 +797,7 @@ static int recover_data(struct f2fs_sb_info *sbi, 
> >> struct list_head *inode_list,
> >>       while (1) {
> >>           struct fsync_inode_entry *entry;
> >>           struct folio *folio;
> >> +        loff_t i_size;
> >>             if (!f2fs_is_valid_blkaddr(sbi, blkaddr, META_POR))
> >>               break;
> >> @@ -828,6 +830,9 @@ static int recover_data(struct f2fs_sb_info *sbi, 
> >> struct list_head *inode_list,
> >>                   break;
> >>               }
> >>               recovered_inode++;
> >> +            i_size = i_size_read(entry->inode);
> >> +            if (entry->max_i_size < i_size)
> >> +                entry->max_i_size = i_size;
> >>           }
> >>           if (entry->last_dentry == blkaddr) {
> >>               err = recover_dentry(entry->inode, folio, dir_list);
> >> @@ -844,8 +849,19 @@ static int recover_data(struct f2fs_sb_info 
> >> *sbi, struct list_head *inode_list,
> >>           }
> >>           recovered_dnode++;
> >>   -        if (entry->blkaddr == blkaddr)
> >> +        if (entry->blkaddr == blkaddr) {
> >> +            i_size = i_size_read(entry->inode);
> >> +            if (entry->max_i_size > i_size) {
> >> +                err = f2fs_truncate_blocks(entry->inode,
> >> +                            i_size, false);
> >> +                if (err) {
> >> +                    f2fs_folio_put(folio, true);
> >> +                    break;
> >> +                }
> >> +                f2fs_mark_inode_dirty_sync(entry->inode, true);
> >> +            }
> >>               list_move_tail(&entry->list, tmp_inode_list);
> >> +        }
> >>   next:
> >>           ra_blocks = adjust_por_ra_blocks(sbi, ra_blocks, blkaddr,
> >>                       next_blkaddr_of_node(folio));
> >
> 


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

Re: [f2fs-dev] [PATCH v3] f2fs:fix missing space reclamation during the recovery process

[王晓珺 via Linux-f2fs-devel]

Hi Chao,

This patch will cause falloc to fail in the following scenarios.
write fileA 2M | fsync | truncate 256K | falloc -k 256K 1M | fsync A | SPO
So I will fix this issue in the next version of the patch.

Thanks,

On 8/8/2025 12:16 PM, Chao Yu wrote:
> Xiaojun,
>
> I just notice generic/483 will fail w/ this change, can you please take a
> look?
>
> Thanks,
>
> On 8/7/2025 4:44 PM, Wang Xiaojun wrote:
>> This patch fixes missing space reclamation during the recovery process.
>> In the following scenarios, F2FS cannot reclaim truncated space.
>> case 1:
>> write file A, size is 1G | CP | truncate A to 1M | fsync A | SPO
>>
>> case 2:
>> CP | write file A, size is 1G | fsync A | truncate A to 1M | fsync A 
>> |SPO
>>
>> During the recovery process, F2FS will recover file A,
>> but the 1M-1G space cannot be reclaimed.
>>
>> Fixes: d624c96fb3249 ("f2fs: add recovery routines for roll-forward")
>>
>> Signed-off-by: Wang Xiaojun <wangxiaojun@vivo.com>
>> ---
>> v3: Add a Fixes line.
>> v2: Apply Chao's suggestion from v1. No logical changes.
>> v1: Fix missing space reclamation during the recovery process.
>> ---
>>   fs/f2fs/f2fs.h     |  1 +
>>   fs/f2fs/recovery.c | 18 +++++++++++++++++-
>>   2 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 46be7560548c..28fce59198ce 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -459,6 +459,7 @@ struct fsync_inode_entry {
>>       struct inode *inode;    /* vfs inode pointer */
>>       block_t blkaddr;    /* block address locating the last fsync */
>>       block_t last_dentry;    /* block address locating the last 
>> dentry */
>> +    loff_t max_i_size;    /* previous max file size for truncate */
>>   };
>>     #define nats_in_cursum(jnl) (le16_to_cpu((jnl)->n_nats))
>> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
>> index 4cb3a91801b4..68b62c8a74d3 100644
>> --- a/fs/f2fs/recovery.c
>> +++ b/fs/f2fs/recovery.c
>> @@ -95,6 +95,7 @@ static struct fsync_inode_entry 
>> *add_fsync_inode(struct f2fs_sb_info *sbi,
>>       entry = f2fs_kmem_cache_alloc(fsync_entry_slab,
>>                       GFP_F2FS_ZERO, true, NULL);
>>       entry->inode = inode;
>> +    entry->max_i_size = i_size_read(inode);
>>       list_add_tail(&entry->list, head);
>>         return entry;
>> @@ -796,6 +797,7 @@ static int recover_data(struct f2fs_sb_info *sbi, 
>> struct list_head *inode_list,
>>       while (1) {
>>           struct fsync_inode_entry *entry;
>>           struct folio *folio;
>> +        loff_t i_size;
>>             if (!f2fs_is_valid_blkaddr(sbi, blkaddr, META_POR))
>>               break;
>> @@ -828,6 +830,9 @@ static int recover_data(struct f2fs_sb_info *sbi, 
>> struct list_head *inode_list,
>>                   break;
>>               }
>>               recovered_inode++;
>> +            i_size = i_size_read(entry->inode);
>> +            if (entry->max_i_size < i_size)
>> +                entry->max_i_size = i_size;
>>           }
>>           if (entry->last_dentry == blkaddr) {
>>               err = recover_dentry(entry->inode, folio, dir_list);
>> @@ -844,8 +849,19 @@ static int recover_data(struct f2fs_sb_info 
>> *sbi, struct list_head *inode_list,
>>           }
>>           recovered_dnode++;
>>   -        if (entry->blkaddr == blkaddr)
>> +        if (entry->blkaddr == blkaddr) {
>> +            i_size = i_size_read(entry->inode);
>> +            if (entry->max_i_size > i_size) {
>> +                err = f2fs_truncate_blocks(entry->inode,
>> +                            i_size, false);
>> +                if (err) {
>> +                    f2fs_folio_put(folio, true);
>> +                    break;
>> +                }
>> +                f2fs_mark_inode_dirty_sync(entry->inode, true);
>> +            }
>>               list_move_tail(&entry->list, tmp_inode_list);
>> +        }
>>   next:
>>           ra_blocks = adjust_por_ra_blocks(sbi, ra_blocks, blkaddr,
>>                       next_blkaddr_of_node(folio));
>


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

Re: [f2fs-dev] [PATCH v3] f2fs:fix missing space reclamation during the recovery process

[王晓珺 via Linux-f2fs-devel]

Hi Chao,

The combination of truncate and falloc complicates the recovery process.

For example, in the following scenario:

write fileA 2M | fsync | truncate 256K | falloc -k 256K 1M | fsync A | SPO
The falloc (256K, 1M) need to be recovered as pre-allocated space.

But in the following scenarios, the situation is the opposite.

write fileA 2M | fsync | falloc -k 2M 10M | fsync A | truncate 256K | 
fsync A | SPO
In this scenario, the space allocated by falloc needs to be truncated.
In fact, the current f2fs cannot reclaim this part of the space.

During the recovery process, it is difficult to distinguish
between the above two types of falloc.

I think that when a file is truncated, a FI_TRUNC_FILE flag should be 
setted.
When the file is fsync'd, if the FI_TRUNC_FILE flag is present,
a checkpoint needs to be performed.

Thanks,

On 8/11/2025 10:54 AM, Wang Xiaojun wrote:
> Hi Chao,
>
> This patch will cause falloc to fail in the following scenarios.
> write fileA 2M | fsync | truncate 256K | falloc -k 256K 1M | fsync A 
> | SPO
> So I will fix this issue in the next version of the patch.
>
> Thanks,
>
> On 8/8/2025 12:16 PM, Chao Yu wrote:
>> Xiaojun,
>>
>> I just notice generic/483 will fail w/ this change, can you please 
>> take a
>> look?
>>
>> Thanks,
>>
>> On 8/7/2025 4:44 PM, Wang Xiaojun wrote:
>>> This patch fixes missing space reclamation during the recovery process.
>>> In the following scenarios, F2FS cannot reclaim truncated space.
>>> case 1:
>>> write file A, size is 1G | CP | truncate A to 1M | fsync A | SPO
>>>
>>> case 2:
>>> CP | write file A, size is 1G | fsync A | truncate A to 1M | fsync A 
>>> |SPO
>>>
>>> During the recovery process, F2FS will recover file A,
>>> but the 1M-1G space cannot be reclaimed.
>>>
>>> Fixes: d624c96fb3249 ("f2fs: add recovery routines for roll-forward")
>>>
>>> Signed-off-by: Wang Xiaojun <wangxiaojun@vivo.com>
>>> ---
>>> v3: Add a Fixes line.
>>> v2: Apply Chao's suggestion from v1. No logical changes.
>>> v1: Fix missing space reclamation during the recovery process.
>>> ---
>>>   fs/f2fs/f2fs.h     |  1 +
>>>   fs/f2fs/recovery.c | 18 +++++++++++++++++-
>>>   2 files changed, 18 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 46be7560548c..28fce59198ce 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -459,6 +459,7 @@ struct fsync_inode_entry {
>>>       struct inode *inode;    /* vfs inode pointer */
>>>       block_t blkaddr;    /* block address locating the last fsync */
>>>       block_t last_dentry;    /* block address locating the last 
>>> dentry */
>>> +    loff_t max_i_size;    /* previous max file size for truncate */
>>>   };
>>>     #define nats_in_cursum(jnl) (le16_to_cpu((jnl)->n_nats))
>>> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
>>> index 4cb3a91801b4..68b62c8a74d3 100644
>>> --- a/fs/f2fs/recovery.c
>>> +++ b/fs/f2fs/recovery.c
>>> @@ -95,6 +95,7 @@ static struct fsync_inode_entry 
>>> *add_fsync_inode(struct f2fs_sb_info *sbi,
>>>       entry = f2fs_kmem_cache_alloc(fsync_entry_slab,
>>>                       GFP_F2FS_ZERO, true, NULL);
>>>       entry->inode = inode;
>>> +    entry->max_i_size = i_size_read(inode);
>>>       list_add_tail(&entry->list, head);
>>>         return entry;
>>> @@ -796,6 +797,7 @@ static int recover_data(struct f2fs_sb_info 
>>> *sbi, struct list_head *inode_list,
>>>       while (1) {
>>>           struct fsync_inode_entry *entry;
>>>           struct folio *folio;
>>> +        loff_t i_size;
>>>             if (!f2fs_is_valid_blkaddr(sbi, blkaddr, META_POR))
>>>               break;
>>> @@ -828,6 +830,9 @@ static int recover_data(struct f2fs_sb_info 
>>> *sbi, struct list_head *inode_list,
>>>                   break;
>>>               }
>>>               recovered_inode++;
>>> +            i_size = i_size_read(entry->inode);
>>> +            if (entry->max_i_size < i_size)
>>> +                entry->max_i_size = i_size;
>>>           }
>>>           if (entry->last_dentry == blkaddr) {
>>>               err = recover_dentry(entry->inode, folio, dir_list);
>>> @@ -844,8 +849,19 @@ static int recover_data(struct f2fs_sb_info 
>>> *sbi, struct list_head *inode_list,
>>>           }
>>>           recovered_dnode++;
>>>   -        if (entry->blkaddr == blkaddr)
>>> +        if (entry->blkaddr == blkaddr) {
>>> +            i_size = i_size_read(entry->inode);
>>> +            if (entry->max_i_size > i_size) {
>>> +                err = f2fs_truncate_blocks(entry->inode,
>>> +                            i_size, false);
>>> +                if (err) {
>>> +                    f2fs_folio_put(folio, true);
>>> +                    break;
>>> +                }
>>> +                f2fs_mark_inode_dirty_sync(entry->inode, true);
>>> +            }
>>>               list_move_tail(&entry->list, tmp_inode_list);
>>> +        }
>>>   next:
>>>           ra_blocks = adjust_por_ra_blocks(sbi, ra_blocks, blkaddr,
>>>                       next_blkaddr_of_node(folio));
>>
>


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

Re: [f2fs-dev] [PATCH v3] f2fs:fix missing space reclamation during the recovery process

[Chao Yu via Linux-f2fs-devel]

On 8/18/25 11:51, 王晓珺 wrote:
> Hi Chao,
> 
> The combination of truncate and falloc complicates the recovery process.
> 
> For example, in the following scenario:
> 
> write fileA 2M | fsync | truncate 256K | falloc -k 256K 1M | fsync A | SPO
> The falloc (256K, 1M) need to be recovered as pre-allocated space.
> 
> But in the following scenarios, the situation is the opposite.
> 
> write fileA 2M | fsync | falloc -k 2M 10M | fsync A | truncate 256K | 
> fsync A | SPO
> In this scenario, the space allocated by falloc needs to be truncated.
> In fact, the current f2fs cannot reclaim this part of the space.
> 
> During the recovery process, it is difficult to distinguish
> between the above two types of falloc.
> 
> I think that when a file is truncated, a FI_TRUNC_FILE flag should be 
> setted.
> When the file is fsync'd, if the FI_TRUNC_FILE flag is present,
> a checkpoint needs to be performed.

truncate to small size and fsync would be a common case, it's better
to not trigger checkpoint during fsync.

What do you think of triggering checkpoint for fsync after falloc -k ...
instead? IMO, it shouldn't be a common case in Android.

Thanks,

> 
> Thanks,
> 
> On 8/11/2025 10:54 AM, Wang Xiaojun wrote:
>> Hi Chao,
>>
>> This patch will cause falloc to fail in the following scenarios.
>> write fileA 2M | fsync | truncate 256K | falloc -k 256K 1M | fsync A 
>> | SPO
>> So I will fix this issue in the next version of the patch.
>>
>> Thanks,
>>
>> On 8/8/2025 12:16 PM, Chao Yu wrote:
>>> Xiaojun,
>>>
>>> I just notice generic/483 will fail w/ this change, can you please 
>>> take a
>>> look?
>>>
>>> Thanks,
>>>
>>> On 8/7/2025 4:44 PM, Wang Xiaojun wrote:
>>>> This patch fixes missing space reclamation during the recovery process.
>>>> In the following scenarios, F2FS cannot reclaim truncated space.
>>>> case 1:
>>>> write file A, size is 1G | CP | truncate A to 1M | fsync A | SPO
>>>>
>>>> case 2:
>>>> CP | write file A, size is 1G | fsync A | truncate A to 1M | fsync A 
>>>> |SPO
>>>>
>>>> During the recovery process, F2FS will recover file A,
>>>> but the 1M-1G space cannot be reclaimed.
>>>>
>>>> Fixes: d624c96fb3249 ("f2fs: add recovery routines for roll-forward")
>>>>
>>>> Signed-off-by: Wang Xiaojun <wangxiaojun@vivo.com>
>>>> ---
>>>> v3: Add a Fixes line.
>>>> v2: Apply Chao's suggestion from v1. No logical changes.
>>>> v1: Fix missing space reclamation during the recovery process.
>>>> ---
>>>>   fs/f2fs/f2fs.h     |  1 +
>>>>   fs/f2fs/recovery.c | 18 +++++++++++++++++-
>>>>   2 files changed, 18 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>> index 46be7560548c..28fce59198ce 100644
>>>> --- a/fs/f2fs/f2fs.h
>>>> +++ b/fs/f2fs/f2fs.h
>>>> @@ -459,6 +459,7 @@ struct fsync_inode_entry {
>>>>       struct inode *inode;    /* vfs inode pointer */
>>>>       block_t blkaddr;    /* block address locating the last fsync */
>>>>       block_t last_dentry;    /* block address locating the last 
>>>> dentry */
>>>> +    loff_t max_i_size;    /* previous max file size for truncate */
>>>>   };
>>>>     #define nats_in_cursum(jnl) (le16_to_cpu((jnl)->n_nats))
>>>> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
>>>> index 4cb3a91801b4..68b62c8a74d3 100644
>>>> --- a/fs/f2fs/recovery.c
>>>> +++ b/fs/f2fs/recovery.c
>>>> @@ -95,6 +95,7 @@ static struct fsync_inode_entry 
>>>> *add_fsync_inode(struct f2fs_sb_info *sbi,
>>>>       entry = f2fs_kmem_cache_alloc(fsync_entry_slab,
>>>>                       GFP_F2FS_ZERO, true, NULL);
>>>>       entry->inode = inode;
>>>> +    entry->max_i_size = i_size_read(inode);
>>>>       list_add_tail(&entry->list, head);
>>>>         return entry;
>>>> @@ -796,6 +797,7 @@ static int recover_data(struct f2fs_sb_info 
>>>> *sbi, struct list_head *inode_list,
>>>>       while (1) {
>>>>           struct fsync_inode_entry *entry;
>>>>           struct folio *folio;
>>>> +        loff_t i_size;
>>>>             if (!f2fs_is_valid_blkaddr(sbi, blkaddr, META_POR))
>>>>               break;
>>>> @@ -828,6 +830,9 @@ static int recover_data(struct f2fs_sb_info 
>>>> *sbi, struct list_head *inode_list,
>>>>                   break;
>>>>               }
>>>>               recovered_inode++;
>>>> +            i_size = i_size_read(entry->inode);
>>>> +            if (entry->max_i_size < i_size)
>>>> +                entry->max_i_size = i_size;
>>>>           }
>>>>           if (entry->last_dentry == blkaddr) {
>>>>               err = recover_dentry(entry->inode, folio, dir_list);
>>>> @@ -844,8 +849,19 @@ static int recover_data(struct f2fs_sb_info 
>>>> *sbi, struct list_head *inode_list,
>>>>           }
>>>>           recovered_dnode++;
>>>>   -        if (entry->blkaddr == blkaddr)
>>>> +        if (entry->blkaddr == blkaddr) {
>>>> +            i_size = i_size_read(entry->inode);
>>>> +            if (entry->max_i_size > i_size) {
>>>> +                err = f2fs_truncate_blocks(entry->inode,
>>>> +                            i_size, false);
>>>> +                if (err) {
>>>> +                    f2fs_folio_put(folio, true);
>>>> +                    break;
>>>> +                }
>>>> +                f2fs_mark_inode_dirty_sync(entry->inode, true);
>>>> +            }
>>>>               list_move_tail(&entry->list, tmp_inode_list);
>>>> +        }
>>>>   next:
>>>>           ra_blocks = adjust_por_ra_blocks(sbi, ra_blocks, blkaddr,
>>>>                       next_blkaddr_of_node(folio));
>>>
>>
> 



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