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

Gregory Price posted 5 patches 1 year ago
There is a newer version of this series
[RFC v2 PATCH 4/5] vmstat: add page-cache numa hints
Posted by Gregory Price 1 year ago
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@gourry.net>
---
 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,
+		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
-- 
2.43.0
Re: [RFC v2 PATCH 4/5] vmstat: add page-cache numa hints
Posted by Donet Tom 11 months, 2 weeks ago
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@gourry.net>
> ---
>   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
Re: [RFC v2 PATCH 4/5] vmstat: add page-cache numa hints
Posted by Gregory Price 11 months, 2 weeks ago
On Fri, Jan 03, 2025 at 03:48:24PM +0530, Donet Tom wrote:
> 
> 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@gourry.net>
> > ---
> >   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.
> 

Thank you for catching this, will add to v3.

v3 is coming soon with more microbenchmark info, then hopefully moving
on to some workload testing.

~Gregory
Re: [RFC v2 PATCH 4/5] vmstat: add page-cache numa hints
Posted by Donet Tom 11 months, 3 weeks ago
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@gourry.net>
> ---
>   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,
> +		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));

I have tested this patch series on my system with my test program. I am able
to see unmapped page cache pages are getting promoted.
numa_hint_faults2269numa_hint_faults_local2245numa_hint_page_cache1244numa_hint_page_cache_local0numa_pages_migrated4501

In my test result numa_hint_page_cache_local is 0. I am seeing
numa_hint_page_cache_local will only be incremented if the folio's
node and the process's running node are the same. This condition
does not occur in the current implementation, correct?

Thanks
Donet

>   		*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
Re: [RFC v2 PATCH 4/5] vmstat: add page-cache numa hints
Posted by Gregory Price 11 months, 3 weeks ago
On Fri, Dec 27, 2024 at 04:18:24PM +0530, Donet Tom wrote:
> 
> On 12/11/24 03:07, Gregory Price wrote:
... snip ...
> > +		NUMA_HINT_PAGE_CACHE,
> > +		NUMA_HINT_PAGE_CACHE_LOCAL,
> >   		NUMA_PAGE_MIGRATE,
... snip ...
> >   	if (folio_nid(folio) == numa_node_id()) {
> > -		count_vm_numa_event(NUMA_HINT_FAULTS_LOCAL);
> > +		count_vm_numa_event(NUMA_HINT_TYPE_LOCAL(vmf));
> 
> I have tested this patch series on my system with my test program. I am able
> to see unmapped page cache pages are getting promoted.
> numa_hint_faults2269numa_hint_faults_local2245numa_hint_page_cache1244numa_hint_page_cache_local0numa_pages_migrated4501
> 
> In my test result numa_hint_page_cache_local is 0. I am seeing
> numa_hint_page_cache_local will only be incremented if the folio's
> node and the process's running node are the same. This condition
> does not occur in the current implementation, correct?
> 

I did not want to assume we'd never use this interface where such a
scenario could occur - so i wanted to:

  a) make such a scenario visible
  b) make the code consistent with existing fault counts

I'm fine removing it.  It's hard to know if this interface ever gets
called with that scenario occurringwithout capturing the data.

~Gregory
Re: [RFC v2 PATCH 4/5] vmstat: add page-cache numa hints
Posted by Donet Tom 11 months, 3 weeks ago
On 12/27/24 21:19, Gregory Price wrote:
> On Fri, Dec 27, 2024 at 04:18:24PM +0530, Donet Tom wrote:
>> On 12/11/24 03:07, Gregory Price wrote:
> ... snip ...
>>> +		NUMA_HINT_PAGE_CACHE,
>>> +		NUMA_HINT_PAGE_CACHE_LOCAL,
>>>    		NUMA_PAGE_MIGRATE,
> ... snip ...
>>>    	if (folio_nid(folio) == numa_node_id()) {
>>> -		count_vm_numa_event(NUMA_HINT_FAULTS_LOCAL);
>>> +		count_vm_numa_event(NUMA_HINT_TYPE_LOCAL(vmf));
>> I have tested this patch series on my system with my test program. I am able
>> to see unmapped page cache pages are getting promoted.
>> numa_hint_faults2269numa_hint_faults_local2245numa_hint_page_cache1244numa_hint_page_cache_local0numa_pages_migrated4501
>>
>> In my test result numa_hint_page_cache_local is 0. I am seeing
>> numa_hint_page_cache_local will only be incremented if the folio's
>> node and the process's running node are the same. This condition
>> does not occur in the current implementation, correct?
>>
> I did not want to assume we'd never use this interface where such a
> scenario could occur - so i wanted to:
>
>    a) make such a scenario visible
>    b) make the code consistent with existing fault counts
Understood, thank you for clarifying!
>
> I'm fine removing it.  It's hard to know if this interface ever gets
> called with that scenario occurringwithout capturing the data.
>
> ~Gregory
>