Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <a766299e-1a75-5293-dfeb-8f5d6c395644@kernel.org>
Date:   Fri, 1 Jul 2022 08:50:02 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.10.0
Subject: Re: [PATCH 7/7] f2fs: optimize shrink count
Content-Language: en-US
To:     Guowei Du <duguoweisz@gmail.com>, jaegeuk@kernel.org
Cc:     linux-f2fs-devel@lists.sourceforge.net,
        linux-kernel@vger.kernel.org, duguowei <duguowei@xiaomi.com>
References: <20220630112629.6611-1-duguoweisz@gmail.com>
From:   Chao Yu <chao@kernel.org>
In-Reply-To: <20220630112629.6611-1-duguoweisz@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2022/6/30 19:26, Guowei Du wrote:
> From: duguowei <duguowei@xiaomi.com>
> 
> Remove the local variable 'run_no', add comments for better
> reading.

Not sure, I guess original implementation considers the race condition for
f2fs_shrink_scan vs f2fs_shrink_scan case? so it uses a local variable to
keep the per-round context info?

Jaegeuk, please help to check that.

Thanks,

> 
> Signed-off-by: duguowei <duguowei@xiaomi.com>
> ---
>   fs/f2fs/shrinker.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
> index dd3c3c7a90ec..7e439496dfca 100644
> --- a/fs/f2fs/shrinker.c
> +++ b/fs/f2fs/shrinker.c
> @@ -76,18 +76,18 @@ unsigned long f2fs_shrink_scan(struct shrinker *shrink,
>   	unsigned long nr = sc->nr_to_scan;
>   	struct f2fs_sb_info *sbi;
>   	struct list_head *p;
> -	unsigned int run_no;
>   	unsigned long freed = 0;
>   
>   	spin_lock(&f2fs_list_lock);
> -	do {
> -		run_no = ++shrinker_run_no;
> -	} while (run_no == 0);
> +	shrinker_run_no++;
> +	/* if overflow, next loop needs to skip number 0 */
> +	if (!shrinker_run_no)
> +		shrinker_run_no = 1;
>   	p = f2fs_list.next;
>   	while (p != &f2fs_list) {
>   		sbi = list_entry(p, struct f2fs_sb_info, s_list);
>   
> -		if (sbi->shrinker_run_no == run_no)
> +		if (sbi->shrinker_run_no == shrinker_run_no)
>   			break;
>   
>   		/* stop f2fs_put_super */
> @@ -97,7 +97,7 @@ unsigned long f2fs_shrink_scan(struct shrinker *shrink,
>   		}
>   		spin_unlock(&f2fs_list_lock);
>   
> -		sbi->shrinker_run_no = run_no;
> +		sbi->shrinker_run_no = shrinker_run_no;
>   
>   		/* shrink extent cache entries */
>   		freed += f2fs_shrink_extent_tree(sbi, nr >> 1);