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

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

[Jaegeuk Kim]

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

And, Yunlong pointed that we can't find a case to avoid write_checkpoint()
mostly due to c) condition.

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

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

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

[Weichao Guo]

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.

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


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

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

[guoweichao]

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

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

[Jaegeuk Kim]

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.

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

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

[Chao Yu]

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

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

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

[Jaegeuk Kim]

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. ;)

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

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

[Jaegeuk Kim]

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
we can do SSR and use it in has_not_enough_free_secs().

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

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

[Chao Yu]

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

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

[Jaegeuk Kim]

On 02/28, Chao Yu wrote:
> 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?

Yup.
Thanks,

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