On 05/12, Chao Yu wrote:
> On 2022/5/12 0:47, Jaegeuk Kim wrote:
> > > > > > @@ -147,6 +147,7 @@ static int gc_thread_func(void *data)
> > > > > >                     gc_control.init_gc_type = sync_mode ? FG_GC : 
> > > > > > BG_GC;
> > > > > >                     gc_control.no_bg_gc = foreground;
> > > > > > +           gc_control.nr_free_secs = foreground ? 1 : 0;
> 
> [snip]
> 
> > 
> > I mean gc_control->nr_free_secs should be 0.
> 
> [snip]
> 
> > > > > > @@ -528,7 +528,8 @@ void f2fs_balance_fs(struct f2fs_sb_info *sbi, 
> > > > > > bool need)
> > > > > >                                     .init_gc_type = BG_GC,
> > > > > >                                     .no_bg_gc = true,
> > > > > >                                     .should_migrate_blocks = false,
> > > > > > -                           .err_gc_skipped = false };
> > > > > > +                           .err_gc_skipped = false,
> > > > > > +                           .nr_free_secs = 1 };
> 
> Oh, so, in above two paths, when .nr_free_secs is 1, no_bg_gc should be true
> to keep skipping BG_GC flow.
> 
> How about adding a check condition in f2fs_gc() to avoid invalid argument
> usage in future?

Sent out v2. Could you please check?

> 
> From: Chao Yu <[email protected]>
> 
> ---
>  fs/f2fs/gc.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 385282017317..a98276fd3cc1 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -1799,10 +1799,19 @@ int f2fs_gc(struct f2fs_sb_info *sbi, struct 
> f2fs_gc_control *gc_control)
>                       gc_type = FG_GC;
>       }
> 
> -     /* f2fs_balance_fs doesn't need to do BG_GC in critical path. */
> -     if (gc_type == BG_GC && gc_control->no_bg_gc) {
> -             ret = -EINVAL;
> -             goto stop;
> +     if (gc_type == BG_GC) {
> +             /* f2fs_balance_fs doesn't need to do BG_GC in critical path. */
> +             if (gc_control->no_bg_gc) {
> +                     ret = -EINVAL;
> +                     goto stop;
> +             }
> +             /*
> +              * BG_GC never guarantee that blocks are migrated synchronously.
> +              */
> +             if (gc_control->nr_free_secs) {
> +                     ret = -EINVAL;
> +                     goto stop;
> +             }
>       }
>  retry:
>       ret = __get_victim(sbi, &segno, gc_type);
> -- 
> 2.25.1
> 
> 
> 
> Thanks,


_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to