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

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

[Weichao Guo via Linux-f2fs-devel]

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.

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;
-- 
2.7.4



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

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

[Chao Yu]

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?

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

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

[Weichao Guo via Linux-f2fs-devel]

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:

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.

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

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

[Chao Yu]

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
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

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

[Weichao Guo via Linux-f2fs-devel]

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

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

[Chao Yu]

On 2021/3/17 15:01, Weichao Guo wrote:
> 
> 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,

Weichao,

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

Yeah,

Thanks,

> 
> 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