* [PATCH] mm: mglru: clear the reference counter for rejected folios
@ 2026-09-07 3:25 Baolin Wang
2026-09-07 4:28 ` Barry Song
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Baolin Wang @ 2026-09-07 3:25 UTC (permalink / raw)
To: akpm
Cc: kasong, qi.zheng, shakeel.butt, baohua, axelrasmussen, yuanchu,
weixugc, hannes, david, mhocko, ljs, ridong.chen, hebaoquan,
baolin.wang, linux-mm, linux-kernel
As per the comment on LRU_REFS_FLAGS, when accessed folios are promoted to
a new generation, LRU_REFS_FLAGS should be cleared so that the reference
counter can start over.
For folios rejected by shrink_folio_list(), we clear LRU_REFS_FLAGS and
set the PG_active flag if the rejected folio is planned to be put back to
the oldest generation. That's fine.
But for those that are not put back to the oldest generation (which can
be treated as a promotion), we do not clear LRU_REFS_FLAGS, which can
violate the promotion mechanism. This means the rejected folio enters the
new generation with stale, inflated tier bits, which can inflate reference
counts and distort eviction statistics for these rejected folios.
Fix this by clearing LRU_REFS_FLAGS for rejected folios, and also do some
measurement. On my 32-core Arm machine, with the memcg limit set to 3G,
running 'make -j32' to build the kernel showed a small improvement in sys
time when using either a zram or NVMe swap device (averaged over 2 runs with
no significant variance).
zram swap:
w/o patch w/ patch
sys time: 1666.5s 1589.5s
NVMe swap:
w/o patch w/patch
sys time: 760s 741.5s
Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
mm/vmscan.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 40d3f1b48a74..42c0a09938ab 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -5021,10 +5021,11 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
}
/* don't add rejected folios to the oldest generation */
- if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) {
- folio_set_lru_refs(folio, 0);
+ if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type])
folio_set_active(folio);
- }
+
+ /* See the comments on LRU_REFS_FLAGS */
+ folio_set_lru_refs(folio, 0);
}
move_folios_to_lru(&list);
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH] mm: mglru: clear the reference counter for rejected folios 2026-09-07 3:25 [PATCH] mm: mglru: clear the reference counter for rejected folios Baolin Wang @ 2026-09-07 4:28 ` Barry Song 2026-09-07 5:53 ` Baolin Wang 2026-09-07 4:57 ` Kairui Song 2026-09-08 2:30 ` Baoquan He 2 siblings, 1 reply; 14+ messages in thread From: Barry Song @ 2026-09-07 4:28 UTC (permalink / raw) To: Baolin Wang Cc: akpm, kasong, qi.zheng, shakeel.butt, axelrasmussen, yuanchu, weixugc, hannes, david, mhocko, ljs, ridong.chen, hebaoquan, linux-mm, linux-kernel On Mon, Sep 7, 2026 at 11:25 AM Baolin Wang <baolin.wang@linux.alibaba.com> wrote: > > As per the comment on LRU_REFS_FLAGS, when accessed folios are promoted to > a new generation, LRU_REFS_FLAGS should be cleared so that the reference > counter can start over. > > For folios rejected by shrink_folio_list(), we clear LRU_REFS_FLAGS and > set the PG_active flag if the rejected folio is planned to be put back to > the oldest generation. That's fine. I find the description a bit confusing. Yes, according to `lru_gen_folio_seq()`, the folio would be put back into the oldest generation. That's why we set the active bit to prevent this from happening. So there is no case where the folio is intentionally put back into the oldest generation. > > But for those that are not put back to the oldest generation (which can > be treated as a promotion), we do not clear LRU_REFS_FLAGS, which can > violate the promotion mechanism. This means the rejected folio enters the > new generation with stale, inflated tier bits, which can inflate reference > counts and distort eviction statistics for these rejected folios. Maybe simply say that folios for which lru_gen_folio_seq() returns something other than min_seq? > > Fix this by clearing LRU_REFS_FLAGS for rejected folios, and also do some > measurement. On my 32-core Arm machine, with the memcg limit set to 3G, > running 'make -j32' to build the kernel showed a small improvement in sys > time when using either a zram or NVMe swap device (averaged over 2 runs with > no significant variance). > > zram swap: > w/o patch w/ patch > sys time: 1666.5s 1589.5s > > NVMe swap: > w/o patch w/patch > sys time: 760s 741.5s > > Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> > --- > mm/vmscan.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 40d3f1b48a74..42c0a09938ab 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -5021,10 +5021,11 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec, > } > > /* don't add rejected folios to the oldest generation */ > - if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) { > - folio_set_lru_refs(folio, 0); > + if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) > folio_set_active(folio); > - } > + > + /* See the comments on LRU_REFS_FLAGS */ > + folio_set_lru_refs(folio, 0); > } The code change makes sense to me if we keep aging and always reclaim only the oldest generation. However, if we go further in `scan_folios()`—for example, continuing to scan after we have fewer than two generations left—we might not be in that case anymore. So far, the code looks correct to me. BTW, Baolin, I wonder if you could find some time to test this change on your arm64 system: https://lore.kernel.org/linux-mm/20260901220430.79810-1-baohua@kernel.org/ It might have addressed one of your previous, similar concerns here. On my x86 machine, I find that the change further reduces the sys time from 3 minutes 40 seconds to 3 minutes 30 seconds. Meanwhile, I will also run your patch on my x86 machine. > > move_folios_to_lru(&list); > -- > 2.47.3 > Thanks Barry ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm: mglru: clear the reference counter for rejected folios 2026-09-07 4:28 ` Barry Song @ 2026-09-07 5:53 ` Baolin Wang 0 siblings, 0 replies; 14+ messages in thread From: Baolin Wang @ 2026-09-07 5:53 UTC (permalink / raw) To: Barry Song Cc: akpm, kasong, qi.zheng, shakeel.butt, axelrasmussen, yuanchu, weixugc, hannes, david, mhocko, ljs, ridong.chen, hebaoquan, linux-mm, linux-kernel On 9/7/26 12:28 PM, Barry Song wrote: > On Mon, Sep 7, 2026 at 11:25 AM Baolin Wang > <baolin.wang@linux.alibaba.com> wrote: >> >> As per the comment on LRU_REFS_FLAGS, when accessed folios are promoted to >> a new generation, LRU_REFS_FLAGS should be cleared so that the reference >> counter can start over. >> >> For folios rejected by shrink_folio_list(), we clear LRU_REFS_FLAGS and >> set the PG_active flag if the rejected folio is planned to be put back to >> the oldest generation. That's fine. > > I find the description a bit confusing. Yes, according to > `lru_gen_folio_seq()`, the folio would be put back into the oldest > generation. That's why we set the active bit to prevent this from > happening. So there is no case where the folio is intentionally put > back into the oldest generation. How about? " For folios rejected by shrink_folio_list(), we clear LRU_REFS_FLAGS and set the PG_active flag when lru_gen_folio_seq() would place them in the oldest generation. " >> But for those that are not put back to the oldest generation (which can >> be treated as a promotion), we do not clear LRU_REFS_FLAGS, which can >> violate the promotion mechanism. This means the rejected folio enters the >> new generation with stale, inflated tier bits, which can inflate reference >> counts and distort eviction statistics for these rejected folios. > > Maybe simply say that folios for which lru_gen_folio_seq() > returns something other than min_seq? OK. >> Fix this by clearing LRU_REFS_FLAGS for rejected folios, and also do some >> measurement. On my 32-core Arm machine, with the memcg limit set to 3G, >> running 'make -j32' to build the kernel showed a small improvement in sys >> time when using either a zram or NVMe swap device (averaged over 2 runs with >> no significant variance). >> >> zram swap: >> w/o patch w/ patch >> sys time: 1666.5s 1589.5s >> >> NVMe swap: >> w/o patch w/patch >> sys time: 760s 741.5s >> >> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> >> --- >> mm/vmscan.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/mm/vmscan.c b/mm/vmscan.c >> index 40d3f1b48a74..42c0a09938ab 100644 >> --- a/mm/vmscan.c >> +++ b/mm/vmscan.c >> @@ -5021,10 +5021,11 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec, >> } >> >> /* don't add rejected folios to the oldest generation */ >> - if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) { >> - folio_set_lru_refs(folio, 0); >> + if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) >> folio_set_active(folio); >> - } >> + >> + /* See the comments on LRU_REFS_FLAGS */ >> + folio_set_lru_refs(folio, 0); >> } > > The code change makes sense to me if we keep aging and always reclaim > only the oldest generation. However, if we go further in > `scan_folios()`—for example, continuing to scan after we have fewer > than two generations left—we might not be in that case anymore. I may not fully understand your concern. If there is only one generation (fewer than two generations), we should aging instead of reclaim. But my point is that regardless, if promotion happens, we should clear the reference counter. Otherwise, as I mentioned, it will distort eviction statistics. > So far, the code looks correct to me. > > BTW, Baolin, I wonder if you could find some time to test this > change on your arm64 system: > https://lore.kernel.org/linux-mm/20260901220430.79810-1-baohua@kernel.org/ > > It might have addressed one of your previous, similar concerns here. > On my x86 machine, I find that the change further reduces the > sys time from 3 minutes 40 seconds to 3 minutes 30 seconds. > > Meanwhile, I will also run your patch on my x86 machine. Sigh. I missed this thread. Yes, I also raised a similar issue before [1] and this is still my next step. I'll find some time to check this thread. Thanks for reminding me. [1] https://lore.kernel.org/all/eb395442-0aad-428a-a5ac-9072d2d89060@linux.alibaba.com/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm: mglru: clear the reference counter for rejected folios 2026-09-07 3:25 [PATCH] mm: mglru: clear the reference counter for rejected folios Baolin Wang 2026-09-07 4:28 ` Barry Song @ 2026-09-07 4:57 ` Kairui Song 2026-09-07 6:27 ` Baolin Wang 2026-09-08 2:30 ` Baoquan He 2 siblings, 1 reply; 14+ messages in thread From: Kairui Song @ 2026-09-07 4:57 UTC (permalink / raw) To: Baolin Wang Cc: akpm, qi.zheng, shakeel.butt, baohua, axelrasmussen, yuanchu, weixugc, hannes, david, mhocko, ljs, ridong.chen, hebaoquan, linux-mm, linux-kernel On Mon, Sep 7, 2026 at 11:29 AM Baolin Wang <baolin.wang@linux.alibaba.com> wrote: > > As per the comment on LRU_REFS_FLAGS, when accessed folios are promoted to > a new generation, LRU_REFS_FLAGS should be cleared so that the reference > counter can start over. > > For folios rejected by shrink_folio_list(), we clear LRU_REFS_FLAGS and > set the PG_active flag if the rejected folio is planned to be put back to > the oldest generation. That's fine. > > But for those that are not put back to the oldest generation (which can > be treated as a promotion), we do not clear LRU_REFS_FLAGS, which can > violate the promotion mechanism. This means the rejected folio enters the > new generation with stale, inflated tier bits, which can inflate reference > counts and distort eviction statistics for these rejected folios. > > Fix this by clearing LRU_REFS_FLAGS for rejected folios, and also do some > measurement. On my 32-core Arm machine, with the memcg limit set to 3G, > running 'make -j32' to build the kernel showed a small improvement in sys > time when using either a zram or NVMe swap device (averaged over 2 runs with > no significant variance). > > zram swap: > w/o patch w/ patch > sys time: 1666.5s 1589.5s > > NVMe swap: > w/o patch w/patch > sys time: 760s 741.5s Hi Baolin, Thanks for the patch, it makes sense and I like the idea! However, I find it interesting that your test setup shows such a significant benefit with several recent changes when I can't observe a performance gain on any of my setups. I'm a bit worried this (not only this patch) might be overfitting into to the kernel build test on specific setups. I did try this optimization before and found no gain, maybe it is somehow tangled with some other recent upstream changes? For example a few recently landed MGLRU optimizations sped up the ZRAM kernel build test on your setup, but slowed down many other cases. I still think this is mergable, but before that, do you have the LRU_REFS_WIDTH data from your kernel build? Or lru_gen_full output? In some cases it shrinks to only 1 or 0 bits, leading to very different performance readings. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm: mglru: clear the reference counter for rejected folios 2026-09-07 4:57 ` Kairui Song @ 2026-09-07 6:27 ` Baolin Wang 0 siblings, 0 replies; 14+ messages in thread From: Baolin Wang @ 2026-09-07 6:27 UTC (permalink / raw) To: Kairui Song Cc: akpm, qi.zheng, shakeel.butt, baohua, axelrasmussen, yuanchu, weixugc, hannes, david, mhocko, ljs, ridong.chen, hebaoquan, linux-mm, linux-kernel On 9/7/26 12:57 PM, Kairui Song wrote: > On Mon, Sep 7, 2026 at 11:29 AM Baolin Wang > <baolin.wang@linux.alibaba.com> wrote: >> >> As per the comment on LRU_REFS_FLAGS, when accessed folios are promoted to >> a new generation, LRU_REFS_FLAGS should be cleared so that the reference >> counter can start over. >> >> For folios rejected by shrink_folio_list(), we clear LRU_REFS_FLAGS and >> set the PG_active flag if the rejected folio is planned to be put back to >> the oldest generation. That's fine. >> >> But for those that are not put back to the oldest generation (which can >> be treated as a promotion), we do not clear LRU_REFS_FLAGS, which can >> violate the promotion mechanism. This means the rejected folio enters the >> new generation with stale, inflated tier bits, which can inflate reference >> counts and distort eviction statistics for these rejected folios. >> >> Fix this by clearing LRU_REFS_FLAGS for rejected folios, and also do some >> measurement. On my 32-core Arm machine, with the memcg limit set to 3G, >> running 'make -j32' to build the kernel showed a small improvement in sys >> time when using either a zram or NVMe swap device (averaged over 2 runs with >> no significant variance). >> >> zram swap: >> w/o patch w/ patch >> sys time: 1666.5s 1589.5s >> >> NVMe swap: >> w/o patch w/patch >> sys time: 760s 741.5s > > Hi Baolin, > > Thanks for the patch, it makes sense and I like the idea! Thanks for taking a look. > However, I find it interesting that your test setup shows such a > significant benefit with several recent changes when I can't observe a > performance gain on any of my setups. I'm a bit worried this (not only > this patch) might be overfitting into to the kernel build test on > specific setups. > > I did try this optimization before and found no gain, maybe it is > somehow tangled with some other recent upstream changes? Probably. > > For example a few recently landed MGLRU optimizations sped up the ZRAM > kernel build test on your setup, but slowed down many other cases. I'm not sure if there are other hardware environment differences. I agree that for complex changes or optimizations, we need to cover more test cases than just the kernel build workload, such as the test coverage in your MGLRU-FG work. However, for the current patch, I think it's more about correcting the correctness of the promotion mechanism, and the goal is not merely performance optimization. If we stack more changes on top of the current broken mechanism, I'm afraid future optimizations will become more fragile. So let's first reach agreement on the underlying promotion mechanism. Also, as I replied to Barry, the promotion in lru_gen_set_refs() needs to be reconsidered as well. It similarly requires clearing reference counters before promotion. So for this simple mechanism correction, I only evaluated the kernel build (which is still somewhat representative as a comprehensive workload) and it did not introduce a regression (which is fortunate). I wouldn't want any workload to rely on this incorrect logic for performance gains. > I still think this is mergable, but before that, do you have the > LRU_REFS_WIDTH data from your kernel build? Or lru_gen_full output? In > some cases it shrinks to only 1 or 0 bits, leading to very different > performance readings. The LRU_REFS_WIDTH is always 2 on my setup. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm: mglru: clear the reference counter for rejected folios 2026-09-07 3:25 [PATCH] mm: mglru: clear the reference counter for rejected folios Baolin Wang 2026-09-07 4:28 ` Barry Song 2026-09-07 4:57 ` Kairui Song @ 2026-09-08 2:30 ` Baoquan He 2026-09-08 2:34 ` Barry Song 2 siblings, 1 reply; 14+ messages in thread From: Baoquan He @ 2026-09-08 2:30 UTC (permalink / raw) To: Baolin Wang Cc: akpm, kasong, qi.zheng, shakeel.butt, baohua, axelrasmussen, yuanchu, weixugc, hannes, david, mhocko, ljs, ridong.chen, hebaoquan, linux-mm, linux-kernel Hi Baolin, On 09/07/26 at 11:25am, Baolin Wang wrote: ......snip... > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 40d3f1b48a74..42c0a09938ab 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c Well, this seems to be based on Andrew's mm-new branch. I usually track mm-unstable branch. Maybe the subject should be marked as below? [PATCH mm-new] mm: mglru: clear the reference counter for rejected > @@ -5021,10 +5021,11 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec, > } > > /* don't add rejected folios to the oldest generation */ > - if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) { > - folio_set_lru_refs(folio, 0); > + if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) > folio_set_active(folio); > - } > + > + /* See the comments on LRU_REFS_FLAGS */ > + folio_set_lru_refs(folio, 0); This looks like a great catch, while the code change could bring issue. Because move_folios_to_lru() relies on folios' flags to decide their new generation. You just cleared it before move_folios_to_lru(). This is no problem for rejected folios that are determined to be put into the oldest generation. But for those rejected folios that are determined to be promoted, this could be wrong. E.g currently gen window is 4, and a folio is referenced, lru_gen_folio_seq() decides its new gen as 1, which is the 2nd oldest generation. While folio_set_lru_refs(folio, 0) clear referenced bit, this causes it being put into the oldest generation in move_folios_to_lru(), this is not expected. Thanks Baoquan > } > > move_folios_to_lru(&list); > -- > 2.47.3 > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm: mglru: clear the reference counter for rejected folios 2026-09-08 2:30 ` Baoquan He @ 2026-09-08 2:34 ` Barry Song 2026-09-08 3:03 ` Baolin Wang 0 siblings, 1 reply; 14+ messages in thread From: Barry Song @ 2026-09-08 2:34 UTC (permalink / raw) To: Baoquan He Cc: Baolin Wang, akpm, kasong, qi.zheng, shakeel.butt, axelrasmussen, yuanchu, weixugc, hannes, david, mhocko, ljs, ridong.chen, hebaoquan, linux-mm, linux-kernel On Tue, Sep 8, 2026 at 10:30 AM Baoquan He <baoquan.he@linux.dev> wrote: > > Hi Baolin, > > On 09/07/26 at 11:25am, Baolin Wang wrote: > ......snip... > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > index 40d3f1b48a74..42c0a09938ab 100644 > > --- a/mm/vmscan.c > > +++ b/mm/vmscan.c > > Well, this seems to be based on Andrew's mm-new branch. I usually track > mm-unstable branch. Maybe the subject should be marked as below? > [PATCH mm-new] mm: mglru: clear the reference counter for rejected > > > @@ -5021,10 +5021,11 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec, > > } > > > > /* don't add rejected folios to the oldest generation */ > > - if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) { > > - folio_set_lru_refs(folio, 0); > > + if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) > > folio_set_active(folio); > > - } > > + > > + /* See the comments on LRU_REFS_FLAGS */ > > + folio_set_lru_refs(folio, 0); > > This looks like a great catch, while the code change could bring issue. > > Because move_folios_to_lru() relies on folios' flags to decide their new > generation. You just cleared it before move_folios_to_lru(). This is no > problem for rejected folios that are determined to be put into the > oldest generation. But for those rejected folios that are determined to > be promoted, this could be wrong. E.g currently gen window is 4, and a > folio is referenced, lru_gen_folio_seq() decides its new gen as 1, which > is the 2nd oldest generation. While folio_set_lru_refs(folio, 0) clear > referenced bit, this causes it being put into the oldest generation in > move_folios_to_lru(), this is not expected. The original code looks quite weird. It even prioritizes folios that won't be promoted by `PG_active`. Do we need to change all the cases just to call `PG_active`? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm: mglru: clear the reference counter for rejected folios 2026-09-08 2:34 ` Barry Song @ 2026-09-08 3:03 ` Baolin Wang 2026-09-08 4:01 ` Baolin Wang 0 siblings, 1 reply; 14+ messages in thread From: Baolin Wang @ 2026-09-08 3:03 UTC (permalink / raw) To: Barry Song, Baoquan He Cc: akpm, kasong, qi.zheng, shakeel.butt, axelrasmussen, yuanchu, weixugc, hannes, david, mhocko, ljs, ridong.chen, hebaoquan, linux-mm, linux-kernel On 9/8/26 10:34 AM, Barry Song wrote: > On Tue, Sep 8, 2026 at 10:30 AM Baoquan He <baoquan.he@linux.dev> wrote: >> >> Hi Baolin, >> >> On 09/07/26 at 11:25am, Baolin Wang wrote: >> ......snip... >>> diff --git a/mm/vmscan.c b/mm/vmscan.c >>> index 40d3f1b48a74..42c0a09938ab 100644 >>> --- a/mm/vmscan.c >>> +++ b/mm/vmscan.c >> >> Well, this seems to be based on Andrew's mm-new branch. I usually track >> mm-unstable branch. Maybe the subject should be marked as below? >> [PATCH mm-new] mm: mglru: clear the reference counter for rejected ACK. >> >>> @@ -5021,10 +5021,11 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec, >>> } >>> >>> /* don't add rejected folios to the oldest generation */ >>> - if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) { >>> - folio_set_lru_refs(folio, 0); >>> + if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) >>> folio_set_active(folio); >>> - } >>> + >>> + /* See the comments on LRU_REFS_FLAGS */ >>> + folio_set_lru_refs(folio, 0); >> >> This looks like a great catch, while the code change could bring issue. >> >> Because move_folios_to_lru() relies on folios' flags to decide their new >> generation. You just cleared it before move_folios_to_lru(). This is no >> problem for rejected folios that are determined to be put into the >> oldest generation. But for those rejected folios that are determined to >> be promoted, this could be wrong. E.g currently gen window is 4, and a >> folio is referenced, lru_gen_folio_seq() decides its new gen as 1, which >> is the 2nd oldest generation. While folio_set_lru_refs(folio, 0) clear >> referenced bit, this causes it being put into the oldest generation in >> move_folios_to_lru(), this is not expected. Yes. As I discussed with Barry earlier, lru_gen_folio_seq() also needs to be reconsidered regarding whether it should rely on PG_referenced [1]. For commit 6cbdd9726fb5, we didn't discuss the impact on rejected folios either. Before commit 6cbdd9726fb5, if rejected folios did not have PG_active set by shrink_folio_list(), evict_folios() would set PG_active on these rejected folios. [1] https://lore.kernel.org/linux-mm/20260901220430.79810-1-baohua@kernel.org/ > The original code looks quite weird. It even prioritizes folios > that won't be promoted by `PG_active`. Do we need to change all > the cases just to call `PG_active`? Yes. Regarding this concern, I plan to change back to the original behavior: /* See the comments on LRU_REFS_FLAGS */ folio_set_lru_refs(folio, 0); /* don't add rejected folios to the oldest generation */ if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) folio_set_active(folio); What do you think? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm: mglru: clear the reference counter for rejected folios 2026-09-08 3:03 ` Baolin Wang @ 2026-09-08 4:01 ` Baolin Wang 2026-09-08 6:59 ` Baoquan He 0 siblings, 1 reply; 14+ messages in thread From: Baolin Wang @ 2026-09-08 4:01 UTC (permalink / raw) To: Barry Song, Baoquan He Cc: akpm, kasong, qi.zheng, shakeel.butt, axelrasmussen, yuanchu, weixugc, hannes, david, mhocko, ljs, ridong.chen, hebaoquan, linux-mm, linux-kernel On 9/8/26 11:03 AM, Baolin Wang wrote: > > > On 9/8/26 10:34 AM, Barry Song wrote: >> On Tue, Sep 8, 2026 at 10:30 AM Baoquan He <baoquan.he@linux.dev> wrote: >>> >>> Hi Baolin, >>> >>> On 09/07/26 at 11:25am, Baolin Wang wrote: >>> ......snip... >>>> diff --git a/mm/vmscan.c b/mm/vmscan.c >>>> index 40d3f1b48a74..42c0a09938ab 100644 >>>> --- a/mm/vmscan.c >>>> +++ b/mm/vmscan.c >>> >>> Well, this seems to be based on Andrew's mm-new branch. I usually track >>> mm-unstable branch. Maybe the subject should be marked as below? >>> [PATCH mm-new] mm: mglru: clear the reference counter for rejected > > ACK. > >>> >>>> @@ -5021,10 +5021,11 @@ static int evict_folios(unsigned long >>>> nr_to_scan, struct lruvec *lruvec, >>>> } >>>> >>>> /* don't add rejected folios to the oldest generation */ >>>> - if (lru_gen_folio_seq(lruvec, folio, false) == >>>> min_seq[type]) { >>>> - folio_set_lru_refs(folio, 0); >>>> + if (lru_gen_folio_seq(lruvec, folio, false) == >>>> min_seq[type]) >>>> folio_set_active(folio); >>>> - } >>>> + >>>> + /* See the comments on LRU_REFS_FLAGS */ >>>> + folio_set_lru_refs(folio, 0); >>> >>> This looks like a great catch, while the code change could bring issue. >>> >>> Because move_folios_to_lru() relies on folios' flags to decide their new >>> generation. You just cleared it before move_folios_to_lru(). This is no >>> problem for rejected folios that are determined to be put into the >>> oldest generation. But for those rejected folios that are determined to >>> be promoted, this could be wrong. E.g currently gen window is 4, and a >>> folio is referenced, lru_gen_folio_seq() decides its new gen as 1, which >>> is the 2nd oldest generation. While folio_set_lru_refs(folio, 0) clear >>> referenced bit, this causes it being put into the oldest generation in >>> move_folios_to_lru(), this is not expected. > > Yes. As I discussed with Barry earlier, lru_gen_folio_seq() also needs > to be reconsidered regarding whether it should rely on PG_referenced [1]. > > For commit 6cbdd9726fb5, we didn't discuss the impact on rejected folios > either. Before commit 6cbdd9726fb5, if rejected folios did not have > PG_active set by shrink_folio_list(), evict_folios() would set PG_active > on these rejected folios. > > [1] https://lore.kernel.org/linux-mm/20260901220430.79810-1- > baohua@kernel.org/ > >> The original code looks quite weird. It even prioritizes folios >> that won't be promoted by `PG_active`. Do we need to change all >> the cases just to call `PG_active`? > > Yes. Regarding this concern, I plan to change back to the original > behavior: > > /* See the comments on LRU_REFS_FLAGS */ > folio_set_lru_refs(folio, 0); > > /* don't add rejected folios to the oldest generation */ > if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) > folio_set_active(folio); > > What do you think? Just FYI, after above changes, the performance improvement on zram is no longer obvious either. I think this also answers Kairui's earlier question about why I saw a performance improvement (which seems related to commit 6cbdd9726fb5). Also, there is no obvious performance regression either. zram swap: w/o patch w/ patch sys time: 1666.5s 1628.5s ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm: mglru: clear the reference counter for rejected folios 2026-09-08 4:01 ` Baolin Wang @ 2026-09-08 6:59 ` Baoquan He 2026-09-08 7:53 ` Baolin Wang 0 siblings, 1 reply; 14+ messages in thread From: Baoquan He @ 2026-09-08 6:59 UTC (permalink / raw) To: Baolin Wang Cc: Barry Song, akpm, kasong, qi.zheng, shakeel.butt, axelrasmussen, yuanchu, weixugc, hannes, david, mhocko, ljs, ridong.chen, hebaoquan, linux-mm, linux-kernel On 09/08/26 at 12:01pm, Baolin Wang wrote: > > > On 9/8/26 11:03 AM, Baolin Wang wrote: > > > > > > On 9/8/26 10:34 AM, Barry Song wrote: > > > On Tue, Sep 8, 2026 at 10:30 AM Baoquan He <baoquan.he@linux.dev> wrote: > > > > > > > > Hi Baolin, > > > > > > > > On 09/07/26 at 11:25am, Baolin Wang wrote: > > > > ......snip... > > > > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > > > > index 40d3f1b48a74..42c0a09938ab 100644 > > > > > --- a/mm/vmscan.c > > > > > +++ b/mm/vmscan.c > > > > > > > > Well, this seems to be based on Andrew's mm-new branch. I usually track > > > > mm-unstable branch. Maybe the subject should be marked as below? > > > > [PATCH mm-new] mm: mglru: clear the reference counter for rejected > > > > ACK. > > > > > > > > > > > @@ -5021,10 +5021,11 @@ static int evict_folios(unsigned > > > > > long nr_to_scan, struct lruvec *lruvec, > > > > > } > > > > > > > > > > /* don't add rejected folios to the oldest generation */ > > > > > - if (lru_gen_folio_seq(lruvec, folio, false) == > > > > > min_seq[type]) { > > > > > - folio_set_lru_refs(folio, 0); > > > > > + if (lru_gen_folio_seq(lruvec, folio, false) == > > > > > min_seq[type]) > > > > > folio_set_active(folio); > > > > > - } > > > > > + > > > > > + /* See the comments on LRU_REFS_FLAGS */ > > > > > + folio_set_lru_refs(folio, 0); > > > > > > > > This looks like a great catch, while the code change could bring issue. > > > > > > > > Because move_folios_to_lru() relies on folios' flags to decide their new > > > > generation. You just cleared it before move_folios_to_lru(). This is no > > > > problem for rejected folios that are determined to be put into the > > > > oldest generation. But for those rejected folios that are determined to > > > > be promoted, this could be wrong. E.g currently gen window is 4, and a > > > > folio is referenced, lru_gen_folio_seq() decides its new gen as 1, which > > > > is the 2nd oldest generation. While folio_set_lru_refs(folio, 0) clear > > > > referenced bit, this causes it being put into the oldest generation in > > > > move_folios_to_lru(), this is not expected. > > > > Yes. As I discussed with Barry earlier, lru_gen_folio_seq() also needs > > to be reconsidered regarding whether it should rely on PG_referenced > > [1]. > > > > For commit 6cbdd9726fb5, we didn't discuss the impact on rejected folios > > either. Before commit 6cbdd9726fb5, if rejected folios did not have > > PG_active set by shrink_folio_list(), evict_folios() would set PG_active > > on these rejected folios. > > > > [1] https://lore.kernel.org/linux-mm/20260901220430.79810-1- > > baohua@kernel.org/ > > > > > The original code looks quite weird. It even prioritizes folios > > > that won't be promoted by `PG_active`. Do we need to change all > > > the cases just to call `PG_active`? I would agree if we can. While I have one concern. A rejected folio that should have gone into the oldest generation is now being promoted to the 2nd newest generation. In the original code, referenced folio is only being promoted to the next gen. I even think this is not a bug, but Yu Zhao intentionally did it: the coldest folio is moved to 2nd newest gen, referenced folio (hot folio) is moved to new gen but carries the referenced bit. Both of them seems to be treated somewhat equally. To me, I would rather move both of them to the next gen, while keep their refs untouched. > > > > Yes. Regarding this concern, I plan to change back to the original > > behavior: > > > > /* See the comments on LRU_REFS_FLAGS */ > > folio_set_lru_refs(folio, 0); > > > > /* don't add rejected folios to the oldest generation */ > > if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) > > folio_set_active(folio); > > > > What do you think? > > Just FYI, after above changes, the performance improvement on zram is no > longer obvious either. I think this also answers Kairui's earlier question > about why I saw a performance improvement (which seems related to commit > 6cbdd9726fb5). Also, there is no obvious performance regression either. Hi Baolin, Not sure if it's convenient to do a little more testing in your side. E.g rejected folios are moved to next gen, but not clearing their flags. Then we can at least know which one is more efficient on yout testing platform. coldest folio referenced folio --------------------------------------------- max_gen -1 next gen + referenced kept max_gen -1 max_gen -1 next_gen next gen + referenced kept Thanks Baoquan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm: mglru: clear the reference counter for rejected folios 2026-09-08 6:59 ` Baoquan He @ 2026-09-08 7:53 ` Baolin Wang 2026-09-08 8:23 ` Baoquan He 0 siblings, 1 reply; 14+ messages in thread From: Baolin Wang @ 2026-09-08 7:53 UTC (permalink / raw) To: Baoquan He Cc: Barry Song, akpm, kasong, qi.zheng, shakeel.butt, axelrasmussen, yuanchu, weixugc, hannes, david, mhocko, ljs, ridong.chen, hebaoquan, linux-mm, linux-kernel On 9/8/26 2:59 PM, Baoquan He wrote: > On 09/08/26 at 12:01pm, Baolin Wang wrote: >> >> >> On 9/8/26 11:03 AM, Baolin Wang wrote: >>> >>> >>> On 9/8/26 10:34 AM, Barry Song wrote: >>>> On Tue, Sep 8, 2026 at 10:30 AM Baoquan He <baoquan.he@linux.dev> wrote: >>>>> >>>>> Hi Baolin, >>>>> >>>>> On 09/07/26 at 11:25am, Baolin Wang wrote: >>>>> ......snip... >>>>>> diff --git a/mm/vmscan.c b/mm/vmscan.c >>>>>> index 40d3f1b48a74..42c0a09938ab 100644 >>>>>> --- a/mm/vmscan.c >>>>>> +++ b/mm/vmscan.c >>>>> >>>>> Well, this seems to be based on Andrew's mm-new branch. I usually track >>>>> mm-unstable branch. Maybe the subject should be marked as below? >>>>> [PATCH mm-new] mm: mglru: clear the reference counter for rejected >>> >>> ACK. >>> >>>>> >>>>>> @@ -5021,10 +5021,11 @@ static int evict_folios(unsigned >>>>>> long nr_to_scan, struct lruvec *lruvec, >>>>>> } >>>>>> >>>>>> /* don't add rejected folios to the oldest generation */ >>>>>> - if (lru_gen_folio_seq(lruvec, folio, false) == >>>>>> min_seq[type]) { >>>>>> - folio_set_lru_refs(folio, 0); >>>>>> + if (lru_gen_folio_seq(lruvec, folio, false) == >>>>>> min_seq[type]) >>>>>> folio_set_active(folio); >>>>>> - } >>>>>> + >>>>>> + /* See the comments on LRU_REFS_FLAGS */ >>>>>> + folio_set_lru_refs(folio, 0); >>>>> >>>>> This looks like a great catch, while the code change could bring issue. >>>>> >>>>> Because move_folios_to_lru() relies on folios' flags to decide their new >>>>> generation. You just cleared it before move_folios_to_lru(). This is no >>>>> problem for rejected folios that are determined to be put into the >>>>> oldest generation. But for those rejected folios that are determined to >>>>> be promoted, this could be wrong. E.g currently gen window is 4, and a >>>>> folio is referenced, lru_gen_folio_seq() decides its new gen as 1, which >>>>> is the 2nd oldest generation. While folio_set_lru_refs(folio, 0) clear >>>>> referenced bit, this causes it being put into the oldest generation in >>>>> move_folios_to_lru(), this is not expected. >>> >>> Yes. As I discussed with Barry earlier, lru_gen_folio_seq() also needs >>> to be reconsidered regarding whether it should rely on PG_referenced >>> [1]. >>> >>> For commit 6cbdd9726fb5, we didn't discuss the impact on rejected folios >>> either. Before commit 6cbdd9726fb5, if rejected folios did not have >>> PG_active set by shrink_folio_list(), evict_folios() would set PG_active >>> on these rejected folios. >>> >>> [1] https://lore.kernel.org/linux-mm/20260901220430.79810-1- >>> baohua@kernel.org/ >>> >>>> The original code looks quite weird. It even prioritizes folios >>>> that won't be promoted by `PG_active`. Do we need to change all >>>> the cases just to call `PG_active`? > > I would agree if we can. While I have one concern. A rejected folio that > should have gone into the oldest generation is now being promoted to the > 2nd newest generation. In the original code, referenced folio is only > being promoted to the next gen. I even think this is not a bug, but Yu That's not quite true. Before commit 6cbdd9726fb5, a rejected referenced folio was also put back to the 2nd youngest gen. > Zhao intentionally did it: the coldest folio is moved to 2nd newest gen, > referenced folio (hot folio) is moved to new gen but carries the referenced > bit. Both of them seems to be treated somewhat equally. > > To me, I would rather move both of them to the next gen, while keep their > refs untouched. IMHO, I strongly recommend not doing this, and that's exactly the motivation behind my patch. Because this is also being treated as a promotion, it should behave like folio_inc_gen() or folio_update_gen() and clear the refs after promotion. I think this is a fundamental principle of promotion. Otherwise, ref-based promotion is already completely broken. Next, I also plan to clean up refs in lru_gen_set_refs() as discussed with Barry. >>> Yes. Regarding this concern, I plan to change back to the original >>> behavior: >>> >>> /* See the comments on LRU_REFS_FLAGS */ >>> folio_set_lru_refs(folio, 0); >>> >>> /* don't add rejected folios to the oldest generation */ >>> if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) >>> folio_set_active(folio); >>> >>> What do you think? >> >> Just FYI, after above changes, the performance improvement on zram is no >> longer obvious either. I think this also answers Kairui's earlier question >> about why I saw a performance improvement (which seems related to commit >> 6cbdd9726fb5). Also, there is no obvious performance regression either. > > Hi Baolin, > > Not sure if it's convenient to do a little more testing in your side. > E.g rejected folios are moved to next gen, but not clearing their flags. I'm not sure what you mean by "next gen" here. If you mean the 2nd oldest gen, I actually tested that too, that is, clearing refs after move_folios_to_lru(), and there wasn't any noticeable performance impact either (but the code was a bit hacky, so I didn't go with this approach). > Then we can at least know which one is more efficient on yout testing > platform. > > coldest folio referenced folio > --------------------------------------------- > max_gen -1 next gen + referenced kept > max_gen -1 max_gen -1 > next_gen next gen + referenced kept > > Thanks > Baoquan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm: mglru: clear the reference counter for rejected folios 2026-09-08 7:53 ` Baolin Wang @ 2026-09-08 8:23 ` Baoquan He 2026-09-08 9:39 ` Baolin Wang 0 siblings, 1 reply; 14+ messages in thread From: Baoquan He @ 2026-09-08 8:23 UTC (permalink / raw) To: Baolin Wang Cc: Barry Song, akpm, kasong, qi.zheng, shakeel.butt, axelrasmussen, yuanchu, weixugc, hannes, david, mhocko, ljs, ridong.chen, hebaoquan, linux-mm, linux-kernel On 09/08/26 at 03:53pm, Baolin Wang wrote: > > > On 9/8/26 2:59 PM, Baoquan He wrote: > > On 09/08/26 at 12:01pm, Baolin Wang wrote: > > > > > > > > > On 9/8/26 11:03 AM, Baolin Wang wrote: > > > > > > > > > > > > On 9/8/26 10:34 AM, Barry Song wrote: > > > > > On Tue, Sep 8, 2026 at 10:30 AM Baoquan He <baoquan.he@linux.dev> wrote: > > > > > > > > > > > > Hi Baolin, > > > > > > > > > > > > On 09/07/26 at 11:25am, Baolin Wang wrote: > > > > > > ......snip... > > > > > > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > > > > > > index 40d3f1b48a74..42c0a09938ab 100644 > > > > > > > --- a/mm/vmscan.c > > > > > > > +++ b/mm/vmscan.c > > > > > > > > > > > > Well, this seems to be based on Andrew's mm-new branch. I usually track > > > > > > mm-unstable branch. Maybe the subject should be marked as below? > > > > > > [PATCH mm-new] mm: mglru: clear the reference counter for rejected > > > > > > > > ACK. > > > > > > > > > > > > > > > > > @@ -5021,10 +5021,11 @@ static int evict_folios(unsigned > > > > > > > long nr_to_scan, struct lruvec *lruvec, > > > > > > > } > > > > > > > > > > > > > > /* don't add rejected folios to the oldest generation */ > > > > > > > - if (lru_gen_folio_seq(lruvec, folio, false) == > > > > > > > min_seq[type]) { > > > > > > > - folio_set_lru_refs(folio, 0); > > > > > > > + if (lru_gen_folio_seq(lruvec, folio, false) == > > > > > > > min_seq[type]) > > > > > > > folio_set_active(folio); > > > > > > > - } > > > > > > > + > > > > > > > + /* See the comments on LRU_REFS_FLAGS */ > > > > > > > + folio_set_lru_refs(folio, 0); > > > > > > > > > > > > This looks like a great catch, while the code change could bring issue. > > > > > > > > > > > > Because move_folios_to_lru() relies on folios' flags to decide their new > > > > > > generation. You just cleared it before move_folios_to_lru(). This is no > > > > > > problem for rejected folios that are determined to be put into the > > > > > > oldest generation. But for those rejected folios that are determined to > > > > > > be promoted, this could be wrong. E.g currently gen window is 4, and a > > > > > > folio is referenced, lru_gen_folio_seq() decides its new gen as 1, which > > > > > > is the 2nd oldest generation. While folio_set_lru_refs(folio, 0) clear > > > > > > referenced bit, this causes it being put into the oldest generation in > > > > > > move_folios_to_lru(), this is not expected. > > > > > > > > Yes. As I discussed with Barry earlier, lru_gen_folio_seq() also needs > > > > to be reconsidered regarding whether it should rely on PG_referenced > > > > [1]. > > > > > > > > For commit 6cbdd9726fb5, we didn't discuss the impact on rejected folios > > > > either. Before commit 6cbdd9726fb5, if rejected folios did not have > > > > PG_active set by shrink_folio_list(), evict_folios() would set PG_active > > > > on these rejected folios. > > > > > > > > [1] https://lore.kernel.org/linux-mm/20260901220430.79810-1- > > > > baohua@kernel.org/ > > > > > > > > > The original code looks quite weird. It even prioritizes folios > > > > > that won't be promoted by `PG_active`. Do we need to change all > > > > > the cases just to call `PG_active`? > > > > I would agree if we can. While I have one concern. A rejected folio that > > should have gone into the oldest generation is now being promoted to the > > 2nd newest generation. In the original code, referenced folio is only > > being promoted to the next gen. I even think this is not a bug, but Yu > > That's not quite true. Before commit 6cbdd9726fb5, a rejected referenced > folio was also put back to the 2nd youngest gen. OK, I didn't follow your earlier discussion, I need take some time to fully understand that commit and the patch thread from Ehab. > > > Zhao intentionally did it: the coldest folio is moved to 2nd newest gen, > > referenced folio (hot folio) is moved to new gen but carries the referenced > > bit. Both of them seems to be treated somewhat equally. > > > > To me, I would rather move both of them to the next gen, while keep their > > refs untouched. > > IMHO, I strongly recommend not doing this, and that's exactly the motivation > behind my patch. Because this is also being treated as a promotion, it > should behave like folio_inc_gen() or folio_update_gen() and clear the refs > after promotion. I think this is a fundamental principle of promotion. > Otherwise, ref-based promotion is already completely broken. OK, it makes sense to me to make principle of promotion strictly applied no efficiency degradation involved. > > Next, I also plan to clean up refs in lru_gen_set_refs() as discussed with > Barry. Looks forward to seeing that. By the way, your discussion with Baryr is private or in public list, do you have pointer if public? Thanks. > > > > > Yes. Regarding this concern, I plan to change back to the original > > > > behavior: > > > > > > > > /* See the comments on LRU_REFS_FLAGS */ > > > > folio_set_lru_refs(folio, 0); > > > > > > > > /* don't add rejected folios to the oldest generation */ > > > > if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) > > > > folio_set_active(folio); > > > > > > > > What do you think? > > > > > > Just FYI, after above changes, the performance improvement on zram is no > > > longer obvious either. I think this also answers Kairui's earlier question > > > about why I saw a performance improvement (which seems related to commit > > > 6cbdd9726fb5). Also, there is no obvious performance regression either. > > > > Hi Baolin, > > > > Not sure if it's convenient to do a little more testing in your side. > > E.g rejected folios are moved to next gen, but not clearing their flags. > > I'm not sure what you mean by "next gen" here. If you mean the 2nd oldest > gen, I actually tested that too, that is, clearing refs after > move_folios_to_lru(), and there wasn't any noticeable performance impact > either (but the code was a bit hacky, so I didn't go with this approach). Yeah, I meant the 2nd oldest gen, while what I am curious about is moving them into 2nd oldest gen but not clearing refs. Clearly it's conflicting with your plan. Anyway, it's just a brain store idea, please forget it. Thanks for the sharing and detailed explanation. > > > Then we can at least know which one is more efficient on yout testing > > platform. > > > > coldest folio referenced folio > > --------------------------------------------- > > max_gen -1 next gen + referenced kept > > max_gen -1 max_gen -1 > > next_gen next gen + referenced kept > > > > Thanks > > Baoquan > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm: mglru: clear the reference counter for rejected folios 2026-09-08 8:23 ` Baoquan He @ 2026-09-08 9:39 ` Baolin Wang 2026-09-08 10:29 ` Baoquan He 0 siblings, 1 reply; 14+ messages in thread From: Baolin Wang @ 2026-09-08 9:39 UTC (permalink / raw) To: Baoquan He Cc: Barry Song, akpm, kasong, qi.zheng, shakeel.butt, axelrasmussen, yuanchu, weixugc, hannes, david, mhocko, ljs, ridong.chen, hebaoquan, linux-mm, linux-kernel On 9/8/26 4:23 PM, Baoquan He wrote: > On 09/08/26 at 03:53pm, Baolin Wang wrote: >> >> >> On 9/8/26 2:59 PM, Baoquan He wrote: >>> On 09/08/26 at 12:01pm, Baolin Wang wrote: >>>> >>>> >>>> On 9/8/26 11:03 AM, Baolin Wang wrote: >>>>> >>>>> >>>>> On 9/8/26 10:34 AM, Barry Song wrote: >>>>>> On Tue, Sep 8, 2026 at 10:30 AM Baoquan He <baoquan.he@linux.dev> wrote: >>>>>>> >>>>>>> Hi Baolin, >>>>>>> >>>>>>> On 09/07/26 at 11:25am, Baolin Wang wrote: >>>>>>> ......snip... >>>>>>>> diff --git a/mm/vmscan.c b/mm/vmscan.c >>>>>>>> index 40d3f1b48a74..42c0a09938ab 100644 >>>>>>>> --- a/mm/vmscan.c >>>>>>>> +++ b/mm/vmscan.c >>>>>>> >>>>>>> Well, this seems to be based on Andrew's mm-new branch. I usually track >>>>>>> mm-unstable branch. Maybe the subject should be marked as below? >>>>>>> [PATCH mm-new] mm: mglru: clear the reference counter for rejected >>>>> >>>>> ACK. >>>>> >>>>>>> >>>>>>>> @@ -5021,10 +5021,11 @@ static int evict_folios(unsigned >>>>>>>> long nr_to_scan, struct lruvec *lruvec, >>>>>>>> } >>>>>>>> >>>>>>>> /* don't add rejected folios to the oldest generation */ >>>>>>>> - if (lru_gen_folio_seq(lruvec, folio, false) == >>>>>>>> min_seq[type]) { >>>>>>>> - folio_set_lru_refs(folio, 0); >>>>>>>> + if (lru_gen_folio_seq(lruvec, folio, false) == >>>>>>>> min_seq[type]) >>>>>>>> folio_set_active(folio); >>>>>>>> - } >>>>>>>> + >>>>>>>> + /* See the comments on LRU_REFS_FLAGS */ >>>>>>>> + folio_set_lru_refs(folio, 0); >>>>>>> >>>>>>> This looks like a great catch, while the code change could bring issue. >>>>>>> >>>>>>> Because move_folios_to_lru() relies on folios' flags to decide their new >>>>>>> generation. You just cleared it before move_folios_to_lru(). This is no >>>>>>> problem for rejected folios that are determined to be put into the >>>>>>> oldest generation. But for those rejected folios that are determined to >>>>>>> be promoted, this could be wrong. E.g currently gen window is 4, and a >>>>>>> folio is referenced, lru_gen_folio_seq() decides its new gen as 1, which >>>>>>> is the 2nd oldest generation. While folio_set_lru_refs(folio, 0) clear >>>>>>> referenced bit, this causes it being put into the oldest generation in >>>>>>> move_folios_to_lru(), this is not expected. >>>>> >>>>> Yes. As I discussed with Barry earlier, lru_gen_folio_seq() also needs >>>>> to be reconsidered regarding whether it should rely on PG_referenced >>>>> [1]. >>>>> >>>>> For commit 6cbdd9726fb5, we didn't discuss the impact on rejected folios >>>>> either. Before commit 6cbdd9726fb5, if rejected folios did not have >>>>> PG_active set by shrink_folio_list(), evict_folios() would set PG_active >>>>> on these rejected folios. >>>>> >>>>> [1] https://lore.kernel.org/linux-mm/20260901220430.79810-1- >>>>> baohua@kernel.org/ >>>>> >>>>>> The original code looks quite weird. It even prioritizes folios >>>>>> that won't be promoted by `PG_active`. Do we need to change all >>>>>> the cases just to call `PG_active`? >>> >>> I would agree if we can. While I have one concern. A rejected folio that >>> should have gone into the oldest generation is now being promoted to the >>> 2nd newest generation. In the original code, referenced folio is only >>> being promoted to the next gen. I even think this is not a bug, but Yu >> >> That's not quite true. Before commit 6cbdd9726fb5, a rejected referenced >> folio was also put back to the 2nd youngest gen. > > OK, I didn't follow your earlier discussion, I need take some time to > fully understand that commit and the patch thread from Ehab. > >> >>> Zhao intentionally did it: the coldest folio is moved to 2nd newest gen, >>> referenced folio (hot folio) is moved to new gen but carries the referenced >>> bit. Both of them seems to be treated somewhat equally. >>> >>> To me, I would rather move both of them to the next gen, while keep their >>> refs untouched. >> >> IMHO, I strongly recommend not doing this, and that's exactly the motivation >> behind my patch. Because this is also being treated as a promotion, it >> should behave like folio_inc_gen() or folio_update_gen() and clear the refs >> after promotion. I think this is a fundamental principle of promotion. >> Otherwise, ref-based promotion is already completely broken. > > OK, it makes sense to me to make principle of promotion strictly applied > no efficiency degradation involved. > >> >> Next, I also plan to clean up refs in lru_gen_set_refs() as discussed with >> Barry. > > Looks forward to seeing that. By the way, your discussion with Baryr is > private or in public list, do you have pointer if public? Thanks. I raised this issue before[1], and recently there has been another discussion[2] about it (we had some private discussions, but I'll post a new patch for discussion). [1] https://lore.kernel.org/all/eb395442-0aad-428a-a5ac-9072d2d89060@linux.alibaba.com/ [2] https://lore.kernel.org/linux-mm/20260901220430.79810-1-baohua@kernel.org/ >>>>> Yes. Regarding this concern, I plan to change back to the original >>>>> behavior: >>>>> >>>>> /* See the comments on LRU_REFS_FLAGS */ >>>>> folio_set_lru_refs(folio, 0); >>>>> >>>>> /* don't add rejected folios to the oldest generation */ >>>>> if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) >>>>> folio_set_active(folio); >>>>> >>>>> What do you think? >>>> >>>> Just FYI, after above changes, the performance improvement on zram is no >>>> longer obvious either. I think this also answers Kairui's earlier question >>>> about why I saw a performance improvement (which seems related to commit >>>> 6cbdd9726fb5). Also, there is no obvious performance regression either. >>> >>> Hi Baolin, >>> >>> Not sure if it's convenient to do a little more testing in your side. >>> E.g rejected folios are moved to next gen, but not clearing their flags. >> >> I'm not sure what you mean by "next gen" here. If you mean the 2nd oldest >> gen, I actually tested that too, that is, clearing refs after >> move_folios_to_lru(), and there wasn't any noticeable performance impact >> either (but the code was a bit hacky, so I didn't go with this approach). > > Yeah, I meant the 2nd oldest gen, while what I am curious about is moving > them into 2nd oldest gen but not clearing refs. Clearly it's conflicting > with your plan. Anyway, it's just a brain store idea, please forget it. > Thanks for the sharing and detailed explanation. Thanks for reviewing. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm: mglru: clear the reference counter for rejected folios 2026-09-08 9:39 ` Baolin Wang @ 2026-09-08 10:29 ` Baoquan He 0 siblings, 0 replies; 14+ messages in thread From: Baoquan He @ 2026-09-08 10:29 UTC (permalink / raw) To: Baolin Wang Cc: Barry Song, akpm, kasong, qi.zheng, shakeel.butt, axelrasmussen, yuanchu, weixugc, hannes, david, mhocko, ljs, ridong.chen, hebaoquan, linux-mm, linux-kernel On 09/08/26 at 05:39pm, Baolin Wang wrote: > > > On 9/8/26 4:23 PM, Baoquan He wrote: > > On 09/08/26 at 03:53pm, Baolin Wang wrote: > > > > > > > > > On 9/8/26 2:59 PM, Baoquan He wrote: > > > > On 09/08/26 at 12:01pm, Baolin Wang wrote: > > > > > > > > > > > > > > > On 9/8/26 11:03 AM, Baolin Wang wrote: > > > > > > > > > > > > > > > > > > On 9/8/26 10:34 AM, Barry Song wrote: > > > > > > > On Tue, Sep 8, 2026 at 10:30 AM Baoquan He <baoquan.he@linux.dev> wrote: > > > > > > > > > > > > > > > > Hi Baolin, > > > > > > > > > > > > > > > > On 09/07/26 at 11:25am, Baolin Wang wrote: > > > > > > > > ......snip... > > > > > > > > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > > > > > > > > index 40d3f1b48a74..42c0a09938ab 100644 > > > > > > > > > --- a/mm/vmscan.c > > > > > > > > > +++ b/mm/vmscan.c > > > > > > > > > > > > > > > > Well, this seems to be based on Andrew's mm-new branch. I usually track > > > > > > > > mm-unstable branch. Maybe the subject should be marked as below? > > > > > > > > [PATCH mm-new] mm: mglru: clear the reference counter for rejected > > > > > > > > > > > > ACK. > > > > > > > > > > > > > > > > > > > > > > > @@ -5021,10 +5021,11 @@ static int evict_folios(unsigned > > > > > > > > > long nr_to_scan, struct lruvec *lruvec, > > > > > > > > > } > > > > > > > > > > > > > > > > > > /* don't add rejected folios to the oldest generation */ > > > > > > > > > - if (lru_gen_folio_seq(lruvec, folio, false) == > > > > > > > > > min_seq[type]) { > > > > > > > > > - folio_set_lru_refs(folio, 0); > > > > > > > > > + if (lru_gen_folio_seq(lruvec, folio, false) == > > > > > > > > > min_seq[type]) > > > > > > > > > folio_set_active(folio); > > > > > > > > > - } > > > > > > > > > + > > > > > > > > > + /* See the comments on LRU_REFS_FLAGS */ > > > > > > > > > + folio_set_lru_refs(folio, 0); > > > > > > > > > > > > > > > > This looks like a great catch, while the code change could bring issue. > > > > > > > > > > > > > > > > Because move_folios_to_lru() relies on folios' flags to decide their new > > > > > > > > generation. You just cleared it before move_folios_to_lru(). This is no > > > > > > > > problem for rejected folios that are determined to be put into the > > > > > > > > oldest generation. But for those rejected folios that are determined to > > > > > > > > be promoted, this could be wrong. E.g currently gen window is 4, and a > > > > > > > > folio is referenced, lru_gen_folio_seq() decides its new gen as 1, which > > > > > > > > is the 2nd oldest generation. While folio_set_lru_refs(folio, 0) clear > > > > > > > > referenced bit, this causes it being put into the oldest generation in > > > > > > > > move_folios_to_lru(), this is not expected. > > > > > > > > > > > > Yes. As I discussed with Barry earlier, lru_gen_folio_seq() also needs > > > > > > to be reconsidered regarding whether it should rely on PG_referenced > > > > > > [1]. > > > > > > > > > > > > For commit 6cbdd9726fb5, we didn't discuss the impact on rejected folios > > > > > > either. Before commit 6cbdd9726fb5, if rejected folios did not have > > > > > > PG_active set by shrink_folio_list(), evict_folios() would set PG_active > > > > > > on these rejected folios. > > > > > > > > > > > > [1] https://lore.kernel.org/linux-mm/20260901220430.79810-1- > > > > > > baohua@kernel.org/ > > > > > > > > > > > > > The original code looks quite weird. It even prioritizes folios > > > > > > > that won't be promoted by `PG_active`. Do we need to change all > > > > > > > the cases just to call `PG_active`? > > > > > > > > I would agree if we can. While I have one concern. A rejected folio that > > > > should have gone into the oldest generation is now being promoted to the > > > > 2nd newest generation. In the original code, referenced folio is only > > > > being promoted to the next gen. I even think this is not a bug, but Yu > > > > > > That's not quite true. Before commit 6cbdd9726fb5, a rejected referenced > > > folio was also put back to the 2nd youngest gen. > > > > OK, I didn't follow your earlier discussion, I need take some time to > > fully understand that commit and the patch thread from Ehab. > > > > > > > > > Zhao intentionally did it: the coldest folio is moved to 2nd newest gen, > > > > referenced folio (hot folio) is moved to new gen but carries the referenced > > > > bit. Both of them seems to be treated somewhat equally. > > > > > > > > To me, I would rather move both of them to the next gen, while keep their > > > > refs untouched. > > > > > > IMHO, I strongly recommend not doing this, and that's exactly the motivation > > > behind my patch. Because this is also being treated as a promotion, it > > > should behave like folio_inc_gen() or folio_update_gen() and clear the refs > > > after promotion. I think this is a fundamental principle of promotion. > > > Otherwise, ref-based promotion is already completely broken. > > > > OK, it makes sense to me to make principle of promotion strictly applied > > no efficiency degradation involved. > > > > > > > > Next, I also plan to clean up refs in lru_gen_set_refs() as discussed with > > > Barry. > > > > Looks forward to seeing that. By the way, your discussion with Baryr is > > private or in public list, do you have pointer if public? Thanks. > > I raised this issue before[1], and recently there has been another > discussion[2] about it (we had some private discussions, but I'll post a new > patch for discussion). > > [1] https://lore.kernel.org/all/eb395442-0aad-428a-a5ac-9072d2d89060@linux.alibaba.com/ > [2] > https://lore.kernel.org/linux-mm/20260901220430.79810-1-baohua@kernel.org/ Got it, thanks. > > > > > > > Yes. Regarding this concern, I plan to change back to the original > > > > > > behavior: > > > > > > > > > > > > /* See the comments on LRU_REFS_FLAGS */ > > > > > > folio_set_lru_refs(folio, 0); > > > > > > > > > > > > /* don't add rejected folios to the oldest generation */ > > > > > > if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) > > > > > > folio_set_active(folio); > > > > > > > > > > > > What do you think? > > > > > > > > > > Just FYI, after above changes, the performance improvement on zram is no > > > > > longer obvious either. I think this also answers Kairui's earlier question > > > > > about why I saw a performance improvement (which seems related to commit > > > > > 6cbdd9726fb5). Also, there is no obvious performance regression either. > > > > > > > > Hi Baolin, > > > > > > > > Not sure if it's convenient to do a little more testing in your side. > > > > E.g rejected folios are moved to next gen, but not clearing their flags. > > > > > > I'm not sure what you mean by "next gen" here. If you mean the 2nd oldest > > > gen, I actually tested that too, that is, clearing refs after > > > move_folios_to_lru(), and there wasn't any noticeable performance impact > > > either (but the code was a bit hacky, so I didn't go with this approach). > > > > Yeah, I meant the 2nd oldest gen, while what I am curious about is moving > > them into 2nd oldest gen but not clearing refs. Clearly it's conflicting > > with your plan. Anyway, it's just a brain store idea, please forget it. > > Thanks for the sharing and detailed explanation. > > Thanks for reviewing. > ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-09-08 10:29 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-07 3:25 [PATCH] mm: mglru: clear the reference counter for rejected folios Baolin Wang 2026-09-07 4:28 ` Barry Song 2026-09-07 5:53 ` Baolin Wang 2026-09-07 4:57 ` Kairui Song 2026-09-07 6:27 ` Baolin Wang 2026-09-08 2:30 ` Baoquan He 2026-09-08 2:34 ` Barry Song 2026-09-08 3:03 ` Baolin Wang 2026-09-08 4:01 ` Baolin Wang 2026-09-08 6:59 ` Baoquan He 2026-09-08 7:53 ` Baolin Wang 2026-09-08 8:23 ` Baoquan He 2026-09-08 9:39 ` Baolin Wang 2026-09-08 10:29 ` Baoquan He
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox