Re: [PATCH] f2fs: skip checkpoint if having a dirty segment but no prefree at BG_GC -> FG_GC

[Chao Yu <yuchao0@huawei.com>]

On 2017/2/28 7:49, Jaegeuk Kim wrote:
> On 02/27, Jaegeuk Kim wrote:
>> On 02/27, Chao Yu wrote:
>>> On 2017/2/26 3:56, Jaegeuk Kim wrote:
>>>> On 02/25, guoweichao wrote:
>>>>> Hi Jaegeuk,
>>>>>
>>>>> I regard no enough free sections as a precondition when talking about
>>>>> BG_GC -> FG_GC. I mean that for both case a) and b) I mentioned has no enough
>>>>> free sections implicitly. 
>>>>>
>>>>> On 2017/2/25 2:49, Jaegeuk Kim wrote:
>>>>>> Hi Weichao,
>>>>>>
>>>>>> On 02/25, Weichao Guo wrote:
>>>>>>> When turning to FG_GC from BG_GC, we need to write checkpoint in 2 cases:
>>>>>>> * a) BG_GC have made some progress, e.g.: some prefree segments.
>>>>>>> * b) There is no victim and no prefree segment.
>>>>>>
>>>>>> You missed
>>>>>>   * c) has_not_enough_free_secs() introduced by
>>>>>>       6e17bfbc75a5cb ("f2fs: fix to overcome inline_data floods")
>>>>> As we have enabled SSR for warm node(5b6c6be2d8 ("f2fs: use SSR for warm node as well")),
>>>>> I think inline data floods should not be a problem in most cases.
>>>>>>
>>>>>> And, Yunlong pointed that we can't find a case to avoid write_checkpoint()
>>>>>> mostly due to c) condition.
>>>>> As inline data floods is an extreme case, and there is little possibility caused panic
>>>>> for inline data floods now, there should be lots of chance to skip checkpoint. I mean
>>>>> that we can make some accurate inline data floods checking before writing checkpoint.
>>>>
>>>> For now, the safest way is our first option. So, I decided to start with doing
>>>> checkpoint due to previous inline_data flooding issue even though it's an
>>>> extreme case under SSR.
>>>>
>>>> Anyway, I agree that we need to find a way to detect when to avoid checkpoint.
>>>
>>> Hi all,
>>>
>>> I proposed a approach before, can you please check that one?
>>>
>>> https://www.mail-archive.com/linux-f2fs-devel@lists.sourceforge.net/msg03632.html
>>
>> Oh, right, let's take a look at this. ;)
> 
> Hmm, I just read this patch again, and realized it doesn't quite address the
> current issue. This patch flushes inline_data inodes in background, which does
> not guarantee this worst case. The key idea would be how to measure the space

Hmm.. Maybe we can cover worst case by moving judgment condition and flushing
operation into f2fs_balance_fs.

> we can do SSR and use it in has_not_enough_free_secs().

We need to stat usage of slack free space accurately both for data/node, right?

Thanks,

> 
> Thanks,
> 
>>
>> Thanks,
>>
>>>
>>> Thanks,
>>>
>>>>
>>>> Thanks,
>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>>>
>>>>>>> For case a), previously, we also check if there is a dirty segment for
>>>>>>> infering blocks moving in last BG_GC. But dirty segments do not always
>>>>>>> indicate that, BG_GC may just start and do not move any blocks at all.
>>>>>>> Futhermore, skipping checkpoint if there are some dirty segments but no
>>>>>>> prefree segments is OK.
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Signed-off-by: Weichao Guo <guoweichao@huawei.com>
>>>>>>> ---
>>>>>>>  fs/f2fs/gc.c | 7 ++++++-
>>>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>>>>>> index 6c996e3..30d206a 100644
>>>>>>> --- a/fs/f2fs/gc.c
>>>>>>> +++ b/fs/f2fs/gc.c
>>>>>>> @@ -958,7 +958,12 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync, bool background)
>>>>>>>  		 * enough free sections, we should flush dent/node blocks and do
>>>>>>>  		 * garbage collections.
>>>>>>>  		 */
>>>>>>> -		ret = write_checkpoint(sbi, &cpc);
>>>>>>> +		if (prefree_segments(sbi))
>>>>>>> +			ret = write_checkpoint(sbi, &cpc);
>>>>>>> +		else if (!__get_victim(sbi, &segno, gc_type) {
>>>>>>> +			segno = NULL_SEGNO;
>>>>>>> +			ret = write_checkpoint(sbi, &cpc);
>>>>>>> +		}
>>>>>>>  		if (ret)
>>>>>>>  			goto stop;
>>>>>>>  	} else if (gc_type == BG_GC && !background) {
>>>>>>> -- 
>>>>>>> 2.10.1
>>>>>>
>>>>>> .
>>>>>>
>>>>>
>>>>> Thanks,
>>>>> Weichao
>>>>
>>>> .
>>>>
>>
>> ------------------------------------------------------------------------------
>> Check out the vibrant tech community on one of the world's most
>> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot