* [PATCH -next] mm: vmscan: correct nr_requested tracing in scan_folios
@ 2025-12-03 9:40 Chen Ridong
2025-12-03 11:33 ` David Hildenbrand (Red Hat)
0 siblings, 1 reply; 6+ messages in thread
From: Chen Ridong @ 2025-12-03 9:40 UTC (permalink / raw)
To: akpm, axelrasmussen, yuanchu, weixugc, hannes, david, mhocko,
zhengqi.arch, shakeel.butt, lorenzo.stoakes, yuzhao,
jaewon31.kim
Cc: linux-mm, linux-kernel, lujialin4, chenridong
From: Chen Ridong <chenridong@huawei.com>
When enabling vmscan tracing, it is observed that nr_requested is always
4096, which is confusing.
mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ...
mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ...
mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ...
mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ...
mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ...
mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ...
mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ...
This is because it prints MAX_LRU_BATCH, which is meaningless as it's a
constant. To fix this, modify it to print nr_to_scan as isolate_lru_folios
does.
Fixes: 8c2214fc9a47 ("mm: multi-gen LRU: reuse some legacy trace events")
Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
mm/vmscan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index fddd168a9737..8cfafd50a7a8 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4601,7 +4601,7 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
count_memcg_events(memcg, item, isolated);
count_memcg_events(memcg, PGREFILL, sorted);
__count_vm_events(PGSCAN_ANON + type, isolated);
- trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, MAX_LRU_BATCH,
+ trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan,
scanned, skipped, isolated,
type ? LRU_INACTIVE_FILE : LRU_INACTIVE_ANON);
if (type == LRU_GEN_FILE)
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH -next] mm: vmscan: correct nr_requested tracing in scan_folios 2025-12-03 9:40 [PATCH -next] mm: vmscan: correct nr_requested tracing in scan_folios Chen Ridong @ 2025-12-03 11:33 ` David Hildenbrand (Red Hat) 2025-12-04 0:46 ` Chen Ridong 2025-12-04 9:05 ` [PATCH -next] mm: vmscan: correct nr_requested tracing in Lance Yang 0 siblings, 2 replies; 6+ messages in thread From: David Hildenbrand (Red Hat) @ 2025-12-03 11:33 UTC (permalink / raw) To: Chen Ridong, akpm, axelrasmussen, yuanchu, weixugc, hannes, mhocko, zhengqi.arch, shakeel.butt, lorenzo.stoakes, yuzhao, jaewon31.kim Cc: linux-mm, linux-kernel, lujialin4, chenridong On 12/3/25 10:40, Chen Ridong wrote: > From: Chen Ridong <chenridong@huawei.com> > > When enabling vmscan tracing, it is observed that nr_requested is always > 4096, which is confusing. > > mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... > mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... > mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... > mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... > mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... > mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... > mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... > > This is because it prints MAX_LRU_BATCH, which is meaningless as it's a > constant. To fix this, modify it to print nr_to_scan as isolate_lru_folios > does. > > Fixes: 8c2214fc9a47 ("mm: multi-gen LRU: reuse some legacy trace events") > Signed-off-by: Chen Ridong <chenridong@huawei.com> > --- > mm/vmscan.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index fddd168a9737..8cfafd50a7a8 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -4601,7 +4601,7 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec, > count_memcg_events(memcg, item, isolated); > count_memcg_events(memcg, PGREFILL, sorted); > __count_vm_events(PGSCAN_ANON + type, isolated); > - trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, MAX_LRU_BATCH, > + trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan, > scanned, skipped, isolated, We do that in isolate_lru_folios(). Given that we do int remaining = min(nr_to_scan, MAX_LRU_BATCH); and effectively cap it, I wonder if we would want to trace that capped valued instead of MAX_LRU_BATCH. -- Cheers David ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] mm: vmscan: correct nr_requested tracing in scan_folios 2025-12-03 11:33 ` David Hildenbrand (Red Hat) @ 2025-12-04 0:46 ` Chen Ridong 2025-12-04 11:54 ` David Hildenbrand (Red Hat) 2025-12-04 9:05 ` [PATCH -next] mm: vmscan: correct nr_requested tracing in Lance Yang 1 sibling, 1 reply; 6+ messages in thread From: Chen Ridong @ 2025-12-04 0:46 UTC (permalink / raw) To: David Hildenbrand (Red Hat), akpm, axelrasmussen, yuanchu, weixugc, hannes, mhocko, zhengqi.arch, shakeel.butt, lorenzo.stoakes, yuzhao, jaewon31.kim Cc: linux-mm, linux-kernel, lujialin4, chenridong On 2025/12/3 19:33, David Hildenbrand (Red Hat) wrote: > On 12/3/25 10:40, Chen Ridong wrote: >> From: Chen Ridong <chenridong@huawei.com> >> >> When enabling vmscan tracing, it is observed that nr_requested is always >> 4096, which is confusing. >> >> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >> >> This is because it prints MAX_LRU_BATCH, which is meaningless as it's a >> constant. To fix this, modify it to print nr_to_scan as isolate_lru_folios >> does. >> >> Fixes: 8c2214fc9a47 ("mm: multi-gen LRU: reuse some legacy trace events") >> Signed-off-by: Chen Ridong <chenridong@huawei.com> >> --- >> mm/vmscan.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/mm/vmscan.c b/mm/vmscan.c >> index fddd168a9737..8cfafd50a7a8 100644 >> --- a/mm/vmscan.c >> +++ b/mm/vmscan.c >> @@ -4601,7 +4601,7 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec, >> count_memcg_events(memcg, item, isolated); >> count_memcg_events(memcg, PGREFILL, sorted); >> __count_vm_events(PGSCAN_ANON + type, isolated); >> - trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, MAX_LRU_BATCH, >> + trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan, >> scanned, skipped, isolated, > > We do that in isolate_lru_folios(). > > Given that we do > > int remaining = min(nr_to_scan, MAX_LRU_BATCH); > > and effectively cap it, I wonder if we would want to trace that capped valued instead of MAX_LRU_BATCH. > I prefer tracing nr_to_scan, as it reflects the original target number of pages we intended to scan. Even if nr_to_scan exceeds MAX_LRU_BATCH, we can still deduce that it was effectively capped by examining the actual scanned, skipped, or isolated counts. However, if we trace min(nr_to_scan, MAX_LRU_BATCH) instead, we would lose visibility into what the original nr_to_scan value was. -- Best regards, Ridong ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] mm: vmscan: correct nr_requested tracing in scan_folios 2025-12-04 0:46 ` Chen Ridong @ 2025-12-04 11:54 ` David Hildenbrand (Red Hat) 2025-12-04 12:19 ` Chen Ridong 0 siblings, 1 reply; 6+ messages in thread From: David Hildenbrand (Red Hat) @ 2025-12-04 11:54 UTC (permalink / raw) To: Chen Ridong, akpm, axelrasmussen, yuanchu, weixugc, hannes, mhocko, zhengqi.arch, shakeel.butt, lorenzo.stoakes, yuzhao, jaewon31.kim Cc: linux-mm, linux-kernel, lujialin4, chenridong On 12/4/25 01:46, Chen Ridong wrote: > > > On 2025/12/3 19:33, David Hildenbrand (Red Hat) wrote: >> On 12/3/25 10:40, Chen Ridong wrote: >>> From: Chen Ridong <chenridong@huawei.com> >>> >>> When enabling vmscan tracing, it is observed that nr_requested is always >>> 4096, which is confusing. >>> >>> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >>> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >>> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >>> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >>> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >>> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >>> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >>> >>> This is because it prints MAX_LRU_BATCH, which is meaningless as it's a >>> constant. To fix this, modify it to print nr_to_scan as isolate_lru_folios >>> does. >>> >>> Fixes: 8c2214fc9a47 ("mm: multi-gen LRU: reuse some legacy trace events") >>> Signed-off-by: Chen Ridong <chenridong@huawei.com> >>> --- >>> mm/vmscan.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/mm/vmscan.c b/mm/vmscan.c >>> index fddd168a9737..8cfafd50a7a8 100644 >>> --- a/mm/vmscan.c >>> +++ b/mm/vmscan.c >>> @@ -4601,7 +4601,7 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec, >>> count_memcg_events(memcg, item, isolated); >>> count_memcg_events(memcg, PGREFILL, sorted); >>> __count_vm_events(PGSCAN_ANON + type, isolated); >>> - trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, MAX_LRU_BATCH, >>> + trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan, >>> scanned, skipped, isolated, >> >> We do that in isolate_lru_folios(). >> >> Given that we do >> >> int remaining = min(nr_to_scan, MAX_LRU_BATCH); >> >> and effectively cap it, I wonder if we would want to trace that capped valued instead of MAX_LRU_BATCH. >> > > I prefer tracing nr_to_scan, as it reflects the original target number of pages we intended to scan. But it's misleading, because we're also tracing "scanned, skipped, isolated", and one might wonder how it relates to nr_to_scan? > Even if nr_to_scan exceeds MAX_LRU_BATCH, we can still deduce that it was effectively capped by > examining the actual scanned, skipped, or isolated counts. However, if we trace min(nr_to_scan, > MAX_LRU_BATCH) instead, we would lose visibility into what the original nr_to_scan value was. Is that really required for the purpose we are tracing here? -- Cheers David ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] mm: vmscan: correct nr_requested tracing in scan_folios 2025-12-04 11:54 ` David Hildenbrand (Red Hat) @ 2025-12-04 12:19 ` Chen Ridong 0 siblings, 0 replies; 6+ messages in thread From: Chen Ridong @ 2025-12-04 12:19 UTC (permalink / raw) To: David Hildenbrand (Red Hat), akpm, axelrasmussen, yuanchu, weixugc, hannes, mhocko, zhengqi.arch, shakeel.butt, lorenzo.stoakes, yuzhao, jaewon31.kim Cc: linux-mm, linux-kernel, lujialin4, chenridong On 2025/12/4 19:54, David Hildenbrand (Red Hat) wrote: > On 12/4/25 01:46, Chen Ridong wrote: >> >> >> On 2025/12/3 19:33, David Hildenbrand (Red Hat) wrote: >>> On 12/3/25 10:40, Chen Ridong wrote: >>>> From: Chen Ridong <chenridong@huawei.com> >>>> >>>> When enabling vmscan tracing, it is observed that nr_requested is always >>>> 4096, which is confusing. >>>> >>>> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >>>> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >>>> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >>>> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >>>> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >>>> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >>>> mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... >>>> >>>> This is because it prints MAX_LRU_BATCH, which is meaningless as it's a >>>> constant. To fix this, modify it to print nr_to_scan as isolate_lru_folios >>>> does. >>>> >>>> Fixes: 8c2214fc9a47 ("mm: multi-gen LRU: reuse some legacy trace events") >>>> Signed-off-by: Chen Ridong <chenridong@huawei.com> >>>> --- >>>> mm/vmscan.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/mm/vmscan.c b/mm/vmscan.c >>>> index fddd168a9737..8cfafd50a7a8 100644 >>>> --- a/mm/vmscan.c >>>> +++ b/mm/vmscan.c >>>> @@ -4601,7 +4601,7 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec, >>>> count_memcg_events(memcg, item, isolated); >>>> count_memcg_events(memcg, PGREFILL, sorted); >>>> __count_vm_events(PGSCAN_ANON + type, isolated); >>>> - trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, MAX_LRU_BATCH, >>>> + trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan, >>>> scanned, skipped, isolated, >>> >>> We do that in isolate_lru_folios(). >>> >>> Given that we do >>> >>> int remaining = min(nr_to_scan, MAX_LRU_BATCH); >>> >>> and effectively cap it, I wonder if we would want to trace that capped valued instead of >>> MAX_LRU_BATCH. >>> >> >> I prefer tracing nr_to_scan, as it reflects the original target number of pages we intended to scan. > > But it's misleading, because we're also tracing "scanned, skipped, isolated", and one might wonder > how it relates to nr_to_scan? > >> Even if nr_to_scan exceeds MAX_LRU_BATCH, we can still deduce that it was effectively capped by >> examining the actual scanned, skipped, or isolated counts. However, if we trace min(nr_to_scan, >> MAX_LRU_BATCH) instead, we would lose visibility into what the original nr_to_scan value was. > > Is that really required for the purpose we are tracing here? > Thank you David, I've seen Lance's response and agree with your point. Using min(nr_to_scan, MAX_LRU_BATCH) would indeed be more appropriate for the trace, as it reflects the actual capped value used during scanning. I'll update the patch accordingly. -- Best regards, Ridong ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] mm: vmscan: correct nr_requested tracing in 2025-12-03 11:33 ` David Hildenbrand (Red Hat) 2025-12-04 0:46 ` Chen Ridong @ 2025-12-04 9:05 ` Lance Yang 1 sibling, 0 replies; 6+ messages in thread From: Lance Yang @ 2025-12-04 9:05 UTC (permalink / raw) To: david Cc: akpm, axelrasmussen, chenridong, chenridong, hannes, jaewon31.kim, linux-kernel, linux-mm, lorenzo.stoakes, lujialin4, mhocko, shakeel.butt, weixugc, yuanchu, yuzhao, zhengqi.arch, Lance Yang From: Lance Yang <lance.yang@linux.dev> On Wed, 3 Dec 2025 12:33:07 +0100, David Hildenbrand (Red Hat) wrote: > On 12/3/25 10:40, Chen Ridong wrote: > > From: Chen Ridong <chenridong@huawei.com> > > > > When enabling vmscan tracing, it is observed that nr_requested is always > > 4096, which is confusing. > > > > mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... > > mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... > > mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... > > mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... > > mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... > > mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... > > mm_vmscan_lru_isolate: classzone=3 order=0 nr_requested=4096 ... > > > > This is because it prints MAX_LRU_BATCH, which is meaningless as it's a > > constant. To fix this, modify it to print nr_to_scan as isolate_lru_folios > > does. > > > > Fixes: 8c2214fc9a47 ("mm: multi-gen LRU: reuse some legacy trace events") > > Signed-off-by: Chen Ridong <chenridong@huawei.com> > > --- > > mm/vmscan.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > index fddd168a9737..8cfafd50a7a8 100644 > > --- a/mm/vmscan.c > > +++ b/mm/vmscan.c > > @@ -4601,7 +4601,7 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec, > > count_memcg_events(memcg, item, isolated); > > count_memcg_events(memcg, PGREFILL, sorted); > > __count_vm_events(PGSCAN_ANON + type, isolated); > > - trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, MAX_LRU_BATCH, > > + trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan, > > scanned, skipped, isolated, > > We do that in isolate_lru_folios(). > > Given that we do > > int remaining = min(nr_to_scan, MAX_LRU_BATCH); > > and effectively cap it, I wonder if we would want to trace that capped > valued instead of MAX_LRU_BATCH. Yeah, since we explicitly clamp the work at MAX_LRU_BATCH, the trace should reflect that reality :) ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-12-04 12:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-12-03 9:40 [PATCH -next] mm: vmscan: correct nr_requested tracing in scan_folios Chen Ridong 2025-12-03 11:33 ` David Hildenbrand (Red Hat) 2025-12-04 0:46 ` Chen Ridong 2025-12-04 11:54 ` David Hildenbrand (Red Hat) 2025-12-04 12:19 ` Chen Ridong 2025-12-04 9:05 ` [PATCH -next] mm: vmscan: correct nr_requested tracing in Lance Yang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox