Date:   Thu, 16 Mar 2023 13:14:05 +0000
From:   Matthew Wilcox <willy@infradead.org>
To:     yang.yang29@zte.com.cn
Cc:     akpm@linux-foundation.org, hannes@cmpxchg.org,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        iamjoonsoo.kim@lge.com
Subject: Re: =?iso-8859-1?Q?=A0=5BPATCH_linux-next?=
 =?iso-8859-1?Q?=5D_mm=3A_workingset=3A_simplify_the=A0calculatio?=
 =?iso-8859-1?Q?n?= of workingset size
Message-ID: <ZBMWHUJfFnIX7F9y@casper.infradead.org>
References: <202303161723055514455@zte.com.cn>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <202303161723055514455@zte.com.cn>
Precedence: bulk

On Thu, Mar 16, 2023 at 05:23:05PM +0800, yang.yang29@zte.com.cn wrote:
>  	 * Compare the distance to the existing workingset size. We
>  	 * don't activate pages that couldn't stay resident even if
> -	 * all the memory was available to the workingset. Whether
> -	 * workingset competition needs to consider anon or not depends
> -	 * on having swap.
> +	 * all the memory was available to the workingset. For page
> +	 * cache whether workingset competition needs to consider
> +	 * anon or not depends on having swap.

I don't mind this change

>  	 */
>  	workingset_size = lruvec_page_state(eviction_lruvec, NR_ACTIVE_FILE);
> +	/* For anonymous page */

This comment adds no value

>  	if (!file) {
> +		workingset_size += lruvec_page_state(eviction_lruvec,
> +						     NR_ACTIVE_ANON);
>  		workingset_size += lruvec_page_state(eviction_lruvec,
>  						     NR_INACTIVE_FILE);
> -	}
> -	if (mem_cgroup_get_nr_swap_pages(eviction_memcg) > 0) {
> +	/* For page cache */

Nor this one

> +	} else if (mem_cgroup_get_nr_swap_pages(eviction_memcg) > 0) {
>  		workingset_size += lruvec_page_state(eviction_lruvec,
>  						     NR_ACTIVE_ANON);
> -		if (file) {
> -			workingset_size += lruvec_page_state(eviction_lruvec,
> +		workingset_size += lruvec_page_state(eviction_lruvec,
>  						     NR_INACTIVE_ANON);
> -		}
>  	}

I don't have an opinion on the actual code changes.