Re: [PATCH RFC 2/4] mm/vmstat: Move pgdemote_* to per-node stats

[Zhijian Li (Fujitsu)]

On 02/11/2023 13:43, Huang, Ying wrote:
> Li Zhijian <lizhijian@xxxxxxxxxxx> writes:
> 
>> This is a prepare to improve the demotion profiling in the later
>> patches.
>>
>> Per-node demotion stats help users to quickly identify which
>> node is in hige stree, and take some special operations if needed.
>>
>> Signed-off-by: Li Zhijian <lizhijian@xxxxxxxxxxx>
>> ---
>>   include/linux/mmzone.h        | 4 ++++
>>   include/linux/vm_event_item.h | 3 ---
>>   mm/vmscan.c                   | 3 ++-
>>   mm/vmstat.c                   | 6 +++---
>>   4 files changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>> index 4106fbc5b4b3..ad0309eea850 100644
>> --- a/include/linux/mmzone.h
>> +++ b/include/linux/mmzone.h
>> @@ -206,6 +206,10 @@ enum node_stat_item {
>>   #ifdef CONFIG_NUMA_BALANCING
>>   	PGPROMOTE_SUCCESS,	/* promote successfully */
>>   	PGPROMOTE_CANDIDATE,	/* candidate pages to promote */
>> +	/* PGDEMOTE_*: pages demoted */
>> +	PGDEMOTE_KSWAPD,
>> +	PGDEMOTE_DIRECT,
>> +	PGDEMOTE_KHUGEPAGED,
>>   #endif
>>   	NR_VM_NODE_STAT_ITEMS
>>   };
>> diff --git a/include/linux/vm_event_item.h b/include/linux/vm_event_item.h
>> index 8abfa1240040..d1b847502f09 100644
>> --- a/include/linux/vm_event_item.h
>> +++ b/include/linux/vm_event_item.h
>> @@ -41,9 +41,6 @@ enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT,
>>   		PGSTEAL_KSWAPD,
>>   		PGSTEAL_DIRECT,
>>   		PGSTEAL_KHUGEPAGED,
>> -		PGDEMOTE_KSWAPD,
>> -		PGDEMOTE_DIRECT,
>> -		PGDEMOTE_KHUGEPAGED,
>>   		PGSCAN_KSWAPD,
>>   		PGSCAN_DIRECT,
>>   		PGSCAN_KHUGEPAGED,
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index 6f13394b112e..2f1fb4ec3235 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -1678,7 +1678,8 @@ static unsigned int demote_folio_list(struct list_head *demote_folios,
>>   		      (unsigned long)&mtc, MIGRATE_ASYNC, MR_DEMOTION,
>>   		      &nr_succeeded);
>>   
>> -	__count_vm_events(PGDEMOTE_KSWAPD + reclaimer_offset(), nr_succeeded);
>> +	mod_node_page_state(NODE_DATA(target_nid),
>> +		    PGDEMOTE_KSWAPD + reclaimer_offset(), nr_succeeded);
> 
> Think again.  It seems that it's better to count demotion event for the
> source node.  Because demotion comes from the memory pressure of the
> source node.  The target node isn't so important.  Do you agree?

Good idea, I will update it.



> 
> --
> Best Regards,
> Huang, Ying
> 
>>   
>>   	return nr_succeeded;
>>   }
>> diff --git a/mm/vmstat.c b/mm/vmstat.c
>> index 00e81e99c6ee..f141c48c39e4 100644
>> --- a/mm/vmstat.c
>> +++ b/mm/vmstat.c
>> @@ -1244,6 +1244,9 @@ const char * const vmstat_text[] = {
>>   #ifdef CONFIG_NUMA_BALANCING
>>   	"pgpromote_success",
>>   	"pgpromote_candidate",
>> +	"pgdemote_kswapd",
>> +	"pgdemote_direct",
>> +	"pgdemote_khugepaged",
>>   #endif
>>   
>>   	/* enum writeback_stat_item counters */
>> @@ -1275,9 +1278,6 @@ const char * const vmstat_text[] = {
>>   	"pgsteal_kswapd",
>>   	"pgsteal_direct",
>>   	"pgsteal_khugepaged",
>> -	"pgdemote_kswapd",
>> -	"pgdemote_direct",
>> -	"pgdemote_khugepaged",
>>   	"pgscan_kswapd",
>>   	"pgscan_direct",
>>   	"pgscan_khugepaged",