Re: [f2fs-dev] [RFC PATCH] f2fs: do not use CURSEG_ALL_DATA_ATGC in foreground or urgent GC

[Weichao Guo via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>]

On 2021/3/17 14:41, Chao Yu wrote:
> On 2021/3/17 11:44, Weichao Guo wrote:
>>
>> On 2021/3/17 10:40, Chao Yu wrote:
>>> On 2021/3/16 21:15, Weichao Guo via Linux-f2fs-devel wrote:
>>>> CURSEG_ALL_DATA_ATGC is introduced by age threshold based GC for 
>>>> better
>>>> hot/cold data seperation and avoiding free segment cost. However, LFS
>>>> write mode is preferred in the scenario of foreground or urgent GC,
>>>> which
>>>> should be finished ASAP. So, let's only use CURSEG_ALL_DATA_ATGC when
>>>> not
>>>> urgent in background GC.
>>>
>>> Weichao,
>>>
>>> select_gc_type()
>>>
>>>      case GC_URGENT_HIGH:
>>>          gc_mode = GC_GREEDY;
>>>
>>> We have selected 'greedy' algorithm rather than 'atgc' algorithm for 
>>> high
>>> urgent gc, did you mean we should use 'greedy' for low urgent gc as 
>>> well?
>>
>> Hi Chao,
>>
>> I mean in the target victim selection, we use AT_SSR mode if only
>> atgc_enabled
>>
>> regardless of the gc mode. My point is:
>
> Yup,
>
>>
>> 1. AT_SSR is good, but LFS mode is perferred in the scenario of
>> foreground GC (at least)
>>
>> & high urgent background GC, which should be finished ASAP.
>>
>> 2. For other GC scenarios using 'greedy' algorithm, I think either
>> AT_SSR or LFS mode is OK.
>>
>> Because the source victim has few valid blocks & may be not cold.
>>
>> 3. And for GC scenarios using CB/ATGC algorithm, I think AT_SSR mode is
>> better.
>
> Alright, it's fine to me to change policy like this.
>
> One more concern is, if user trigger GC_URGENT_LOW, we don't need to
> force GC to use non-AT_SSR allocation, right? as now GC_URGENT_LOW only

Chao,

I agree with you, so using non-AT_SSR in foreground GC & high urgent 
background GC?

I will send  a new version of this patch later.

Thanks,

Weichao

> controls IO awareness of discard/GC thread.
>
> +       if (sbi->gc_mode == GC_URGENT_LOW &&
> +                       (type == DISCARD_TIME || type == GC_TIME))
> +               return true;
>
> Thanks,
>
>>
>> Thanks,
>>
>> Weichao
>>
>>>
>>> Thanks,
>>>
>>>>
>>>> Signed-off-by: Weichao Guo <guoweichao@oppo.com>
>>>> Signed-off-by: Huang Jianan <huangjianan@oppo.com>
>>>> ---
>>>>    fs/f2fs/gc.c      | 3 ++-
>>>>    fs/f2fs/segment.c | 4 +++-
>>>>    2 files changed, 5 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>>> index 86ba8ed..ee42f6e 100644
>>>> --- a/fs/f2fs/gc.c
>>>> +++ b/fs/f2fs/gc.c
>>>> @@ -1120,7 +1120,8 @@ static int move_data_block(struct inode *inode,
>>>> block_t bidx,
>>>>        block_t newaddr;
>>>>        int err = 0;
>>>>        bool lfs_mode = f2fs_lfs_mode(fio.sbi);
>>>> -    int type = fio.sbi->am.atgc_enabled ?
>>>> +    int type = fio.sbi->am.atgc_enabled && gc_type == BG_GC &&
>>>> +                fio.sbi->gc_mode < GC_URGENT_HIGH ?
>>>>                    CURSEG_ALL_DATA_ATGC : CURSEG_COLD_DATA;
>>>>          /* do not read out */
>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>>> index 32b915e..cbfb0d1 100644
>>>> --- a/fs/f2fs/segment.c
>>>> +++ b/fs/f2fs/segment.c
>>>> @@ -3264,7 +3264,9 @@ static int __get_segment_type_6(struct
>>>> f2fs_io_info *fio)
>>>>            struct inode *inode = fio->page->mapping->host;
>>>>              if (is_cold_data(fio->page)) {
>>>> -            if (fio->sbi->am.atgc_enabled)
>>>> +            if (fio->sbi->am.atgc_enabled &&
>>>> +                fio->io_type == FS_DATA_IO &&
>>>> +                fio->sbi->gc_mode < GC_URGENT_HIGH)
>>>>                    return CURSEG_ALL_DATA_ATGC;
>>>>                else
>>>>                    return CURSEG_COLD_DATA;
>>>>
>> .
>>


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