Re: [RFC v2 PATCH 4/5] vmstat: add page-cache numa hints

[Donet Tom <donettom@xxxxxxxxxxxxx>]

On 12/11/24 03:07, Gregory Price wrote:
> Count non-page-fault events as page-cache numa hints instead of
> fault hints in vmstat. Add a define to select the hint type to
> keep the code clean.
>
> Signed-off-by: Gregory Price <gourry@xxxxxxxxxx>
> ---
>   include/linux/vm_event_item.h | 8 ++++++++
>   mm/memory.c                   | 6 +++---
>   mm/vmstat.c                   | 2 ++
>   3 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/vm_event_item.h b/include/linux/vm_event_item.h
> index f70d0958095c..c5abb0f7cca7 100644
> --- a/include/linux/vm_event_item.h
> +++ b/include/linux/vm_event_item.h
> @@ -63,6 +63,8 @@ enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT,
>   		NUMA_HUGE_PTE_UPDATES,
>   		NUMA_HINT_FAULTS,
>   		NUMA_HINT_FAULTS_LOCAL,
> +		NUMA_HINT_PAGE_CACHE,
Hi Gregory,

While running tests on the patch, I encountered the following warning
message on the console.

[  187.943052] ------------[ cut here ]------------
[  187.943234] __count_memcg_events: missing stat item 49
[  187.943287] WARNING: CPU: 0 PID: 3121 at mm/memcontrol.c:852 
__count_memcg_events+0x3fc/0x42c

The warning occurred because NUMA_HINT_PAGE_CACHE was not added
in memcg_vm_event_stat.

I did below change, Now the warnings are not coming.

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 7b3503d12aaf..fbb360cfea30 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -460,6 +460,7 @@ static const unsigned int memcg_vm_event_stat[] = {
        NUMA_PAGE_MIGRATE,
        NUMA_PTE_UPDATES,
        NUMA_HINT_FAULTS,
+       NUMA_HINT_PAGE_CACHE,
 #endif
 };

Without the change stat output
========================
 #  cat /proc/vmstat  |grep -i numa_hint_page_cache
 numa_hint_page_cache 274
 numa_hint_page_cache_local 0

 #cat /sys/fs/cgroup/memory.stat  |grep -i numa_hint_page_cache
 #

With the change stat output
========================
 # cat /proc/vmstat  |grep -i numa_hint_page_cache
 numa_hint_page_cache 274
 numa_hint_page_cache_local 0
 #
 # cat /sys/fs/cgroup/memory.stat  |grep -i numa_hint_page_cache
 numa_hint_page_cache 274
 #

-Donet
> +		NUMA_HINT_PAGE_CACHE_LOCAL,
>   		NUMA_PAGE_MIGRATE,
>   #endif
>   #ifdef CONFIG_MIGRATION
> @@ -185,6 +187,12 @@ enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT,
>   		NR_VM_EVENT_ITEMS
>   };
>   
> +#ifdef CONFIG_NUMA_BALANCING
> +#define NUMA_HINT_TYPE(vmf) (vmf ? NUMA_HINT_FAULTS : NUMA_HINT_PAGE_CACHE)
> +#define NUMA_HINT_TYPE_LOCAL(vmf) (vmf ? NUMA_HINT_FAULTS_LOCAL : \
> +					 NUMA_HINT_PAGE_CACHE_LOCAL)
> +#endif
> +
>   #ifndef CONFIG_TRANSPARENT_HUGEPAGE
>   #define THP_FILE_ALLOC ({ BUILD_BUG(); 0; })
>   #define THP_FILE_FALLBACK ({ BUILD_BUG(); 0; })
> diff --git a/mm/memory.c b/mm/memory.c
> index af7ba56a4e1e..620e2045af7b 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -5578,10 +5578,10 @@ int numa_migrate_check(struct folio *folio, struct vm_fault *vmf,
>   		*last_cpupid = folio_last_cpupid(folio);
>   
>   #ifdef CONFIG_NUMA_BALANCING
> -	count_vm_numa_event(NUMA_HINT_FAULTS);
> -	count_memcg_folio_events(folio, NUMA_HINT_FAULTS, 1);
> +	count_vm_numa_event(NUMA_HINT_TYPE(vmf));
> +	count_memcg_folio_events(folio, NUMA_HINT_TYPE(vmf), 1);
>   	if (folio_nid(folio) == numa_node_id()) {
> -		count_vm_numa_event(NUMA_HINT_FAULTS_LOCAL);
> +		count_vm_numa_event(NUMA_HINT_TYPE_LOCAL(vmf));
>   		*flags |= TNF_FAULT_LOCAL;
>   	}
>   #endif
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index 4d016314a56c..bcd9be11e957 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -1338,6 +1338,8 @@ const char * const vmstat_text[] = {
>   	"numa_huge_pte_updates",
>   	"numa_hint_faults",
>   	"numa_hint_faults_local",
> +	"numa_hint_page_cache",
> +	"numa_hint_page_cache_local",
>   	"numa_pages_migrated",
>   #endif
>   #ifdef CONFIG_MIGRATION