Received-SPF: pass (google.com: domain of linux-kernel+bounces-68084-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240208061825.36640-1-byungchul@sk.com>
In-Reply-To: <20240208061825.36640-1-byungchul@sk.com>
From: Yu Zhao <yuzhao@google.com>
Date: Fri, 16 Feb 2024 00:55:17 -0500
Message-ID: <CAOUHufYUC-oWePfqbbmm15Ue9QLfPg1G2nhXn6iSX_A460O6Uw@mail.gmail.com>
Subject: Re: [PATCH] mm, vmscan: Don't turn on cache_trim_mode at the highest
 scan priority
To: Byungchul Park <byungchul@sk.com>
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org, 
	linux-mm@kvack.org, kernel_team@skhynix.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Feb 8, 2024 at 1:18=E2=80=AFAM Byungchul Park <byungchul@sk.com> wr=
ote:
>
> With cache_trim_mode on, reclaim logic doesn't bother reclaiming anon
> pages. However, it should be more careful to turn on the mode because
> it's going to prevent anon pages from reclaimed even if there are huge
> ammount of anon pages that are very cold so should be reclaimed. Even
> worse, that can lead kswapd_failures to be MAX_RECLAIM_RETRIES and stop
> until direct reclaim eventually works to resume kswapd.

Is a theory or something observed in the real world? If it's the
former, would this change risk breaking existing use cases? It's the
latter, where are the performance numbers to show what it looks like
before and after this patch?

> Signed-off-by: Byungchul Park <byungchul@sk.com>
> ---
>  mm/vmscan.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index bba207f41b14..25b55fdc0d41 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2268,7 +2268,8 @@ static void prepare_scan_control(pg_data_t *pgdat, =
struct scan_control *sc)
>          * anonymous pages.
>          */
>         file =3D lruvec_page_state(target_lruvec, NR_INACTIVE_FILE);
> -       if (file >> sc->priority && !(sc->may_deactivate & DEACTIVATE_FIL=
E))
> +       if (sc->priority !=3D 1 && file >> sc->priority &

Why 1?

> +           !(sc->may_deactivate & DEACTIVATE_FILE))
>                 sc->cache_trim_mode =3D 1;
>         else
>                 sc->cache_trim_mode =3D 0;