[PATCH] f2fs: fix BG_GC count stat

[PATCH] f2fs: fix BG_GC count stat

[Weichao Guo]

We should call stat_inc_bggc_count only after BG_GC has been performed
successfully. Otherwise, BG_GC count may be larger than total count
when f2fs_gc failed, e.g., no victim was selected.

Signed-off-by: Weichao Guo <guoweichao@huawei.com>
---
 fs/f2fs/gc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 0265221..fbea046 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -81,11 +81,11 @@ static int gc_thread_func(void *data)
 		else
 			increase_sleep_time(gc_th, &wait_ms);
 
-		stat_inc_bggc_count(sbi);
-
 		/* if return value is not zero, no victim was selected */
 		if (f2fs_gc(sbi, test_opt(sbi, FORCE_FG_GC), true, NULL_SEGNO))
 			wait_ms = gc_th->no_gc_sleep_time;
+		else
+			stat_inc_bggc_count(sbi);
 
 		trace_f2fs_background_gc(sbi->sb, wait_ms,
 				prefree_segments(sbi), free_segments(sbi));
-- 
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: fix BG_GC count stat

[Jaegeuk Kim]

On 05/09, Weichao Guo wrote:
> We should call stat_inc_bggc_count only after BG_GC has been performed
> successfully. Otherwise, BG_GC count may be larger than total count
> when f2fs_gc failed, e.g., no victim was selected.

BTW, if we do this, we can't monitor the call count of BGGC. Actually we don't
need to match this with total GC count. How about adding another variable to
show successful GC count which looks like (BG: x/y)?

Thanks,

> 
> Signed-off-by: Weichao Guo <guoweichao@huawei.com>
> ---
>  fs/f2fs/gc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 0265221..fbea046 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -81,11 +81,11 @@ static int gc_thread_func(void *data)
>  		else
>  			increase_sleep_time(gc_th, &wait_ms);
>  
> -		stat_inc_bggc_count(sbi);
> -
>  		/* if return value is not zero, no victim was selected */
>  		if (f2fs_gc(sbi, test_opt(sbi, FORCE_FG_GC), true, NULL_SEGNO))
>  			wait_ms = gc_th->no_gc_sleep_time;
> +		else
> +			stat_inc_bggc_count(sbi);
>  
>  		trace_f2fs_background_gc(sbi->sb, wait_ms,
>  				prefree_segments(sbi), free_segments(sbi));
> -- 
> 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: fix BG_GC count stat

[guoweichao]

Hi Jaegeuk,

I agree that stat info miss matching is not a big problem. But consistent stat view is better I think.
I will give a new patch as you suggested.

Thanks,
Weichao
-----邮件原件-----
发件人: Jaegeuk Kim [mailto:jaegeuk@kernel.org] 
发送时间: 2017年5月10日 1:52
收件人: guoweichao
抄送: linux-f2fs-devel@lists.sourceforge.net; Wangbintian
主题: Re: [PATCH] f2fs: fix BG_GC count stat

On 05/09, Weichao Guo wrote:
> We should call stat_inc_bggc_count only after BG_GC has been performed 
> successfully. Otherwise, BG_GC count may be larger than total count 
> when f2fs_gc failed, e.g., no victim was selected.

BTW, if we do this, we can't monitor the call count of BGGC. Actually we don't need to match this with total GC count. How about adding another variable to show successful GC count which looks like (BG: x/y)?

Thanks,

> 
> Signed-off-by: Weichao Guo <guoweichao@huawei.com>
> ---
>  fs/f2fs/gc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index 0265221..fbea046 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -81,11 +81,11 @@ static int gc_thread_func(void *data)
>  		else
>  			increase_sleep_time(gc_th, &wait_ms);
>  
> -		stat_inc_bggc_count(sbi);
> -
>  		/* if return value is not zero, no victim was selected */
>  		if (f2fs_gc(sbi, test_opt(sbi, FORCE_FG_GC), true, NULL_SEGNO))
>  			wait_ms = gc_th->no_gc_sleep_time;
> +		else
> +			stat_inc_bggc_count(sbi);
>  
>  		trace_f2fs_background_gc(sbi->sb, wait_ms,
>  				prefree_segments(sbi), free_segments(sbi));
> --
> 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
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel