linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [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
  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

* 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

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