[PATCH] f2fs: avoid unnecessary fg_gc

[PATCH] f2fs: avoid unnecessary fg_gc

[Hou Pengyang]

Under scenerio with large number of dirty nodes, and these nodes are flushed
in SSR mode during cp. enough free segemts now, no need to do fggc.

Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
---
 fs/f2fs/gc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 6c996e3..41bdfb7 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -959,7 +959,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync, bool background)
 		 * garbage collections.
 		 */
 		ret = write_checkpoint(sbi, &cpc);
-		if (ret)
+		if (ret || !has_not_enough_free_secs(sbi, sec_freed, 0))
 			goto stop;
 	} else if (gc_type == BG_GC && !background) {
 		/* f2fs_balance_fs doesn't need to do BG_GC in critical path. */
-- 
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: avoid unnecessary fg_gc

[Chao Yu]

On 2017/2/24 18:01, Hou Pengyang wrote:
> Under scenerio with large number of dirty nodes, and these nodes are flushed
> in SSR mode during cp. enough free segemts now, no need to do fggc.

We'd better break out of GC flow once we encounter cp error, so additional
condition judgment is not needed.

Thanks,

> 
> Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
> ---
>  fs/f2fs/gc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 6c996e3..41bdfb7 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -959,7 +959,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync, bool background)
>  		 * garbage collections.
>  		 */
>  		ret = write_checkpoint(sbi, &cpc);
> -		if (ret)
> +		if (ret || !has_not_enough_free_secs(sbi, sec_freed, 0))
>  			goto stop;
>  	} else if (gc_type == BG_GC && !background) {
>  		/* f2fs_balance_fs doesn't need to do BG_GC in critical path. */
> 


------------------------------------------------------------------------------
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: avoid unnecessary fg_gc

[Hou Pengyang]

On 2017/2/24 18:28, Chao Yu wrote:
> On 2017/2/24 18:01, Hou Pengyang wrote:
>> Under scenerio with large number of dirty nodes, and these nodes are flushed
>> in SSR mode during cp. enough free segemts now, no need to do fggc.
>
> We'd better break out of GC flow once we encounter cp error, so additional
> condition judgment is not needed.
>

In (ret || !has_not_enough_free_secs(sbi, sec_freed, 0)),
if cp return error(NOT zero), flow will goto stop directly without 
has_not_enough_free_secs checking;
if cp return ok(zero), has_not_enough_free_secs would be checked.

Thanks,

> Thanks,
>
>>
>> Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
>> ---
>>   fs/f2fs/gc.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>> index 6c996e3..41bdfb7 100644
>> --- a/fs/f2fs/gc.c
>> +++ b/fs/f2fs/gc.c
>> @@ -959,7 +959,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync, bool background)
>>   		 * garbage collections.
>>   		 */
>>   		ret = write_checkpoint(sbi, &cpc);
>> -		if (ret)
>> +		if (ret || !has_not_enough_free_secs(sbi, sec_freed, 0))
>>   			goto stop;
>>   	} else if (gc_type == BG_GC && !background) {
>>   		/* f2fs_balance_fs doesn't need to do BG_GC in critical path. */
>>
>
>
> .
>



------------------------------------------------------------------------------
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: avoid unnecessary fg_gc

[Chao Yu]

On 2017/2/25 9:44, Hou Pengyang wrote:
> On 2017/2/24 18:28, Chao Yu wrote:
>> On 2017/2/24 18:01, Hou Pengyang wrote:
>>> Under scenerio with large number of dirty nodes, and these nodes are flushed
>>> in SSR mode during cp. enough free segemts now, no need to do fggc.
>>
>> We'd better break out of GC flow once we encounter cp error, so additional
>> condition judgment is not needed.
>>
> 
> In (ret || !has_not_enough_free_secs(sbi, sec_freed, 0)),
> if cp return error(NOT zero), flow will goto stop directly without 
> has_not_enough_free_secs checking;
> if cp return ok(zero), has_not_enough_free_secs would be checked.

Oh, sorry, I misunderstood your patch, can you add some comments in the code too?

Thanks,

> 
> Thanks,
> 
>> Thanks,
>>
>>>
>>> Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
>>> ---
>>>   fs/f2fs/gc.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>> index 6c996e3..41bdfb7 100644
>>> --- a/fs/f2fs/gc.c
>>> +++ b/fs/f2fs/gc.c
>>> @@ -959,7 +959,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync, bool background)
>>>   		 * garbage collections.
>>>   		 */
>>>   		ret = write_checkpoint(sbi, &cpc);
>>> -		if (ret)
>>> +		if (ret || !has_not_enough_free_secs(sbi, sec_freed, 0))
>>>   			goto stop;
>>>   	} else if (gc_type == BG_GC && !background) {
>>>   		/* f2fs_balance_fs doesn't need to do BG_GC in critical path. */
>>>
>>
>>
>> .
>>
> 
> 
> 
> .
> 


------------------------------------------------------------------------------
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: avoid unnecessary fg_gc

[Jaegeuk Kim]

On 02/25, Hou Pengyang wrote:
> On 2017/2/24 18:28, Chao Yu wrote:
> > On 2017/2/24 18:01, Hou Pengyang wrote:
> > > Under scenerio with large number of dirty nodes, and these nodes are flushed
> > > in SSR mode during cp. enough free segemts now, no need to do fggc.
> > 
> > We'd better break out of GC flow once we encounter cp error, so additional
> > condition judgment is not needed.
> > 
> 
> In (ret || !has_not_enough_free_secs(sbi, sec_freed, 0)),
> if cp return error(NOT zero), flow will goto stop directly without
> has_not_enough_free_secs checking;
> if cp return ok(zero), has_not_enough_free_secs would be checked.

Well, I think it'd be fine to do one gc by a background thread.

Thanks,

> 
> Thanks,
> 
> > Thanks,
> > 
> > > 
> > > Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
> > > ---
> > >   fs/f2fs/gc.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> > > index 6c996e3..41bdfb7 100644
> > > --- a/fs/f2fs/gc.c
> > > +++ b/fs/f2fs/gc.c
> > > @@ -959,7 +959,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync, bool background)
> > >   		 * garbage collections.
> > >   		 */
> > >   		ret = write_checkpoint(sbi, &cpc);
> > > -		if (ret)
> > > +		if (ret || !has_not_enough_free_secs(sbi, sec_freed, 0))
> > >   			goto stop;
> > >   	} else if (gc_type == BG_GC && !background) {
> > >   		/* f2fs_balance_fs doesn't need to do BG_GC in critical path. */
> > > 
> > 
> > 
> > .
> > 
> 

------------------------------------------------------------------------------
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: avoid unnecessary fg_gc

[heyunlei]

Hi all,

I am really confused by the condition below use sec_freed, is it always equal to zero?

Or, gc more process will recheck this condition?

	if (gc_type == BG_GC && has_not_enough_free_secs(sbi, sec_freed, 0))


On 2017/2/25 10:07, Jaegeuk Kim wrote:
> On 02/25, Hou Pengyang wrote:
>> On 2017/2/24 18:28, Chao Yu wrote:
>>> On 2017/2/24 18:01, Hou Pengyang wrote:
>>>> Under scenerio with large number of dirty nodes, and these nodes are flushed
>>>> in SSR mode during cp. enough free segemts now, no need to do fggc.
>>>
>>> We'd better break out of GC flow once we encounter cp error, so additional
>>> condition judgment is not needed.
>>>
>>
>> In (ret || !has_not_enough_free_secs(sbi, sec_freed, 0)),
>> if cp return error(NOT zero), flow will goto stop directly without
>> has_not_enough_free_secs checking;
>> if cp return ok(zero), has_not_enough_free_secs would be checked.
>
> Well, I think it'd be fine to do one gc by a background thread.
>
> Thanks,
>
>>
>> Thanks,
>>
>>> Thanks,
>>>
>>>>
>>>> Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
>>>> ---
>>>>   fs/f2fs/gc.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>>> index 6c996e3..41bdfb7 100644
>>>> --- a/fs/f2fs/gc.c
>>>> +++ b/fs/f2fs/gc.c
>>>> @@ -959,7 +959,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync, bool background)
>>>>   		 * garbage collections.
>>>>   		 */
>>>>   		ret = write_checkpoint(sbi, &cpc);
>>>> -		if (ret)
>>>> +		if (ret || !has_not_enough_free_secs(sbi, sec_freed, 0))
>>>>   			goto stop;
>>>>   	} else if (gc_type == BG_GC && !background) {
>>>>   		/* f2fs_balance_fs doesn't need to do BG_GC in critical path. */
>>>>
>>>
>>>
>>> .
>>>
>>
>
> ------------------------------------------------------------------------------
> 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: avoid unnecessary fg_gc

[Hou Pengyang]

On 2017/2/25 10:07, Jaegeuk Kim wrote:
> On 02/25, Hou Pengyang wrote:
>> On 2017/2/24 18:28, Chao Yu wrote:
>>> On 2017/2/24 18:01, Hou Pengyang wrote:
>>>> Under scenerio with large number of dirty nodes, and these nodes are flushed
>>>> in SSR mode during cp. enough free segemts now, no need to do fggc.
>>>
>>> We'd better break out of GC flow once we encounter cp error, so additional
>>> condition judgment is not needed.
>>>
>>
>> In (ret || !has_not_enough_free_secs(sbi, sec_freed, 0)),
>> if cp return error(NOT zero), flow will goto stop directly without
>> has_not_enough_free_secs checking;
>> if cp return ok(zero), has_not_enough_free_secs would be checked.
>
> Well, I think it'd be fine to do one gc by a background thread.
>
Currently we decide whether convert a bggc to fggc by:

if (gc_type == BG_GC && has_not_enough_free_secs(sbi, sec_freed, 0)) { 

          gc_type = FG_GC;

So how about move up write_checkpoint before gc_type = FG_GC, like:

if (gc_type == BG_GC && has_not_enough_free_secs(sbi, sec_freed, 0)) {
          write_checkpoint;
	 if still has_not_enough_free_secs:
               gc_type = FG_GC;
          else
               stay BG_GC;

Thanks,

> Thanks,
>
>>
>> Thanks,
>>
>>> Thanks,
>>>
>>>>
>>>> Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
>>>> ---
>>>>    fs/f2fs/gc.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>>> index 6c996e3..41bdfb7 100644
>>>> --- a/fs/f2fs/gc.c
>>>> +++ b/fs/f2fs/gc.c
>>>> @@ -959,7 +959,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync, bool background)
>>>>    		 * garbage collections.
>>>>    		 */
>>>>    		ret = write_checkpoint(sbi, &cpc);
>>>> -		if (ret)
>>>> +		if (ret || !has_not_enough_free_secs(sbi, sec_freed, 0))
>>>>    			goto stop;
>>>>    	} else if (gc_type == BG_GC && !background) {
>>>>    		/* f2fs_balance_fs doesn't need to do BG_GC in critical path. */
>>>>
>>>
>>>
>>> .
>>>
>>
>
> .
>



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

[PATCH 1/2] remove stale comment info about cp before fggc

[Hou Pengyang]

In commit c026d3677 (f2fs: remove unnecessary condition check for write_checkpoint in f2fs_gc)
we have removed all the prefree segments/dirty segments checking before cp, remove related comment.

Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
---
 fs/f2fs/gc.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 6c996e3..ffff67ac 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -953,11 +953,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync, bool background)
 
 	if (gc_type == BG_GC && has_not_enough_free_secs(sbi, sec_freed, 0)) {
 		gc_type = FG_GC;
-		/*
-		 * If there is no victim and no prefree segment but still not
-		 * enough free sections, we should flush dent/node blocks and do
-		 * garbage collections.
-		 */
+
 		ret = write_checkpoint(sbi, &cpc);
 		if (ret)
 			goto stop;
-- 
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 2/2] f2fs: avoid bggc->fggc when enough free segments are avaliable after cp

[Hou Pengyang]

We use has_not_enough_free_secs to check if there are enough free segments,

    	(free_sections(sbi) + freed) <=
	        (node_secs + 2 * dent_secs + imeta_secs +
			         reserved_sections(sbi) + needed);

Under scenario with large number of dirty nodes, these nodes would be flushed during cp,
as a result, right side of the inequality would be decreased, while left side stays unchanged
if these nodes are flushed in SSR way, which means there are enough free segments after
this cp.

For this case, we just do a bggc instead of fggc.

Signed-off-by: Hou Pengyang <houpengyang@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 ffff67ac..8cf751f 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -952,11 +952,16 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync, bool background)
 	}
 
 	if (gc_type == BG_GC && has_not_enough_free_secs(sbi, sec_freed, 0)) {
-		gc_type = FG_GC;
 
+		/*
+		 * After cp, there may be enough free segments avaliable, In this case,
+		 * We just do a bggc instead of fggc.
+		 */
 		ret = write_checkpoint(sbi, &cpc);
 		if (ret)
 			goto stop;
+		if (has_not_enough_free_secs(sbi, sec_freed, 0))
+			gc_type = FG_GC;
 	} else if (gc_type == BG_GC && !background) {
 		/* f2fs_balance_fs doesn't need to do BG_GC in critical path. */
 		goto stop;
-- 
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: avoid unnecessary fg_gc

[Jaegeuk Kim]

On 02/25, heyunlei wrote:
> Hi all,
> 
> I am really confused by the condition below use sec_freed, is it always equal to zero?

Seems it is always zero. Let's fix it together.

Pengyang,

I merged your patches and finally got this.
How do you think?

>From d1c2206e4f245a8fedec3a8f21ad522b3b1b2d0c Mon Sep 17 00:00:00 2001
From: Hou Pengyang <houpengyang@huawei.com>
Date: Sat, 25 Feb 2017 03:57:38 +0000
Subject: [PATCH] f2fs: avoid bggc->fggc when enough free segments are
 avaliable after cp

We use has_not_enough_free_secs to check if there are enough free segments,

    	(free_sections(sbi) + freed) <=
	        (node_secs + 2 * dent_secs + imeta_secs +
			         reserved_sections(sbi) + needed);

Under scenario with large number of dirty nodes, these nodes would be flushed
during cp, as a result, right side of the inequality would be decreased, while
left side stays unchanged if these nodes are flushed in SSR way, which means
there are enough free segments after this cp.

For this case, we just do a bggc instead of fggc.

Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/gc.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 1af17c5af603..471ed899f639 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -954,21 +954,22 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync, bool background)
 		goto stop;
 	}
 
-	if (gc_type == BG_GC && has_not_enough_free_secs(sbi, sec_freed, 0)) {
-		gc_type = FG_GC;
+	if (gc_type == BG_GC && has_not_enough_free_secs(sbi, 0, 0)) {
 		/*
-		 * If there is no victim and no prefree segment but still not
-		 * enough free sections, we should flush dent/node blocks and do
-		 * garbage collections.
+		 * For example, if there are many prefree_segments below given
+		 * threshold, we can make them free by checkpoint. Then, we
+		 * secure free segments which doesn't need fggc any more.
 		 */
 		ret = write_checkpoint(sbi, &cpc);
 		if (ret)
 			goto stop;
-	} else if (gc_type == BG_GC && !background) {
-		/* f2fs_balance_fs doesn't need to do BG_GC in critical path. */
-		goto stop;
+		if (has_not_enough_free_secs(sbi, 0, 0))
+			gc_type = FG_GC;
 	}
 
+	/* f2fs_balance_fs doesn't need to do BG_GC in critical path. */
+	if (gc_type == BG_GC && !background)
+		goto stop;
 	if (!__get_victim(sbi, &segno, gc_type))
 		goto stop;
 	ret = 0;
-- 
2.11.0


------------------------------------------------------------------------------
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: avoid unnecessary fg_gc

[Hou Pengyang]

On 2017/2/26 3:39, Jaegeuk Kim wrote:
> On 02/25, heyunlei wrote:
>> Hi all,
>>
>> I am really confused by the condition below use sec_freed, is it always equal to zero?
>
> Seems it is always zero. Let's fix it together.
>
> Pengyang,
>
> I merged your patches and finally got this.
> How do you think?
>
Seems OK to me :)

Thanks,

>>From d1c2206e4f245a8fedec3a8f21ad522b3b1b2d0c Mon Sep 17 00:00:00 2001
> From: Hou Pengyang <houpengyang@huawei.com>
> Date: Sat, 25 Feb 2017 03:57:38 +0000
> Subject: [PATCH] f2fs: avoid bggc->fggc when enough free segments are
>   avaliable after cp
>
> We use has_not_enough_free_secs to check if there are enough free segments,
>
>      	(free_sections(sbi) + freed) <=
> 	        (node_secs + 2 * dent_secs + imeta_secs +
> 			         reserved_sections(sbi) + needed);
>
> Under scenario with large number of dirty nodes, these nodes would be flushed
> during cp, as a result, right side of the inequality would be decreased, while
> left side stays unchanged if these nodes are flushed in SSR way, which means
> there are enough free segments after this cp.
>
> For this case, we just do a bggc instead of fggc.
>
> Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>   fs/f2fs/gc.c | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 1af17c5af603..471ed899f639 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -954,21 +954,22 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync, bool background)
>   		goto stop;
>   	}
>
> -	if (gc_type == BG_GC && has_not_enough_free_secs(sbi, sec_freed, 0)) {
> -		gc_type = FG_GC;
> +	if (gc_type == BG_GC && has_not_enough_free_secs(sbi, 0, 0)) {
>   		/*
> -		 * If there is no victim and no prefree segment but still not
> -		 * enough free sections, we should flush dent/node blocks and do
> -		 * garbage collections.
> +		 * For example, if there are many prefree_segments below given
> +		 * threshold, we can make them free by checkpoint. Then, we
> +		 * secure free segments which doesn't need fggc any more.
>   		 */
>   		ret = write_checkpoint(sbi, &cpc);
>   		if (ret)
>   			goto stop;
> -	} else if (gc_type == BG_GC && !background) {
> -		/* f2fs_balance_fs doesn't need to do BG_GC in critical path. */
> -		goto stop;
> +		if (has_not_enough_free_secs(sbi, 0, 0))
> +			gc_type = FG_GC;
>   	}
>
> +	/* f2fs_balance_fs doesn't need to do BG_GC in critical path. */
> +	if (gc_type == BG_GC && !background)
> +		goto stop;
>   	if (!__get_victim(sbi, &segno, gc_type))
>   		goto stop;
>   	ret = 0;
>



------------------------------------------------------------------------------
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: avoid unnecessary fg_gc

[heyunlei]

Hi Jaegeuk,

On 2017/2/27 9:59, Hou Pengyang wrote:
> We use has_not_enough_free_secs to check if there are enough free segments,
>
>          (free_sections(sbi) + freed) <=
>             (node_secs + 2 * dent_secs + imeta_secs +
>                      reserved_sections(sbi) + needed);

Now node SSR is enable, how can we change this condition simply as:

	(free_sections(sbi) + freed) <= reserved_sections(sbi)

If dirty node pages can find SSR segment, write it in SSR mode,

or it will consume free segments, and then we do FG_GC.



Another thing is that each process has a chance to do FG_GC, it's no problem

for all normal processes, but may be not a good thing for a process within

certain limits such as lower priority, with bound cpu to do FG_GC. Maybe it

will block system for a long time. So what can we do to improve this?

Thanks.



------------------------------------------------------------------------------
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: avoid unnecessary fg_gc

[Chao Yu]

On 2017/2/26 3:39, Jaegeuk Kim wrote:
> On 02/25, heyunlei wrote:
>> Hi all,
>>
>> I am really confused by the condition below use sec_freed, is it always equal to zero?
> 
> Seems it is always zero. Let's fix it together.
> 
> Pengyang,
> 
> I merged your patches and finally got this.
> How do you think?
> 
>>From d1c2206e4f245a8fedec3a8f21ad522b3b1b2d0c Mon Sep 17 00:00:00 2001
> From: Hou Pengyang <houpengyang@huawei.com>
> Date: Sat, 25 Feb 2017 03:57:38 +0000
> Subject: [PATCH] f2fs: avoid bggc->fggc when enough free segments are
>  avaliable after cp
> 
> We use has_not_enough_free_secs to check if there are enough free segments,
> 
>     	(free_sections(sbi) + freed) <=
> 	        (node_secs + 2 * dent_secs + imeta_secs +
> 			         reserved_sections(sbi) + needed);
> 
> Under scenario with large number of dirty nodes, these nodes would be flushed
> during cp, as a result, right side of the inequality would be decreased, while
> left side stays unchanged if these nodes are flushed in SSR way, which means
> there are enough free segments after this cp.
> 
> For this case, we just do a bggc instead of fggc.
> 
> Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,


------------------------------------------------------------------------------
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: avoid unnecessary fg_gc

[Jaegeuk Kim]

On 02/27, heyunlei wrote:
> Hi Jaegeuk,
> 
> On 2017/2/27 9:59, Hou Pengyang wrote:
> > We use has_not_enough_free_secs to check if there are enough free segments,
> > 
> >          (free_sections(sbi) + freed) <=
> >             (node_secs + 2 * dent_secs + imeta_secs +
> >                      reserved_sections(sbi) + needed);
> 
> Now node SSR is enable, how can we change this condition simply as:
> 
> 	(free_sections(sbi) + freed) <= reserved_sections(sbi)

Currently, we can't guarantee successful SSR allocation all the time.

> If dirty node pages can find SSR segment, write it in SSR mode,
> 
> or it will consume free segments, and then we do FG_GC.
> 
> 
> 
> Another thing is that each process has a chance to do FG_GC, it's no problem
> 
> for all normal processes, but may be not a good thing for a process within
> 
> certain limits such as lower priority, with bound cpu to do FG_GC. Maybe it
> 
> will block system for a long time. So what can we do to improve this?

Could you please elaborate this in more detail?

Thanks,

> 
> Thanks.
> 

------------------------------------------------------------------------------
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: avoid unnecessary fg_gc

[heyunlei]

On 2017/2/28 7:40, Jaegeuk Kim wrote:
> On 02/27, heyunlei wrote:
>> Hi Jaegeuk,
>>
>> On 2017/2/27 9:59, Hou Pengyang wrote:
>>> We use has_not_enough_free_secs to check if there are enough free segments,
>>>
>>>          (free_sections(sbi) + freed) <=
>>>             (node_secs + 2 * dent_secs + imeta_secs +
>>>                      reserved_sections(sbi) + needed);
>>
>> Now node SSR is enable, how can we change this condition simply as:
>>
>> 	(free_sections(sbi) + freed) <= reserved_sections(sbi)
>
> Currently, we can't guarantee successful SSR allocation all the time.

Okay, thanks.
>
>> If dirty node pages can find SSR segment, write it in SSR mode,
>>
>> or it will consume free segments, and then we do FG_GC.
>>
>>
>>
>> Another thing is that each process has a chance to do FG_GC, it's no problem
>>
>> for all normal processes, but may be not a good thing for a process within
>>
>> certain limits such as lower priority, with bound cpu to do FG_GC. Maybe it
>>
>> will block system for a long time. So what can we do to improve this?
>
> Could you please elaborate this in more detail?

System bind lots of background processes to one or two cpus. In some corner case,

thread need to do FG GC will compete with other background processes. FG GC can

not be completed in time and then blocks other processes.

Thanks.
>
> Thanks,
>
>>
>> Thanks.
>>
>
> .
>


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