[f2fs-dev] [PATCH V2] f2fs: The GC triggered by ioctl also needs to mark the segno as victim

[f2fs-dev] [PATCH V2] f2fs: The GC triggered by ioctl also needs to mark the segno as victim

[Yongpeng Yang]

In SSR mode, the segment selected for allocation might be the same as
the target segment of the GC triggered by ioctl, resulting in the GC
moving the CURSEG_I(sbi, type)->segno.
Thread A				Thread B or Thread A
- f2fs_ioc_gc_range
 - __f2fs_ioc_gc_range(.victim_segno=segno#N)
  - f2fs_gc
   - __get_victim
    - f2fs_get_victim
    : segno#N is valid, return segno#N as source segment of GC
					- f2fs_allocate_data_block
						- need_new_seg
						- get_ssr_segment
						- f2fs_get_victim
						: get segno #N as destination segment
						- change_curseg

Fixes: e066b83c9b40 ("f2fs: add ioctl to flush data from faster device to cold area")
Signed-off-by: Yongpeng Yang <yangyongpeng1@oppo.com>
---
V2: Include an explanation of concurrent scenarios.
---
 fs/f2fs/gc.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 3e1b6d2ff3a7..8029369bb71d 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -806,11 +806,14 @@ int f2fs_get_victim(struct f2fs_sb_info *sbi, unsigned int *result,
 			goto out;
 		}
 
-		if (sec_usage_check(sbi, GET_SEC_FROM_SEG(sbi, *result)))
+		if (sec_usage_check(sbi, GET_SEC_FROM_SEG(sbi, *result))) {
 			ret = -EBUSY;
-		else
-			p.min_segno = *result;
-		goto out;
+			goto out;
+		}
+		if (gc_type == FG_GC)
+			clear_bit(GET_SEC_FROM_SEG(sbi, *result), dirty_i->victim_secmap);
+		p.min_segno = *result;
+		goto got_result;
 	}
 
 	ret = -ENODATA;
-- 
2.34.1

Re: [f2fs-dev] [PATCH V2] f2fs: The GC triggered by ioctl also needs to mark the segno as victim

[Chao Yu]

On 2024/12/4 11:31, Yongpeng Yang wrote:
> In SSR mode, the segment selected for allocation might be the same as
> the target segment of the GC triggered by ioctl, resulting in the GC
> moving the CURSEG_I(sbi, type)->segno.
> Thread A				Thread B or Thread A
> - f2fs_ioc_gc_range
>   - __f2fs_ioc_gc_range(.victim_segno=segno#N)
>    - f2fs_gc
>     - __get_victim
>      - f2fs_get_victim
>      : segno#N is valid, return segno#N as source segment of GC
> 					- f2fs_allocate_data_block
> 						- need_new_seg
> 						- get_ssr_segment
> 						- f2fs_get_victim
> 						: get segno #N as destination segment
> 						- change_curseg
> 
> Fixes: e066b83c9b40 ("f2fs: add ioctl to flush data from faster device to cold area")
> Signed-off-by: Yongpeng Yang <yangyongpeng1@oppo.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

Re: [f2fs-dev] [PATCH V2] f2fs: The GC triggered by ioctl also needs to mark the segno as victim

[patchwork-bot+f2fs]

Hello:

This patch was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:

On Wed,  4 Dec 2024 11:31:13 +0800 you wrote:
> In SSR mode, the segment selected for allocation might be the same as
> the target segment of the GC triggered by ioctl, resulting in the GC
> moving the CURSEG_I(sbi, type)->segno.
> Thread A				Thread B or Thread A
> - f2fs_ioc_gc_range
>  - __f2fs_ioc_gc_range(.victim_segno=segno#N)
>   - f2fs_gc
>    - __get_victim
>     - f2fs_get_victim
>     : segno#N is valid, return segno#N as source segment of GC
> 					- f2fs_allocate_data_block
> 						- need_new_seg
> 						- get_ssr_segment
> 						- f2fs_get_victim
> 						: get segno #N as destination segment
> 						- change_curseg
> 
> [...]

Here is the summary with links:
  - [f2fs-dev,V2] f2fs: The GC triggered by ioctl also needs to mark the segno as victim
    https://git.kernel.org/jaegeuk/f2fs/c/e9a844f6e487

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html