* [PATCH] mm, vmscan: Don't turn on cache_trim_mode at the highest scan priority
@ 2024-02-08 6:18 Byungchul Park
2024-02-16 5:55 ` Yu Zhao
0 siblings, 1 reply; 6+ messages in thread
From: Byungchul Park @ 2024-02-08 6:18 UTC (permalink / raw
To: akpm; +Cc: linux-kernel, linux-mm, kernel_team
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.
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 = lruvec_page_state(target_lruvec, NR_INACTIVE_FILE);
- if (file >> sc->priority && !(sc->may_deactivate & DEACTIVATE_FILE))
+ if (sc->priority != 1 && file >> sc->priority &&
+ !(sc->may_deactivate & DEACTIVATE_FILE))
sc->cache_trim_mode = 1;
else
sc->cache_trim_mode = 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mm, vmscan: Don't turn on cache_trim_mode at the highest scan priority
2024-02-08 6:18 [PATCH] mm, vmscan: Don't turn on cache_trim_mode at the highest scan priority Byungchul Park
@ 2024-02-16 5:55 ` Yu Zhao
2024-02-16 7:24 ` Byungchul Park
0 siblings, 1 reply; 6+ messages in thread
From: Yu Zhao @ 2024-02-16 5:55 UTC (permalink / raw
To: Byungchul Park; +Cc: akpm, linux-kernel, linux-mm, kernel_team
On Thu, Feb 8, 2024 at 1:18 AM Byungchul Park <byungchul@sk.com> wrote:
>
> 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 = lruvec_page_state(target_lruvec, NR_INACTIVE_FILE);
> - if (file >> sc->priority && !(sc->may_deactivate & DEACTIVATE_FILE))
> + if (sc->priority != 1 && file >> sc->priority &
Why 1?
> + !(sc->may_deactivate & DEACTIVATE_FILE))
> sc->cache_trim_mode = 1;
> else
> sc->cache_trim_mode = 0;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm, vmscan: Don't turn on cache_trim_mode at the highest scan priority
2024-02-16 5:55 ` Yu Zhao
@ 2024-02-16 7:24 ` Byungchul Park
2024-02-17 5:11 ` Yu Zhao
0 siblings, 1 reply; 6+ messages in thread
From: Byungchul Park @ 2024-02-16 7:24 UTC (permalink / raw
To: Yu Zhao; +Cc: akpm, linux-kernel, linux-mm, kernel_team
On Fri, Feb 16, 2024 at 12:55:17AM -0500, Yu Zhao wrote:
> On Thu, Feb 8, 2024 at 1:18 AM Byungchul Park <byungchul@sk.com> wrote:
> >
> > 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
I faced the latter case.
> latter, where are the performance numbers to show what it looks like
> before and after this patch?
Before:
Whenever the system meets the condition to turn on cache_trim_mode but
few cache pages to trim, kswapd fails without scanning anon pages that
are plenty and cold for sure and it retries 8 times and looks *stopped
for ever*.
After:
When the system meets the condition to turn on cache_trim_mode but few
cache pages to trim, kswapd finally works at the highest scan priority.
So kswap looks working well even in the same condition.
> > 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 = lruvec_page_state(target_lruvec, NR_INACTIVE_FILE);
> > - if (file >> sc->priority && !(sc->may_deactivate & DEACTIVATE_FILE))
> > + if (sc->priority != 1 && file >> sc->priority &
>
> Why 1?
It means the highest scan priority. The priority goes from DEF_PRIORITY
to 1.
Byungchul
> > + !(sc->may_deactivate & DEACTIVATE_FILE))
> > sc->cache_trim_mode = 1;
> > else
> > sc->cache_trim_mode = 0;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm, vmscan: Don't turn on cache_trim_mode at the highest scan priority
2024-02-16 7:24 ` Byungchul Park
@ 2024-02-17 5:11 ` Yu Zhao
2024-02-21 22:30 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Yu Zhao @ 2024-02-17 5:11 UTC (permalink / raw
To: Byungchul Park; +Cc: akpm, linux-kernel, linux-mm, kernel_team
On Fri, Feb 16, 2024 at 2:24 AM Byungchul Park <byungchul@sk.com> wrote:
>
> On Fri, Feb 16, 2024 at 12:55:17AM -0500, Yu Zhao wrote:
> > On Thu, Feb 8, 2024 at 1:18 AM Byungchul Park <byungchul@sk.com> wrote:
> > >
> > > 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
>
> I faced the latter case.
>
> > latter, where are the performance numbers to show what it looks like
> > before and after this patch?
Let me ask again: where are the performance numbers to show what it
looks like before and after this patch?
> Before:
>
> Whenever the system meets the condition to turn on cache_trim_mode but
> few cache pages to trim, kswapd fails without scanning anon pages that
> are plenty and cold for sure and it retries 8 times and looks *stopped
> for ever*.
>
> After:
>
> When the system meets the condition to turn on cache_trim_mode but few
> cache pages to trim, kswapd finally works at the highest scan priority.
> So kswap looks working well even in the same condition.
These are not performance numbers -- what test cases can prove what's
described here?
> > > 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 = lruvec_page_state(target_lruvec, NR_INACTIVE_FILE);
> > > - if (file >> sc->priority && !(sc->may_deactivate & DEACTIVATE_FILE))
> > > + if (sc->priority != 1 && file >> sc->priority &
> >
> > Why 1?
>
> It means the highest scan priority. The priority goes from DEF_PRIORITY
> to 1.
This is not true -- sc->priority can go all the way to zero.
> Byungchul
>
> > > + !(sc->may_deactivate & DEACTIVATE_FILE))
> > > sc->cache_trim_mode = 1;
> > > else
> > > sc->cache_trim_mode = 0;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm, vmscan: Don't turn on cache_trim_mode at the highest scan priority
2024-02-17 5:11 ` Yu Zhao
@ 2024-02-21 22:30 ` Andrew Morton
2024-02-22 3:27 ` Byungchul Park
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2024-02-21 22:30 UTC (permalink / raw
To: Yu Zhao; +Cc: Byungchul Park, linux-kernel, linux-mm, kernel_team
On Sat, 17 Feb 2024 00:11:25 -0500 Yu Zhao <yuzhao@google.com> wrote:
> On Fri, Feb 16, 2024 at 2:24 AM Byungchul Park <byungchul@sk.com> wrote:
> >
> > On Fri, Feb 16, 2024 at 12:55:17AM -0500, Yu Zhao wrote:
> > > On Thu, Feb 8, 2024 at 1:18 AM Byungchul Park <byungchul@sk.com> wrote:
> > > >
> > > > 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
> >
> > I faced the latter case.
> >
> > > latter, where are the performance numbers to show what it looks like
> > > before and after this patch?
>
> Let me ask again: where are the performance numbers to show what it
> looks like before and after this patch?
>
> > Before:
> >
> > Whenever the system meets the condition to turn on cache_trim_mode but
> > few cache pages to trim, kswapd fails without scanning anon pages that
> > are plenty and cold for sure and it retries 8 times and looks *stopped
> > for ever*.
Does "stopped for ever" mean that kswapd simply stops functioning?
If so, that's a pretty serious issue. Please fully describe all of
this in the changelog. Please also address Yu Zhao's review comments
and send us a v2 patch? Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm, vmscan: Don't turn on cache_trim_mode at the highest scan priority
2024-02-21 22:30 ` Andrew Morton
@ 2024-02-22 3:27 ` Byungchul Park
0 siblings, 0 replies; 6+ messages in thread
From: Byungchul Park @ 2024-02-22 3:27 UTC (permalink / raw
To: Andrew Morton; +Cc: Yu Zhao, linux-kernel, linux-mm, kernel_team
On Wed, Feb 21, 2024 at 02:30:13PM -0800, Andrew Morton wrote:
> On Sat, 17 Feb 2024 00:11:25 -0500 Yu Zhao <yuzhao@google.com> wrote:
>
> > On Fri, Feb 16, 2024 at 2:24 AM Byungchul Park <byungchul@sk.com> wrote:
> > >
> > > On Fri, Feb 16, 2024 at 12:55:17AM -0500, Yu Zhao wrote:
> > > > On Thu, Feb 8, 2024 at 1:18 AM Byungchul Park <byungchul@sk.com> wrote:
> > > > >
> > > > > 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
> > >
> > > I faced the latter case.
> > >
> > > > latter, where are the performance numbers to show what it looks like
> > > > before and after this patch?
> >
> > Let me ask again: where are the performance numbers to show what it
> > looks like before and after this patch?
> >
> > > Before:
> > >
> > > Whenever the system meets the condition to turn on cache_trim_mode but
> > > few cache pages to trim, kswapd fails without scanning anon pages that
> > > are plenty and cold for sure and it retries 8 times and looks *stopped
> > > for ever*.
>
> Does "stopped for ever" mean that kswapd simply stops functioning?
Yes. kswapd stops its functioning. Even worse, after being stopped, any
request to wake up kswapd fails until ->kswapd_failures gets reset to 0
by direct reclaim or something.
It's more like a bug fix than a performance improvement.
> If so, that's a pretty serious issue. Please fully describe all of
> this in the changelog. Please also address Yu Zhao's review comments
> and send us a v2 patch? Thanks.
I will post v2 with vmstat numbers between before and after.
Byungchul
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-02-22 3:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-08 6:18 [PATCH] mm, vmscan: Don't turn on cache_trim_mode at the highest scan priority Byungchul Park
2024-02-16 5:55 ` Yu Zhao
2024-02-16 7:24 ` Byungchul Park
2024-02-17 5:11 ` Yu Zhao
2024-02-21 22:30 ` Andrew Morton
2024-02-22 3:27 ` Byungchul Park
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).