Re: [PATCH] f2fs: enable atgc if atgc_age_threshold from user is less than elapsed_time

[Zhiguo Niu]

On Mon, May 27, 2024 at 11:49 AM Chao Yu <chao@xxxxxxxxxx> wrote:
>
> On 2024/5/20 19:36, Zhiguo Niu wrote:
> > Now atgc can be enabled based on the following conditions:
> > -ATGC mount option is set
> > -elapsed_time is more than atgc_age_threshold already
> > but these conditions are check when umounted->mounted device again.
> > If the device has not be umounted->mounted for a long time, atgc can
> > not work even the above conditions met.
>
> Zhiguo, I didn't get it, can you please explain more about this issue?
>
> Thanks,
Hi Chao,

At present, the point of atgc enale is checked during the mount
process. What I mean is that if a device has not been rebooted
(re-mounted) for a long time, even if the above two conditions are
met(ATGC mount option is set, and the device has been powered on long
enough， more than atgc default threshold ), atgc cannot be dynamically
enabled.

If the user is used to not restarting the phone after turning it on,
it will be difficult to use atgc.
thanks!
>
> >
> > It is better to enable atgc dynamiclly when user change atgc_age_threshold
> > meanwhile this vale is less than elapsed_time and ATGC mount option is on.
> >
> > Signed-off-by: Zhiguo Niu <zhiguo.niu@xxxxxxxxxx>
> > ---
> >   fs/f2fs/f2fs.h    |  1 +
> >   fs/f2fs/segment.c |  9 ++++-----
> >   fs/f2fs/sysfs.c   | 16 ++++++++++++++++
> >   3 files changed, 21 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 1974b6a..e441d2d 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -3694,6 +3694,7 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi,
> >   int f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi);
> >   void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi);
> >   void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi);
> > +int f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi);
> >   int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >                                       unsigned int start, unsigned int end);
> >   int f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type, bool force);
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 71dc8042..44923d4 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -2931,14 +2931,11 @@ static int get_atssr_segment(struct f2fs_sb_info *sbi, int type,
> >       return ret;
> >   }
> >
> > -static int __f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi)
> > +int f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi)
> >   {
> >       struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC);
> >       int ret = 0;
> >
> > -     if (!sbi->am.atgc_enabled)
> > -             return 0;
> > -
> >       f2fs_down_read(&SM_I(sbi)->curseg_lock);
> >
> >       mutex_lock(&curseg->curseg_mutex);
> > @@ -2955,7 +2952,9 @@ static int __f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi)
> >   }
> >   int f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi)
> >   {
> > -     return __f2fs_init_atgc_curseg(sbi);
> > +     if (!sbi->am.atgc_enabled)
> > +             return 0;
> > +     return f2fs_init_atgc_curseg(sbi);
> >   }
> >
> >   static void __f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi, int type)
> > diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> > index 09d3ecf..72676c5 100644
> > --- a/fs/f2fs/sysfs.c
> > +++ b/fs/f2fs/sysfs.c
> > @@ -673,6 +673,22 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
> >               return count;
> >       }
> >
> > +     if (!strcmp(a->attr.name, "atgc_age_threshold")) {
> > +             if (t < 0)
> > +                     return -EINVAL;
> > +             sbi->am.age_threshold = t;
> > +             if (sbi->am.atgc_enabled)
> > +                     return count;
> > +
> > +             if (test_opt(sbi, ATGC) &&
> > +                     le64_to_cpu(sbi->ckpt->elapsed_time) >= t) {
> > +                     if (f2fs_init_atgc_curseg(sbi))
> > +                             return -EINVAL;
> > +                     sbi->am.atgc_enabled = true;
> > +             }
> > +             return count;
> > +     }
> > +
> >       if (!strcmp(a->attr.name, "gc_segment_mode")) {
> >               if (t < MAX_GC_MODE)
> >                       sbi->gc_segment_mode = t;