[PATCH v4 0/2] mm: add per-order mTHP alloc and swpout counters

[PATCH v4 0/2] mm: add per-order mTHP alloc and swpout counters

[Barry Song]

From: Barry Song <v-songbaohua@oppo.com>

The patchset introduces a framework to facilitate mTHP counters, starting
with the allocation and swap-out counters. Currently, only four new nodes
are appended to the stats directory for each mTHP size.

/sys/kernel/mm/transparent_hugepage/hugepages-<size>/stats
	anon_alloc
	anon_alloc_fallback
	anon_swpout
	anon_swpout_fallback

These four nodes are crucial for us to monitor the fragmentation levels of
both the buddy system and the swapfile. In the future, we may consider
adding additional nodes for further insights.

-v4:
  * Many thanks to David and Ryan for your patience and valuable insights
    throughout the numerous renaming efforts!
  * Guard the case order > PMD_ORDER in count func rather than in callers,
    Ryan;
  * Add swpout counters;
  * Add a helper DEFINE_MTHP_STAT_ATTR to avoid code duplication for various
    counters;

-v3:
  https://lore.kernel.org/linux-mm/20240403035502.71356-1-21cnbao@gmail.com/

Barry Song (2):
  mm: add per-order mTHP anon_alloc and anon_alloc_fallback counters
  mm: add per-order mTHP anon_swpout and anon_swpout_fallback counters

 include/linux/huge_mm.h | 21 +++++++++++++++++
 mm/huge_memory.c        | 52 +++++++++++++++++++++++++++++++++++++++++
 mm/memory.c             |  2 ++
 mm/page_io.c            |  6 ++++-
 mm/vmscan.c             |  3 +++
 5 files changed, 83 insertions(+), 1 deletion(-)

-- 
2.34.1

[PATCH v4 1/2] mm: add per-order mTHP anon_alloc and anon_alloc_fallback counters

[Barry Song]

From: Barry Song <v-songbaohua@oppo.com>

Profiling a system blindly with mTHP has become challenging due to the
lack of visibility into its operations. Presenting the success rate of
mTHP allocations appears to be pressing need.

Recently, I've been experiencing significant difficulty debugging
performance improvements and regressions without these figures.
It's crucial for us to understand the true effectiveness of mTHP in
real-world scenarios, especially in systems with fragmented memory.

This patch sets up the framework for per-order mTHP counters, starting
with the introduction of anon_alloc and anon_alloc_fallback counters.
Incorporating additional counters should now be straightforward as well.

Signed-off-by: Barry Song <v-songbaohua@oppo.com>
---
 include/linux/huge_mm.h | 19 ++++++++++++++++
 mm/huge_memory.c        | 48 +++++++++++++++++++++++++++++++++++++++++
 mm/memory.c             |  2 ++
 3 files changed, 69 insertions(+)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index e896ca4760f6..c5d33017a4dd 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -264,6 +264,25 @@ unsigned long thp_vma_allowable_orders(struct vm_area_struct *vma,
 					  enforce_sysfs, orders);
 }
 
+enum mthp_stat_item {
+	MTHP_STAT_ANON_ALLOC,
+	MTHP_STAT_ANON_ALLOC_FALLBACK,
+	__MTHP_STAT_COUNT
+};
+
+struct mthp_stat {
+	unsigned long stats[PMD_ORDER + 1][__MTHP_STAT_COUNT];
+};
+
+DECLARE_PER_CPU(struct mthp_stat, mthp_stats);
+
+static inline void count_mthp_stat(int order, enum mthp_stat_item item)
+{
+	if (unlikely(order > PMD_ORDER))
+		return;
+	this_cpu_inc(mthp_stats.stats[order][item]);
+}
+
 #define transparent_hugepage_use_zero_page()				\
 	(transparent_hugepage_flags &					\
 	 (1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG))
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 9d4b2fbf6872..5b875f0fc923 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -526,6 +526,46 @@ static const struct kobj_type thpsize_ktype = {
 	.sysfs_ops = &kobj_sysfs_ops,
 };
 
+DEFINE_PER_CPU(struct mthp_stat, mthp_stats) = {{{0}}};
+
+static unsigned long sum_mthp_stat(int order, enum mthp_stat_item item)
+{
+	unsigned long sum = 0;
+	int cpu;
+
+	for_each_online_cpu(cpu) {
+		struct mthp_stat *this = &per_cpu(mthp_stats, cpu);
+
+		sum += this->stats[order][item];
+	}
+
+	return sum;
+}
+
+#define DEFINE_MTHP_STAT_ATTR(_name, _index)					\
+static ssize_t _name##_show(struct kobject *kobj,			\
+			struct kobj_attribute *attr, char *buf)		\
+{									\
+	int order = to_thpsize(kobj)->order;				\
+									\
+	return sysfs_emit(buf, "%lu\n", sum_mthp_stat(order, _index));	\
+}									\
+static struct kobj_attribute _name##_attr = __ATTR_RO(_name)
+
+DEFINE_MTHP_STAT_ATTR(anon_alloc, MTHP_STAT_ANON_ALLOC);
+DEFINE_MTHP_STAT_ATTR(anon_alloc_fallback, MTHP_STAT_ANON_ALLOC_FALLBACK);
+
+static struct attribute *stats_attrs[] = {
+	&anon_alloc_attr.attr,
+	&anon_alloc_fallback_attr.attr,
+	NULL,
+};
+
+static struct attribute_group stats_attr_group = {
+	.name = "stats",
+	.attrs = stats_attrs,
+};
+
 static struct thpsize *thpsize_create(int order, struct kobject *parent)
 {
 	unsigned long size = (PAGE_SIZE << order) / SZ_1K;
@@ -549,6 +589,12 @@ static struct thpsize *thpsize_create(int order, struct kobject *parent)
 		return ERR_PTR(ret);
 	}
 
+	ret = sysfs_create_group(&thpsize->kobj, &stats_attr_group);
+	if (ret) {
+		kobject_put(&thpsize->kobj);
+		return ERR_PTR(ret);
+	}
+
 	thpsize->order = order;
 	return thpsize;
 }
@@ -1050,8 +1096,10 @@ vm_fault_t do_huge_pmd_anonymous_page(struct vm_fault *vmf)
 	folio = vma_alloc_folio(gfp, HPAGE_PMD_ORDER, vma, haddr, true);
 	if (unlikely(!folio)) {
 		count_vm_event(THP_FAULT_FALLBACK);
+		count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_ALLOC_FALLBACK);
 		return VM_FAULT_FALLBACK;
 	}
+	count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_ALLOC);
 	return __do_huge_pmd_anonymous_page(vmf, &folio->page, gfp);
 }
 
diff --git a/mm/memory.c b/mm/memory.c
index 649e3ed94487..1723c8ddf9cb 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4374,8 +4374,10 @@ static struct folio *alloc_anon_folio(struct vm_fault *vmf)
 			}
 			folio_throttle_swaprate(folio, gfp);
 			clear_huge_page(&folio->page, vmf->address, 1 << order);
+			count_mthp_stat(order, MTHP_STAT_ANON_ALLOC);
 			return folio;
 		}
+		count_mthp_stat(order, MTHP_STAT_ANON_ALLOC_FALLBACK);
 next:
 		order = next_order(&orders, order);
 	}
-- 
2.34.1

Re: [PATCH v4 1/2] mm: add per-order mTHP anon_alloc and anon_alloc_fallback counters

[Ryan Roberts]

On 05/04/2024 11:27, Barry Song wrote:
> From: Barry Song <v-songbaohua@oppo.com>
> 
> Profiling a system blindly with mTHP has become challenging due to the
> lack of visibility into its operations. Presenting the success rate of
> mTHP allocations appears to be pressing need.
> 
> Recently, I've been experiencing significant difficulty debugging
> performance improvements and regressions without these figures.
> It's crucial for us to understand the true effectiveness of mTHP in
> real-world scenarios, especially in systems with fragmented memory.
> 
> This patch sets up the framework for per-order mTHP counters, starting
> with the introduction of anon_alloc and anon_alloc_fallback counters.
> Incorporating additional counters should now be straightforward as well.
> 
> Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> ---
>  include/linux/huge_mm.h | 19 ++++++++++++++++
>  mm/huge_memory.c        | 48 +++++++++++++++++++++++++++++++++++++++++
>  mm/memory.c             |  2 ++
>  3 files changed, 69 insertions(+)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index e896ca4760f6..c5d33017a4dd 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -264,6 +264,25 @@ unsigned long thp_vma_allowable_orders(struct vm_area_struct *vma,
>  					  enforce_sysfs, orders);
>  }
>  
> +enum mthp_stat_item {
> +	MTHP_STAT_ANON_ALLOC,
> +	MTHP_STAT_ANON_ALLOC_FALLBACK,
> +	__MTHP_STAT_COUNT
> +};
> +
> +struct mthp_stat {
> +	unsigned long stats[PMD_ORDER + 1][__MTHP_STAT_COUNT];

I saw a fix for this allocation dynamically due to powerpc PMD_ORDER not being
constant. I wonder if ilog2(MAX_PTRS_PER_PTE) would help here?

> +};
> +
> +DECLARE_PER_CPU(struct mthp_stat, mthp_stats);
> +
> +static inline void count_mthp_stat(int order, enum mthp_stat_item item)

I thought we were going to call this always counting up type of stat and event?
"count_mthp_event"? But I'm happy with it as is, personally.

> +{
> +	if (unlikely(order > PMD_ORDER))
> +		return;

I'm wondering if it also makes sense to ignore order == 0? Although I guess if
called for order-0 its safe since the storage exists and sum_mthp_stat() is
never be called for 0. Ignore this comment :)

> +	this_cpu_inc(mthp_stats.stats[order][item]);
> +}
> +
>  #define transparent_hugepage_use_zero_page()				\
>  	(transparent_hugepage_flags &					\
>  	 (1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG))
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 9d4b2fbf6872..5b875f0fc923 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -526,6 +526,46 @@ static const struct kobj_type thpsize_ktype = {
>  	.sysfs_ops = &kobj_sysfs_ops,
>  };
>  
> +DEFINE_PER_CPU(struct mthp_stat, mthp_stats) = {{{0}}};
> +
> +static unsigned long sum_mthp_stat(int order, enum mthp_stat_item item)
> +{
> +	unsigned long sum = 0;
> +	int cpu;
> +
> +	for_each_online_cpu(cpu) {

What happens if a cpu that was online and collected a bunch of stats gets
offlined? The user will see stats get smaller?

Perhaps this should be for_each_possible_cpu()? Although I'm not sure what
happens to percpu data when a cpu goes offline? Is the data preserved? Or wiped,
or unmapped? dunno. Might we need to rescue stats into a global counter at
offline-time?

> +		struct mthp_stat *this = &per_cpu(mthp_stats, cpu);
> +
> +		sum += this->stats[order][item];
> +	}
> +
> +	return sum;
> +}
> +
> +#define DEFINE_MTHP_STAT_ATTR(_name, _index)					\
> +static ssize_t _name##_show(struct kobject *kobj,			\
> +			struct kobj_attribute *attr, char *buf)		\
> +{									\
> +	int order = to_thpsize(kobj)->order;				\
> +									\
> +	return sysfs_emit(buf, "%lu\n", sum_mthp_stat(order, _index));	\
> +}									\
> +static struct kobj_attribute _name##_attr = __ATTR_RO(_name)

Very nice!

> +
> +DEFINE_MTHP_STAT_ATTR(anon_alloc, MTHP_STAT_ANON_ALLOC);
> +DEFINE_MTHP_STAT_ATTR(anon_alloc_fallback, MTHP_STAT_ANON_ALLOC_FALLBACK);
> +
> +static struct attribute *stats_attrs[] = {
> +	&anon_alloc_attr.attr,
> +	&anon_alloc_fallback_attr.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group stats_attr_group = {
> +	.name = "stats",
> +	.attrs = stats_attrs,
> +};
> +
>  static struct thpsize *thpsize_create(int order, struct kobject *parent)
>  {
>  	unsigned long size = (PAGE_SIZE << order) / SZ_1K;
> @@ -549,6 +589,12 @@ static struct thpsize *thpsize_create(int order, struct kobject *parent)
>  		return ERR_PTR(ret);
>  	}
>  
> +	ret = sysfs_create_group(&thpsize->kobj, &stats_attr_group);
> +	if (ret) {
> +		kobject_put(&thpsize->kobj);
> +		return ERR_PTR(ret);
> +	}
> +
>  	thpsize->order = order;
>  	return thpsize;
>  }
> @@ -1050,8 +1096,10 @@ vm_fault_t do_huge_pmd_anonymous_page(struct vm_fault *vmf)
>  	folio = vma_alloc_folio(gfp, HPAGE_PMD_ORDER, vma, haddr, true);
>  	if (unlikely(!folio)) {
>  		count_vm_event(THP_FAULT_FALLBACK);
> +		count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_ALLOC_FALLBACK);

I think we should aim for the PMD-oder MTHP_STAT_ANON_ALLOC and
MTHP_STAT_ANON_ALLOC_FALLBACK to match THP_FAULT_ALLOC and THP_FAULT_FALLBACK.
Its not currently setup this way...


>  		return VM_FAULT_FALLBACK;
>  	}
> +	count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_ALLOC);
>  	return __do_huge_pmd_anonymous_page(vmf, &folio->page, gfp);
>  }
>  
> diff --git a/mm/memory.c b/mm/memory.c
> index 649e3ed94487..1723c8ddf9cb 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4374,8 +4374,10 @@ static struct folio *alloc_anon_folio(struct vm_fault *vmf)
>  			}
>  			folio_throttle_swaprate(folio, gfp);
>  			clear_huge_page(&folio->page, vmf->address, 1 << order);
> +			count_mthp_stat(order, MTHP_STAT_ANON_ALLOC);
>  			return folio;
>  		}
> +		count_mthp_stat(order, MTHP_STAT_ANON_ALLOC_FALLBACK);

...And we should follow the usage same pattern for the smaller mTHP here too.
Which means MTHP_STAT_ANON_ALLOC_FALLBACK should be after the next: label. We
could introduce a MTHP_STAT_ANON_ALLOC_FALLBACK_CHARGE which would only trigger
on a fallback due to charge failure, just like THP_FAULT_FALLBACK_CHARGE?

>  next:
>  		order = next_order(&orders, order);
>  	}

Re: [PATCH v4 1/2] mm: add per-order mTHP anon_alloc and anon_alloc_fallback counters

[Barry Song]

On Fri, Apr 12, 2024 at 4:38 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>
> On 05/04/2024 11:27, Barry Song wrote:
> > From: Barry Song <v-songbaohua@oppo.com>
> >
> > Profiling a system blindly with mTHP has become challenging due to the
> > lack of visibility into its operations. Presenting the success rate of
> > mTHP allocations appears to be pressing need.
> >
> > Recently, I've been experiencing significant difficulty debugging
> > performance improvements and regressions without these figures.
> > It's crucial for us to understand the true effectiveness of mTHP in
> > real-world scenarios, especially in systems with fragmented memory.
> >
> > This patch sets up the framework for per-order mTHP counters, starting
> > with the introduction of anon_alloc and anon_alloc_fallback counters.
> > Incorporating additional counters should now be straightforward as well.
> >
> > Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> > ---
> >  include/linux/huge_mm.h | 19 ++++++++++++++++
> >  mm/huge_memory.c        | 48 +++++++++++++++++++++++++++++++++++++++++
> >  mm/memory.c             |  2 ++
> >  3 files changed, 69 insertions(+)
> >
> > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> > index e896ca4760f6..c5d33017a4dd 100644
> > --- a/include/linux/huge_mm.h
> > +++ b/include/linux/huge_mm.h
> > @@ -264,6 +264,25 @@ unsigned long thp_vma_allowable_orders(struct vm_area_struct *vma,
> >                                         enforce_sysfs, orders);
> >  }
> >
> > +enum mthp_stat_item {
> > +     MTHP_STAT_ANON_ALLOC,
> > +     MTHP_STAT_ANON_ALLOC_FALLBACK,
> > +     __MTHP_STAT_COUNT
> > +};
> > +
> > +struct mthp_stat {
> > +     unsigned long stats[PMD_ORDER + 1][__MTHP_STAT_COUNT];
>
> I saw a fix for this allocation dynamically due to powerpc PMD_ORDER not being
> constant. I wonder if ilog2(MAX_PTRS_PER_PTE) would help here?
>

It's a possibility. However, since we've passed all the build tests
using dynamic
allocation, it might not be worth the effort to attempt static
allocation again. Who
knows what will happen next :-)

> > +};
> > +
> > +DECLARE_PER_CPU(struct mthp_stat, mthp_stats);
> > +
> > +static inline void count_mthp_stat(int order, enum mthp_stat_item item)
>
> I thought we were going to call this always counting up type of stat and event?
> "count_mthp_event"? But I'm happy with it as is, personally.
>
> > +{
> > +     if (unlikely(order > PMD_ORDER))
> > +             return;
>
> I'm wondering if it also makes sense to ignore order == 0? Although I guess if
> called for order-0 its safe since the storage exists and sum_mthp_stat() is
> never be called for 0. Ignore this comment :)

Agreed. I'd like to change it to ignore oder 0;

>
> > +     this_cpu_inc(mthp_stats.stats[order][item]);
> > +}
> > +
> >  #define transparent_hugepage_use_zero_page()                         \
> >       (transparent_hugepage_flags &                                   \
> >        (1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG))
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 9d4b2fbf6872..5b875f0fc923 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -526,6 +526,46 @@ static const struct kobj_type thpsize_ktype = {
> >       .sysfs_ops = &kobj_sysfs_ops,
> >  };
> >
> > +DEFINE_PER_CPU(struct mthp_stat, mthp_stats) = {{{0}}};
> > +
> > +static unsigned long sum_mthp_stat(int order, enum mthp_stat_item item)
> > +{
> > +     unsigned long sum = 0;
> > +     int cpu;
> > +
> > +     for_each_online_cpu(cpu) {
>
> What happens if a cpu that was online and collected a bunch of stats gets
> offlined? The user will see stats get smaller?
>
> Perhaps this should be for_each_possible_cpu()? Although I'm not sure what
> happens to percpu data when a cpu goes offline? Is the data preserved? Or wiped,
> or unmapped? dunno. Might we need to rescue stats into a global counter at
> offline-time?

Good catch. I see /proc/vmstat is always using the  for_each_online_cpu() but it
doesn't have the issue, but mTHP counters do have the problem.

* step 1: cat  the current thp_swpout value before running a test
program which does
swpout;

/ # cat /proc/vmstat | grep thp_swpout
thp_swpout 0
/ # cat /sys/kernel/mm/transparent_hugepage/hugepages-2048kB/stats/anon_swpout
0

* step 2: run the test program on cpu2;

/ # taskset -c 2 /home/barry/develop/linux/swpcache-2m

* step 3: cat the current thp_swpout value after running a test
program which does
swpout;

/ # cat /proc/vmstat | grep thp_swpout
thp_swpout 98
/ # cat /sys/kernel/mm/transparent_hugepage/hugepages-2048kB/stats/anon_swpout
98

*step 4: offline cpu2 and read thp_swpout;

/ # echo 0 > /sys/devices/system/cpu/cpu2/online
[  339.058661] psci: CPU2 killed (polled 0 ms)

/ # cat /proc/vmstat | grep thp_swpout
thp_swpout 98
/ # cat /sys/kernel/mm/transparent_hugepage/hugepages-2048kB/stats/anon_swpout
0

*step 5: online cpu2 and read thp_swpout

/ # echo 1 > /sys/devices/system/cpu/cpu2/online
[  791.642058] CPU2: Booted secondary processor 0x0000000002 [0x000f0510]

/ # cat /proc/vmstat | grep thp_swpout
thp_swpout 98
/ # cat /sys/kernel/mm/transparent_hugepage/hugepages-2048kB/stats/anon_swpout
98



As you can see, in step 4, /proc/vmstat is all good but mTHP counters become
zero.

The reason is /proc/vmstat will fold the offline cpu to an online cpu
but mthp counters lack
it:

/*
 * Fold the foreign cpu events into our own.
 *
 * This is adding to the events on one processor
 * but keeps the global counts constant.
 */
void vm_events_fold_cpu(int cpu)
{
        struct vm_event_state *fold_state = &per_cpu(vm_event_states, cpu);
        int i;

        for (i = 0; i < NR_VM_EVENT_ITEMS; i++) {
                count_vm_events(i, fold_state->event[i]);
                fold_state->event[i] = 0;
        }
}

static int page_alloc_cpu_dead(unsigned int cpu)
{
        ...
        /*
         * Spill the event counters of the dead processor
         * into the current processors event counters.
         * This artificially elevates the count of the current
         * processor.
         */
        vm_events_fold_cpu(cpu);
        ...

        return 0;
}

So I will do the same thing for mTHP counters - fold offline cpu
counters to online one.

>
> > +             struct mthp_stat *this = &per_cpu(mthp_stats, cpu);
> > +
> > +             sum += this->stats[order][item];
> > +     }
> > +
> > +     return sum;
> > +}
> > +
> > +#define DEFINE_MTHP_STAT_ATTR(_name, _index)                                 \
> > +static ssize_t _name##_show(struct kobject *kobj,                    \
> > +                     struct kobj_attribute *attr, char *buf)         \
> > +{                                                                    \
> > +     int order = to_thpsize(kobj)->order;                            \
> > +                                                                     \
> > +     return sysfs_emit(buf, "%lu\n", sum_mthp_stat(order, _index));  \
> > +}                                                                    \
> > +static struct kobj_attribute _name##_attr = __ATTR_RO(_name)
>
> Very nice!

Right. I got duplicated copy-paste and bad small in code so I wrote this macro.

>
> > +
> > +DEFINE_MTHP_STAT_ATTR(anon_alloc, MTHP_STAT_ANON_ALLOC);
> > +DEFINE_MTHP_STAT_ATTR(anon_alloc_fallback, MTHP_STAT_ANON_ALLOC_FALLBACK);
> > +
> > +static struct attribute *stats_attrs[] = {
> > +     &anon_alloc_attr.attr,
> > +     &anon_alloc_fallback_attr.attr,
> > +     NULL,
> > +};
> > +
> > +static struct attribute_group stats_attr_group = {
> > +     .name = "stats",
> > +     .attrs = stats_attrs,
> > +};
> > +
> >  static struct thpsize *thpsize_create(int order, struct kobject *parent)
> >  {
> >       unsigned long size = (PAGE_SIZE << order) / SZ_1K;
> > @@ -549,6 +589,12 @@ static struct thpsize *thpsize_create(int order, struct kobject *parent)
> >               return ERR_PTR(ret);
> >       }
> >
> > +     ret = sysfs_create_group(&thpsize->kobj, &stats_attr_group);
> > +     if (ret) {
> > +             kobject_put(&thpsize->kobj);
> > +             return ERR_PTR(ret);
> > +     }
> > +
> >       thpsize->order = order;
> >       return thpsize;
> >  }
> > @@ -1050,8 +1096,10 @@ vm_fault_t do_huge_pmd_anonymous_page(struct vm_fault *vmf)
> >       folio = vma_alloc_folio(gfp, HPAGE_PMD_ORDER, vma, haddr, true);
> >       if (unlikely(!folio)) {
> >               count_vm_event(THP_FAULT_FALLBACK);
> > +             count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_ALLOC_FALLBACK);
>
> I think we should aim for the PMD-oder MTHP_STAT_ANON_ALLOC and
> MTHP_STAT_ANON_ALLOC_FALLBACK to match THP_FAULT_ALLOC and THP_FAULT_FALLBACK.
> Its not currently setup this way...

right. I also realized this and asked for your comments on this in another
thread.

>
>
> >               return VM_FAULT_FALLBACK;
> >       }
> > +     count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_ALLOC);
> >       return __do_huge_pmd_anonymous_page(vmf, &folio->page, gfp);
> >  }
> >
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 649e3ed94487..1723c8ddf9cb 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -4374,8 +4374,10 @@ static struct folio *alloc_anon_folio(struct vm_fault *vmf)
> >                       }
> >                       folio_throttle_swaprate(folio, gfp);
> >                       clear_huge_page(&folio->page, vmf->address, 1 << order);
> > +                     count_mthp_stat(order, MTHP_STAT_ANON_ALLOC);
> >                       return folio;
> >               }
> > +             count_mthp_stat(order, MTHP_STAT_ANON_ALLOC_FALLBACK);
>
> ...And we should follow the usage same pattern for the smaller mTHP here too.
> Which means MTHP_STAT_ANON_ALLOC_FALLBACK should be after the next: label. We

The only difference is the case

if (mem_cgroup_charge(folio, vma->vm_mm, gfp))
      goto next;

but vmstat is counting this as fallback so i feel good to move after next,

        if (mem_cgroup_charge(folio, vma->vm_mm, gfp)) {
                folio_put(folio);
                count_vm_event(THP_FAULT_FALLBACK);
                count_vm_event(THP_FAULT_FALLBACK_CHARGE);
                return VM_FAULT_FALLBACK;
        }

> could introduce a MTHP_STAT_ANON_ALLOC_FALLBACK_CHARGE which would only trigger
> on a fallback due to charge failure, just like THP_FAULT_FALLBACK_CHARGE?

it is fine to add this THP_FAULT_FALLBACK_CHARGE though it is not that
useful for profiling buddy fragmentation.

>
> >  next:
> >               order = next_order(&orders, order);
> >       }
>

Thanks
Barry

Re: [PATCH v4 1/2] mm: add per-order mTHP anon_alloc and anon_alloc_fallback counters

[Ryan Roberts]

On 11/04/2024 23:40, Barry Song wrote:
> On Fri, Apr 12, 2024 at 4:38 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>>
>> On 05/04/2024 11:27, Barry Song wrote:
>>> From: Barry Song <v-songbaohua@oppo.com>
>>>
>>> Profiling a system blindly with mTHP has become challenging due to the
>>> lack of visibility into its operations. Presenting the success rate of
>>> mTHP allocations appears to be pressing need.
>>>
>>> Recently, I've been experiencing significant difficulty debugging
>>> performance improvements and regressions without these figures.
>>> It's crucial for us to understand the true effectiveness of mTHP in
>>> real-world scenarios, especially in systems with fragmented memory.
>>>
>>> This patch sets up the framework for per-order mTHP counters, starting
>>> with the introduction of anon_alloc and anon_alloc_fallback counters.
>>> Incorporating additional counters should now be straightforward as well.
>>>
>>> Signed-off-by: Barry Song <v-songbaohua@oppo.com>
>>> ---
>>>  include/linux/huge_mm.h | 19 ++++++++++++++++
>>>  mm/huge_memory.c        | 48 +++++++++++++++++++++++++++++++++++++++++
>>>  mm/memory.c             |  2 ++
>>>  3 files changed, 69 insertions(+)
>>>
>>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>>> index e896ca4760f6..c5d33017a4dd 100644
>>> --- a/include/linux/huge_mm.h
>>> +++ b/include/linux/huge_mm.h
>>> @@ -264,6 +264,25 @@ unsigned long thp_vma_allowable_orders(struct vm_area_struct *vma,
>>>                                         enforce_sysfs, orders);
>>>  }
>>>
>>> +enum mthp_stat_item {
>>> +     MTHP_STAT_ANON_ALLOC,
>>> +     MTHP_STAT_ANON_ALLOC_FALLBACK,
>>> +     __MTHP_STAT_COUNT
>>> +};
>>> +
>>> +struct mthp_stat {
>>> +     unsigned long stats[PMD_ORDER + 1][__MTHP_STAT_COUNT];
>>
>> I saw a fix for this allocation dynamically due to powerpc PMD_ORDER not being
>> constant. I wonder if ilog2(MAX_PTRS_PER_PTE) would help here?
>>
> 
> It's a possibility. However, since we've passed all the build tests
> using dynamic
> allocation, it might not be worth the effort to attempt static
> allocation again. Who
> knows what will happen next :-)

If the dynamic version is clear and obvious then fair enough. I tried doing
something similar for the swap-out series but it turned out a mess, so ended up
falling back to static allocation which was much easier to understand.

> 
>>> +};
>>> +
>>> +DECLARE_PER_CPU(struct mthp_stat, mthp_stats);
>>> +
>>> +static inline void count_mthp_stat(int order, enum mthp_stat_item item)
>>
>> I thought we were going to call this always counting up type of stat and event?
>> "count_mthp_event"? But I'm happy with it as is, personally.
>>
>>> +{
>>> +     if (unlikely(order > PMD_ORDER))
>>> +             return;
>>
>> I'm wondering if it also makes sense to ignore order == 0? Although I guess if
>> called for order-0 its safe since the storage exists and sum_mthp_stat() is
>> never be called for 0. Ignore this comment :)
> 
> Agreed. I'd like to change it to ignore oder 0;
> 
>>
>>> +     this_cpu_inc(mthp_stats.stats[order][item]);
>>> +}
>>> +
>>>  #define transparent_hugepage_use_zero_page()                         \
>>>       (transparent_hugepage_flags &                                   \
>>>        (1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG))
>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>>> index 9d4b2fbf6872..5b875f0fc923 100644
>>> --- a/mm/huge_memory.c
>>> +++ b/mm/huge_memory.c
>>> @@ -526,6 +526,46 @@ static const struct kobj_type thpsize_ktype = {
>>>       .sysfs_ops = &kobj_sysfs_ops,
>>>  };
>>>
>>> +DEFINE_PER_CPU(struct mthp_stat, mthp_stats) = {{{0}}};
>>> +
>>> +static unsigned long sum_mthp_stat(int order, enum mthp_stat_item item)
>>> +{
>>> +     unsigned long sum = 0;
>>> +     int cpu;
>>> +
>>> +     for_each_online_cpu(cpu) {
>>
>> What happens if a cpu that was online and collected a bunch of stats gets
>> offlined? The user will see stats get smaller?
>>
>> Perhaps this should be for_each_possible_cpu()? Although I'm not sure what
>> happens to percpu data when a cpu goes offline? Is the data preserved? Or wiped,
>> or unmapped? dunno. Might we need to rescue stats into a global counter at
>> offline-time?
> 
> Good catch. I see /proc/vmstat is always using the  for_each_online_cpu() but it
> doesn't have the issue, but mTHP counters do have the problem.
> 
> * step 1: cat  the current thp_swpout value before running a test
> program which does
> swpout;
> 
> / # cat /proc/vmstat | grep thp_swpout
> thp_swpout 0
> / # cat /sys/kernel/mm/transparent_hugepage/hugepages-2048kB/stats/anon_swpout
> 0
> 
> * step 2: run the test program on cpu2;
> 
> / # taskset -c 2 /home/barry/develop/linux/swpcache-2m
> 
> * step 3: cat the current thp_swpout value after running a test
> program which does
> swpout;
> 
> / # cat /proc/vmstat | grep thp_swpout
> thp_swpout 98
> / # cat /sys/kernel/mm/transparent_hugepage/hugepages-2048kB/stats/anon_swpout
> 98
> 
> *step 4: offline cpu2 and read thp_swpout;
> 
> / # echo 0 > /sys/devices/system/cpu/cpu2/online
> [  339.058661] psci: CPU2 killed (polled 0 ms)
> 
> / # cat /proc/vmstat | grep thp_swpout
> thp_swpout 98
> / # cat /sys/kernel/mm/transparent_hugepage/hugepages-2048kB/stats/anon_swpout
> 0
> 
> *step 5: online cpu2 and read thp_swpout
> 
> / # echo 1 > /sys/devices/system/cpu/cpu2/online
> [  791.642058] CPU2: Booted secondary processor 0x0000000002 [0x000f0510]
> 
> / # cat /proc/vmstat | grep thp_swpout
> thp_swpout 98
> / # cat /sys/kernel/mm/transparent_hugepage/hugepages-2048kB/stats/anon_swpout
> 98
> 
> 
> 
> As you can see, in step 4, /proc/vmstat is all good but mTHP counters become
> zero.
> 
> The reason is /proc/vmstat will fold the offline cpu to an online cpu
> but mthp counters lack
> it:
> 
> /*
>  * Fold the foreign cpu events into our own.
>  *
>  * This is adding to the events on one processor
>  * but keeps the global counts constant.
>  */
> void vm_events_fold_cpu(int cpu)
> {
>         struct vm_event_state *fold_state = &per_cpu(vm_event_states, cpu);
>         int i;
> 
>         for (i = 0; i < NR_VM_EVENT_ITEMS; i++) {
>                 count_vm_events(i, fold_state->event[i]);
>                 fold_state->event[i] = 0;
>         }
> }
> 
> static int page_alloc_cpu_dead(unsigned int cpu)
> {
>         ...
>         /*
>          * Spill the event counters of the dead processor
>          * into the current processors event counters.
>          * This artificially elevates the count of the current
>          * processor.
>          */
>         vm_events_fold_cpu(cpu);
>         ...
> 
>         return 0;
> }
> 
> So I will do the same thing for mTHP counters - fold offline cpu
> counters to online one.

That all looks like a complete mess - better avoided if possible! A quick search
for "for_each_possible_cpu" shows loads of places where code is iterating over
all *possible* cpus and grabbing its per-cpu data. So the data definitely
remains accessible when the cpu is offline. Looks like it doesn't get wiped either.

So can't you just change your sum function to iterate over all possible cpus?

> 
>>
>>> +             struct mthp_stat *this = &per_cpu(mthp_stats, cpu);
>>> +
>>> +             sum += this->stats[order][item];
>>> +     }
>>> +
>>> +     return sum;
>>> +}
>>> +
>>> +#define DEFINE_MTHP_STAT_ATTR(_name, _index)                                 \
>>> +static ssize_t _name##_show(struct kobject *kobj,                    \
>>> +                     struct kobj_attribute *attr, char *buf)         \
>>> +{                                                                    \
>>> +     int order = to_thpsize(kobj)->order;                            \
>>> +                                                                     \
>>> +     return sysfs_emit(buf, "%lu\n", sum_mthp_stat(order, _index));  \
>>> +}                                                                    \
>>> +static struct kobj_attribute _name##_attr = __ATTR_RO(_name)
>>
>> Very nice!
> 
> Right. I got duplicated copy-paste and bad small in code so I wrote this macro.
> 
>>
>>> +
>>> +DEFINE_MTHP_STAT_ATTR(anon_alloc, MTHP_STAT_ANON_ALLOC);
>>> +DEFINE_MTHP_STAT_ATTR(anon_alloc_fallback, MTHP_STAT_ANON_ALLOC_FALLBACK);
>>> +
>>> +static struct attribute *stats_attrs[] = {
>>> +     &anon_alloc_attr.attr,
>>> +     &anon_alloc_fallback_attr.attr,
>>> +     NULL,
>>> +};
>>> +
>>> +static struct attribute_group stats_attr_group = {
>>> +     .name = "stats",
>>> +     .attrs = stats_attrs,
>>> +};
>>> +
>>>  static struct thpsize *thpsize_create(int order, struct kobject *parent)
>>>  {
>>>       unsigned long size = (PAGE_SIZE << order) / SZ_1K;
>>> @@ -549,6 +589,12 @@ static struct thpsize *thpsize_create(int order, struct kobject *parent)
>>>               return ERR_PTR(ret);
>>>       }
>>>
>>> +     ret = sysfs_create_group(&thpsize->kobj, &stats_attr_group);
>>> +     if (ret) {
>>> +             kobject_put(&thpsize->kobj);
>>> +             return ERR_PTR(ret);
>>> +     }
>>> +
>>>       thpsize->order = order;
>>>       return thpsize;
>>>  }
>>> @@ -1050,8 +1096,10 @@ vm_fault_t do_huge_pmd_anonymous_page(struct vm_fault *vmf)
>>>       folio = vma_alloc_folio(gfp, HPAGE_PMD_ORDER, vma, haddr, true);
>>>       if (unlikely(!folio)) {
>>>               count_vm_event(THP_FAULT_FALLBACK);
>>> +             count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_ALLOC_FALLBACK);
>>
>> I think we should aim for the PMD-oder MTHP_STAT_ANON_ALLOC and
>> MTHP_STAT_ANON_ALLOC_FALLBACK to match THP_FAULT_ALLOC and THP_FAULT_FALLBACK.
>> Its not currently setup this way...
> 
> right. I also realized this and asked for your comments on this in another
> thread.

Ahh sorry - must have missed that.

> 
>>
>>
>>>               return VM_FAULT_FALLBACK;
>>>       }
>>> +     count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_ALLOC);
>>>       return __do_huge_pmd_anonymous_page(vmf, &folio->page, gfp);
>>>  }
>>>
>>> diff --git a/mm/memory.c b/mm/memory.c
>>> index 649e3ed94487..1723c8ddf9cb 100644
>>> --- a/mm/memory.c
>>> +++ b/mm/memory.c
>>> @@ -4374,8 +4374,10 @@ static struct folio *alloc_anon_folio(struct vm_fault *vmf)
>>>                       }
>>>                       folio_throttle_swaprate(folio, gfp);
>>>                       clear_huge_page(&folio->page, vmf->address, 1 << order);
>>> +                     count_mthp_stat(order, MTHP_STAT_ANON_ALLOC);
>>>                       return folio;
>>>               }
>>> +             count_mthp_stat(order, MTHP_STAT_ANON_ALLOC_FALLBACK);
>>
>> ...And we should follow the usage same pattern for the smaller mTHP here too.
>> Which means MTHP_STAT_ANON_ALLOC_FALLBACK should be after the next: label. We
> 
> The only difference is the case
> 
> if (mem_cgroup_charge(folio, vma->vm_mm, gfp))
>       goto next;
> 
> but vmstat is counting this as fallback so i feel good to move after next,
> 
>         if (mem_cgroup_charge(folio, vma->vm_mm, gfp)) {
>                 folio_put(folio);
>                 count_vm_event(THP_FAULT_FALLBACK);
>                 count_vm_event(THP_FAULT_FALLBACK_CHARGE);
>                 return VM_FAULT_FALLBACK;
>         }
> 
>> could introduce a MTHP_STAT_ANON_ALLOC_FALLBACK_CHARGE which would only trigger
>> on a fallback due to charge failure, just like THP_FAULT_FALLBACK_CHARGE?
> 
> it is fine to add this THP_FAULT_FALLBACK_CHARGE though it is not that
> useful for profiling buddy fragmentation.

Well I thought you were interested in isolating fallback due to fragmentation
only. You would get that with (FAULT_FALLBACK - FAULT_FALLBACK_CHARGE)? But if
you think the latter will be relatively small/unimportant for now, and therefore
FAULT_FALLBACK will give good enough approximation on its own, then I'm happy
not to add FAULT_FALLBACK_CHARGE for now.

> 
>>
>>>  next:
>>>               order = next_order(&orders, order);
>>>       }
>>
> 
> Thanks
> Barry

Re: [PATCH v4 1/2] mm: add per-order mTHP anon_alloc and anon_alloc_fallback counters

[Barry Song]

On Fri, Apr 12, 2024 at 9:16 PM Ryan Roberts <ryan.roberts@arm.com> wrote:
>
> On 11/04/2024 23:40, Barry Song wrote:
> > On Fri, Apr 12, 2024 at 4:38 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
> >>
> >> On 05/04/2024 11:27, Barry Song wrote:
> >>> From: Barry Song <v-songbaohua@oppo.com>
> >>>
> >>> Profiling a system blindly with mTHP has become challenging due to the
> >>> lack of visibility into its operations. Presenting the success rate of
> >>> mTHP allocations appears to be pressing need.
> >>>
> >>> Recently, I've been experiencing significant difficulty debugging
> >>> performance improvements and regressions without these figures.
> >>> It's crucial for us to understand the true effectiveness of mTHP in
> >>> real-world scenarios, especially in systems with fragmented memory.
> >>>
> >>> This patch sets up the framework for per-order mTHP counters, starting
> >>> with the introduction of anon_alloc and anon_alloc_fallback counters.
> >>> Incorporating additional counters should now be straightforward as well.
> >>>
> >>> Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> >>> ---
> >>>  include/linux/huge_mm.h | 19 ++++++++++++++++
> >>>  mm/huge_memory.c        | 48 +++++++++++++++++++++++++++++++++++++++++
> >>>  mm/memory.c             |  2 ++
> >>>  3 files changed, 69 insertions(+)
> >>>
> >>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> >>> index e896ca4760f6..c5d33017a4dd 100644
> >>> --- a/include/linux/huge_mm.h
> >>> +++ b/include/linux/huge_mm.h
> >>> @@ -264,6 +264,25 @@ unsigned long thp_vma_allowable_orders(struct vm_area_struct *vma,
> >>>                                         enforce_sysfs, orders);
> >>>  }
> >>>
> >>> +enum mthp_stat_item {
> >>> +     MTHP_STAT_ANON_ALLOC,
> >>> +     MTHP_STAT_ANON_ALLOC_FALLBACK,
> >>> +     __MTHP_STAT_COUNT
> >>> +};
> >>> +
> >>> +struct mthp_stat {
> >>> +     unsigned long stats[PMD_ORDER + 1][__MTHP_STAT_COUNT];
> >>
> >> I saw a fix for this allocation dynamically due to powerpc PMD_ORDER not being
> >> constant. I wonder if ilog2(MAX_PTRS_PER_PTE) would help here?
> >>
> >
> > It's a possibility. However, since we've passed all the build tests
> > using dynamic
> > allocation, it might not be worth the effort to attempt static
> > allocation again. Who
> > knows what will happen next :-)
>
> If the dynamic version is clear and obvious then fair enough. I tried doing
> something similar for the swap-out series but it turned out a mess, so ended up
> falling back to static allocation which was much easier to understand.

mthp_stats doesn't need to be dynamically released, as long as hugepage_init()
succeeds, we need it forever even if sysfs is released or disabled.

>
> >
> >>> +};
> >>> +
> >>> +DECLARE_PER_CPU(struct mthp_stat, mthp_stats);
> >>> +
> >>> +static inline void count_mthp_stat(int order, enum mthp_stat_item item)
> >>
> >> I thought we were going to call this always counting up type of stat and event?
> >> "count_mthp_event"? But I'm happy with it as is, personally.
> >>
> >>> +{
> >>> +     if (unlikely(order > PMD_ORDER))
> >>> +             return;
> >>
> >> I'm wondering if it also makes sense to ignore order == 0? Although I guess if
> >> called for order-0 its safe since the storage exists and sum_mthp_stat() is
> >> never be called for 0. Ignore this comment :)
> >
> > Agreed. I'd like to change it to ignore oder 0;
> >
> >>
> >>> +     this_cpu_inc(mthp_stats.stats[order][item]);
> >>> +}
> >>> +
> >>>  #define transparent_hugepage_use_zero_page()                         \
> >>>       (transparent_hugepage_flags &                                   \
> >>>        (1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG))
> >>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> >>> index 9d4b2fbf6872..5b875f0fc923 100644
> >>> --- a/mm/huge_memory.c
> >>> +++ b/mm/huge_memory.c
> >>> @@ -526,6 +526,46 @@ static const struct kobj_type thpsize_ktype = {
> >>>       .sysfs_ops = &kobj_sysfs_ops,
> >>>  };
> >>>
> >>> +DEFINE_PER_CPU(struct mthp_stat, mthp_stats) = {{{0}}};
> >>> +
> >>> +static unsigned long sum_mthp_stat(int order, enum mthp_stat_item item)
> >>> +{
> >>> +     unsigned long sum = 0;
> >>> +     int cpu;
> >>> +
> >>> +     for_each_online_cpu(cpu) {
> >>
> >> What happens if a cpu that was online and collected a bunch of stats gets
> >> offlined? The user will see stats get smaller?
> >>
> >> Perhaps this should be for_each_possible_cpu()? Although I'm not sure what
> >> happens to percpu data when a cpu goes offline? Is the data preserved? Or wiped,
> >> or unmapped? dunno. Might we need to rescue stats into a global counter at
> >> offline-time?
> >
> > Good catch. I see /proc/vmstat is always using the  for_each_online_cpu() but it
> > doesn't have the issue, but mTHP counters do have the problem.
> >
> > * step 1: cat  the current thp_swpout value before running a test
> > program which does
> > swpout;
> >
> > / # cat /proc/vmstat | grep thp_swpout
> > thp_swpout 0
> > / # cat /sys/kernel/mm/transparent_hugepage/hugepages-2048kB/stats/anon_swpout
> > 0
> >
> > * step 2: run the test program on cpu2;
> >
> > / # taskset -c 2 /home/barry/develop/linux/swpcache-2m
> >
> > * step 3: cat the current thp_swpout value after running a test
> > program which does
> > swpout;
> >
> > / # cat /proc/vmstat | grep thp_swpout
> > thp_swpout 98
> > / # cat /sys/kernel/mm/transparent_hugepage/hugepages-2048kB/stats/anon_swpout
> > 98
> >
> > *step 4: offline cpu2 and read thp_swpout;
> >
> > / # echo 0 > /sys/devices/system/cpu/cpu2/online
> > [  339.058661] psci: CPU2 killed (polled 0 ms)
> >
> > / # cat /proc/vmstat | grep thp_swpout
> > thp_swpout 98
> > / # cat /sys/kernel/mm/transparent_hugepage/hugepages-2048kB/stats/anon_swpout
> > 0
> >
> > *step 5: online cpu2 and read thp_swpout
> >
> > / # echo 1 > /sys/devices/system/cpu/cpu2/online
> > [  791.642058] CPU2: Booted secondary processor 0x0000000002 [0x000f0510]
> >
> > / # cat /proc/vmstat | grep thp_swpout
> > thp_swpout 98
> > / # cat /sys/kernel/mm/transparent_hugepage/hugepages-2048kB/stats/anon_swpout
> > 98
> >
> >
> >
> > As you can see, in step 4, /proc/vmstat is all good but mTHP counters become
> > zero.
> >
> > The reason is /proc/vmstat will fold the offline cpu to an online cpu
> > but mthp counters lack
> > it:
> >
> > /*
> >  * Fold the foreign cpu events into our own.
> >  *
> >  * This is adding to the events on one processor
> >  * but keeps the global counts constant.
> >  */
> > void vm_events_fold_cpu(int cpu)
> > {
> >         struct vm_event_state *fold_state = &per_cpu(vm_event_states, cpu);
> >         int i;
> >
> >         for (i = 0; i < NR_VM_EVENT_ITEMS; i++) {
> >                 count_vm_events(i, fold_state->event[i]);
> >                 fold_state->event[i] = 0;
> >         }
> > }
> >
> > static int page_alloc_cpu_dead(unsigned int cpu)
> > {
> >         ...
> >         /*
> >          * Spill the event counters of the dead processor
> >          * into the current processors event counters.
> >          * This artificially elevates the count of the current
> >          * processor.
> >          */
> >         vm_events_fold_cpu(cpu);
> >         ...
> >
> >         return 0;
> > }
> >
> > So I will do the same thing for mTHP counters - fold offline cpu
> > counters to online one.
>
> That all looks like a complete mess - better avoided if possible! A quick search
> for "for_each_possible_cpu" shows loads of places where code is iterating over
> all *possible* cpus and grabbing its per-cpu data. So the data definitely
> remains accessible when the cpu is offline. Looks like it doesn't get wiped either.
>
> So can't you just change your sum function to iterate over all possible cpus?().

I don't find why not. but i sent a v5[1] similar to  vm_events_fold_cpu.
i can move to for_each_possible_cpu() in v6. if it is a better approach,
i can even further refine that for vmstat.

[1] https://lore.kernel.org/linux-mm/20240412073740.294272-2-21cnbao@gmail.com/

>
> >
> >>
> >>> +             struct mthp_stat *this = &per_cpu(mthp_stats, cpu);
> >>> +
> >>> +             sum += this->stats[order][item];
> >>> +     }
> >>> +
> >>> +     return sum;
> >>> +}
> >>> +
> >>> +#define DEFINE_MTHP_STAT_ATTR(_name, _index)                                 \
> >>> +static ssize_t _name##_show(struct kobject *kobj,                    \
> >>> +                     struct kobj_attribute *attr, char *buf)         \
> >>> +{                                                                    \
> >>> +     int order = to_thpsize(kobj)->order;                            \
> >>> +                                                                     \
> >>> +     return sysfs_emit(buf, "%lu\n", sum_mthp_stat(order, _index));  \
> >>> +}                                                                    \
> >>> +static struct kobj_attribute _name##_attr = __ATTR_RO(_name)
> >>
> >> Very nice!
> >
> > Right. I got duplicated copy-paste and bad small in code so I wrote this macro.
> >
> >>
> >>> +
> >>> +DEFINE_MTHP_STAT_ATTR(anon_alloc, MTHP_STAT_ANON_ALLOC);
> >>> +DEFINE_MTHP_STAT_ATTR(anon_alloc_fallback, MTHP_STAT_ANON_ALLOC_FALLBACK);
> >>> +
> >>> +static struct attribute *stats_attrs[] = {
> >>> +     &anon_alloc_attr.attr,
> >>> +     &anon_alloc_fallback_attr.attr,
> >>> +     NULL,
> >>> +};
> >>> +
> >>> +static struct attribute_group stats_attr_group = {
> >>> +     .name = "stats",
> >>> +     .attrs = stats_attrs,
> >>> +};
> >>> +
> >>>  static struct thpsize *thpsize_create(int order, struct kobject *parent)
> >>>  {
> >>>       unsigned long size = (PAGE_SIZE << order) / SZ_1K;
> >>> @@ -549,6 +589,12 @@ static struct thpsize *thpsize_create(int order, struct kobject *parent)
> >>>               return ERR_PTR(ret);
> >>>       }
> >>>
> >>> +     ret = sysfs_create_group(&thpsize->kobj, &stats_attr_group);
> >>> +     if (ret) {
> >>> +             kobject_put(&thpsize->kobj);
> >>> +             return ERR_PTR(ret);
> >>> +     }
> >>> +
> >>>       thpsize->order = order;
> >>>       return thpsize;
> >>>  }
> >>> @@ -1050,8 +1096,10 @@ vm_fault_t do_huge_pmd_anonymous_page(struct vm_fault *vmf)
> >>>       folio = vma_alloc_folio(gfp, HPAGE_PMD_ORDER, vma, haddr, true);
> >>>       if (unlikely(!folio)) {
> >>>               count_vm_event(THP_FAULT_FALLBACK);
> >>> +             count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_ALLOC_FALLBACK);
> >>
> >> I think we should aim for the PMD-oder MTHP_STAT_ANON_ALLOC and
> >> MTHP_STAT_ANON_ALLOC_FALLBACK to match THP_FAULT_ALLOC and THP_FAULT_FALLBACK.
> >> Its not currently setup this way...
> >
> > right. I also realized this and asked for your comments on this in another
> > thread.
>
> Ahh sorry - must have missed that.
>
> >
> >>
> >>
> >>>               return VM_FAULT_FALLBACK;
> >>>       }
> >>> +     count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_ALLOC);
> >>>       return __do_huge_pmd_anonymous_page(vmf, &folio->page, gfp);
> >>>  }
> >>>
> >>> diff --git a/mm/memory.c b/mm/memory.c
> >>> index 649e3ed94487..1723c8ddf9cb 100644
> >>> --- a/mm/memory.c
> >>> +++ b/mm/memory.c
> >>> @@ -4374,8 +4374,10 @@ static struct folio *alloc_anon_folio(struct vm_fault *vmf)
> >>>                       }
> >>>                       folio_throttle_swaprate(folio, gfp);
> >>>                       clear_huge_page(&folio->page, vmf->address, 1 << order);
> >>> +                     count_mthp_stat(order, MTHP_STAT_ANON_ALLOC);
> >>>                       return folio;
> >>>               }
> >>> +             count_mthp_stat(order, MTHP_STAT_ANON_ALLOC_FALLBACK);
> >>
> >> ...And we should follow the usage same pattern for the smaller mTHP here too.
> >> Which means MTHP_STAT_ANON_ALLOC_FALLBACK should be after the next: label. We
> >
> > The only difference is the case
> >
> > if (mem_cgroup_charge(folio, vma->vm_mm, gfp))
> >       goto next;
> >
> > but vmstat is counting this as fallback so i feel good to move after next,
> >
> >         if (mem_cgroup_charge(folio, vma->vm_mm, gfp)) {
> >                 folio_put(folio);
> >                 count_vm_event(THP_FAULT_FALLBACK);
> >                 count_vm_event(THP_FAULT_FALLBACK_CHARGE);
> >                 return VM_FAULT_FALLBACK;
> >         }
> >
> >> could introduce a MTHP_STAT_ANON_ALLOC_FALLBACK_CHARGE which would only trigger
> >> on a fallback due to charge failure, just like THP_FAULT_FALLBACK_CHARGE?
> >
> > it is fine to add this THP_FAULT_FALLBACK_CHARGE though it is not that
> > useful for profiling buddy fragmentation.
>
> Well I thought you were interested in isolating fallback due to fragmentation
> only. You would get that with (FAULT_FALLBACK - FAULT_FALLBACK_CHARGE)? But if
> you think the latter will be relatively small/unimportant for now, and therefore
> FAULT_FALLBACK will give good enough approximation on its own, then I'm happy
> not to add FAULT_FALLBACK_CHARGE for now.

in v5, i actually added FAULT_FALLBACK_CHARGE. Please take a look at v5 :-)

>
> >
> >>
> >>>  next:
> >>>               order = next_order(&orders, order);
> >>>       }
> >>
> >
> > Thanks
> > Barry
>

[PATCH v4 2/2] mm: add per-order mTHP anon_swpout and anon_swpout_fallback counters

[Barry Song]

From: Barry Song <v-songbaohua@oppo.com>

This helps to display the fragmentation situation of the swapfile,
knowing the proportion of how much we haven't split large folios.
So far, we only support non-split swapout for anon memory, with
the possibility of expanding to shmem in the future.  So, we add
the "anon" prefix to the counter names.

Signed-off-by: Barry Song <v-songbaohua@oppo.com>
---
 include/linux/huge_mm.h | 2 ++
 mm/huge_memory.c        | 4 ++++
 mm/page_io.c            | 6 +++++-
 mm/vmscan.c             | 3 +++
 4 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index c5d33017a4dd..1d893a358df6 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -267,6 +267,8 @@ unsigned long thp_vma_allowable_orders(struct vm_area_struct *vma,
 enum mthp_stat_item {
 	MTHP_STAT_ANON_ALLOC,
 	MTHP_STAT_ANON_ALLOC_FALLBACK,
+	MTHP_STAT_ANON_SWPOUT,
+	MTHP_STAT_ANON_SWPOUT_FALLBACK,
 	__MTHP_STAT_COUNT
 };
 
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 5b875f0fc923..28113f8fdf18 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -554,10 +554,14 @@ static struct kobj_attribute _name##_attr = __ATTR_RO(_name)
 
 DEFINE_MTHP_STAT_ATTR(anon_alloc, MTHP_STAT_ANON_ALLOC);
 DEFINE_MTHP_STAT_ATTR(anon_alloc_fallback, MTHP_STAT_ANON_ALLOC_FALLBACK);
+DEFINE_MTHP_STAT_ATTR(anon_swpout, MTHP_STAT_ANON_SWPOUT);
+DEFINE_MTHP_STAT_ATTR(anon_swpout_fallback, MTHP_STAT_ANON_SWPOUT_FALLBACK);
 
 static struct attribute *stats_attrs[] = {
 	&anon_alloc_attr.attr,
 	&anon_alloc_fallback_attr.attr,
+	&anon_swpout_attr.attr,
+	&anon_swpout_fallback_attr.attr,
 	NULL,
 };
 
diff --git a/mm/page_io.c b/mm/page_io.c
index a9a7c236aecc..7669452e8b4d 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -212,13 +212,17 @@ int swap_writepage(struct page *page, struct writeback_control *wbc)
 
 static inline void count_swpout_vm_event(struct folio *folio)
 {
+	long nr_pages = folio_nr_pages(folio);
+
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 	if (unlikely(folio_test_pmd_mappable(folio))) {
 		count_memcg_folio_events(folio, THP_SWPOUT, 1);
 		count_vm_event(THP_SWPOUT);
 	}
+	if (nr_pages > 0)
+		count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPOUT);
 #endif
-	count_vm_events(PSWPOUT, folio_nr_pages(folio));
+	count_vm_events(PSWPOUT, nr_pages);
 }
 
 #if defined(CONFIG_MEMCG) && defined(CONFIG_BLK_CGROUP)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index ffc4553c8615..b30e6294f82a 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1247,6 +1247,9 @@ static unsigned int shrink_folio_list(struct list_head *folio_list,
 						count_vm_event(
 							THP_SWPOUT_FALLBACK);
 					}
+					if (nr_pages > 0)
+						count_mthp_stat(get_order(nr_pages * PAGE_SIZE),
+							MTHP_STAT_ANON_SWPOUT_FALLBACK);
 #endif
 					if (!add_to_swap(folio))
 						goto activate_locked_split;
-- 
2.34.1

Re: [PATCH v4 2/2] mm: add per-order mTHP anon_swpout and anon_swpout_fallback counters

[Ryan Roberts]

On 05/04/2024 11:27, Barry Song wrote:
> From: Barry Song <v-songbaohua@oppo.com>
> 
> This helps to display the fragmentation situation of the swapfile,
> knowing the proportion of how much we haven't split large folios.
> So far, we only support non-split swapout for anon memory, with
> the possibility of expanding to shmem in the future.  So, we add
> the "anon" prefix to the counter names.
> 
> Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> ---
>  include/linux/huge_mm.h | 2 ++
>  mm/huge_memory.c        | 4 ++++
>  mm/page_io.c            | 6 +++++-
>  mm/vmscan.c             | 3 +++
>  4 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index c5d33017a4dd..1d893a358df6 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -267,6 +267,8 @@ unsigned long thp_vma_allowable_orders(struct vm_area_struct *vma,
>  enum mthp_stat_item {
>  	MTHP_STAT_ANON_ALLOC,
>  	MTHP_STAT_ANON_ALLOC_FALLBACK,
> +	MTHP_STAT_ANON_SWPOUT,
> +	MTHP_STAT_ANON_SWPOUT_FALLBACK,
>  	__MTHP_STAT_COUNT
>  };
>  
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 5b875f0fc923..28113f8fdf18 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -554,10 +554,14 @@ static struct kobj_attribute _name##_attr = __ATTR_RO(_name)
>  
>  DEFINE_MTHP_STAT_ATTR(anon_alloc, MTHP_STAT_ANON_ALLOC);
>  DEFINE_MTHP_STAT_ATTR(anon_alloc_fallback, MTHP_STAT_ANON_ALLOC_FALLBACK);
> +DEFINE_MTHP_STAT_ATTR(anon_swpout, MTHP_STAT_ANON_SWPOUT);
> +DEFINE_MTHP_STAT_ATTR(anon_swpout_fallback, MTHP_STAT_ANON_SWPOUT_FALLBACK);
>  
>  static struct attribute *stats_attrs[] = {
>  	&anon_alloc_attr.attr,
>  	&anon_alloc_fallback_attr.attr,
> +	&anon_swpout_attr.attr,
> +	&anon_swpout_fallback_attr.attr,
>  	NULL,
>  };
>  
> diff --git a/mm/page_io.c b/mm/page_io.c
> index a9a7c236aecc..7669452e8b4d 100644
> --- a/mm/page_io.c
> +++ b/mm/page_io.c
> @@ -212,13 +212,17 @@ int swap_writepage(struct page *page, struct writeback_control *wbc)
>  
>  static inline void count_swpout_vm_event(struct folio *folio)
>  {
> +	long nr_pages = folio_nr_pages(folio);
> +
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>  	if (unlikely(folio_test_pmd_mappable(folio))) {
>  		count_memcg_folio_events(folio, THP_SWPOUT, 1);
>  		count_vm_event(THP_SWPOUT);
>  	}
> +	if (nr_pages > 0)
> +		count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPOUT);

I think you mean "nr_pages > 1"? Although as established in my comments against
patch 1, its safe to call for order-0, so could just unconditionally call this
(as you effectively are at the moment).

>  #endif
> -	count_vm_events(PSWPOUT, folio_nr_pages(folio));
> +	count_vm_events(PSWPOUT, nr_pages);
>  }
>  
>  #if defined(CONFIG_MEMCG) && defined(CONFIG_BLK_CGROUP)
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index ffc4553c8615..b30e6294f82a 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1247,6 +1247,9 @@ static unsigned int shrink_folio_list(struct list_head *folio_list,
>  						count_vm_event(
>  							THP_SWPOUT_FALLBACK);
>  					}
> +					if (nr_pages > 0)
> +						count_mthp_stat(get_order(nr_pages * PAGE_SIZE),
> +							MTHP_STAT_ANON_SWPOUT_FALLBACK);

Same comment.

>  #endif
>  					if (!add_to_swap(folio))
>  						goto activate_locked_split;

Re: [PATCH v4 2/2] mm: add per-order mTHP anon_swpout and anon_swpout_fallback counters

[Barry Song]

On Fri, Apr 12, 2024 at 4:43 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>
> On 05/04/2024 11:27, Barry Song wrote:
> > From: Barry Song <v-songbaohua@oppo.com>
> >
> > This helps to display the fragmentation situation of the swapfile,
> > knowing the proportion of how much we haven't split large folios.
> > So far, we only support non-split swapout for anon memory, with
> > the possibility of expanding to shmem in the future.  So, we add
> > the "anon" prefix to the counter names.
> >
> > Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> > ---
> >  include/linux/huge_mm.h | 2 ++
> >  mm/huge_memory.c        | 4 ++++
> >  mm/page_io.c            | 6 +++++-
> >  mm/vmscan.c             | 3 +++
> >  4 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> > index c5d33017a4dd..1d893a358df6 100644
> > --- a/include/linux/huge_mm.h
> > +++ b/include/linux/huge_mm.h
> > @@ -267,6 +267,8 @@ unsigned long thp_vma_allowable_orders(struct vm_area_struct *vma,
> >  enum mthp_stat_item {
> >       MTHP_STAT_ANON_ALLOC,
> >       MTHP_STAT_ANON_ALLOC_FALLBACK,
> > +     MTHP_STAT_ANON_SWPOUT,
> > +     MTHP_STAT_ANON_SWPOUT_FALLBACK,
> >       __MTHP_STAT_COUNT
> >  };
> >
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 5b875f0fc923..28113f8fdf18 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -554,10 +554,14 @@ static struct kobj_attribute _name##_attr = __ATTR_RO(_name)
> >
> >  DEFINE_MTHP_STAT_ATTR(anon_alloc, MTHP_STAT_ANON_ALLOC);
> >  DEFINE_MTHP_STAT_ATTR(anon_alloc_fallback, MTHP_STAT_ANON_ALLOC_FALLBACK);
> > +DEFINE_MTHP_STAT_ATTR(anon_swpout, MTHP_STAT_ANON_SWPOUT);
> > +DEFINE_MTHP_STAT_ATTR(anon_swpout_fallback, MTHP_STAT_ANON_SWPOUT_FALLBACK);
> >
> >  static struct attribute *stats_attrs[] = {
> >       &anon_alloc_attr.attr,
> >       &anon_alloc_fallback_attr.attr,
> > +     &anon_swpout_attr.attr,
> > +     &anon_swpout_fallback_attr.attr,
> >       NULL,
> >  };
> >
> > diff --git a/mm/page_io.c b/mm/page_io.c
> > index a9a7c236aecc..7669452e8b4d 100644
> > --- a/mm/page_io.c
> > +++ b/mm/page_io.c
> > @@ -212,13 +212,17 @@ int swap_writepage(struct page *page, struct writeback_control *wbc)
> >
> >  static inline void count_swpout_vm_event(struct folio *folio)
> >  {
> > +     long nr_pages = folio_nr_pages(folio);
> > +
> >  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> >       if (unlikely(folio_test_pmd_mappable(folio))) {
> >               count_memcg_folio_events(folio, THP_SWPOUT, 1);
> >               count_vm_event(THP_SWPOUT);
> >       }
> > +     if (nr_pages > 0)
> > +             count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPOUT);
>
> I think you mean "nr_pages > 1"? Although as established in my comments against
> patch 1, its safe to call for order-0, so could just unconditionally call this
> (as you effectively are at the moment).

yes. nr_pages > 1 is the correct condition. though it doesn't cause
any crash for
nr_pages > 0 as we have memory for order-0. but it is a useless job to count
for nr_pages==1 as there is no place to show it in sysfs.

>
> >  #endif
> > -     count_vm_events(PSWPOUT, folio_nr_pages(folio));
> > +     count_vm_events(PSWPOUT, nr_pages);
> >  }
> >
> >  #if defined(CONFIG_MEMCG) && defined(CONFIG_BLK_CGROUP)
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index ffc4553c8615..b30e6294f82a 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -1247,6 +1247,9 @@ static unsigned int shrink_folio_list(struct list_head *folio_list,
> >                                               count_vm_event(
> >                                                       THP_SWPOUT_FALLBACK);
> >                                       }
> > +                                     if (nr_pages > 0)
> > +                                             count_mthp_stat(get_order(nr_pages * PAGE_SIZE),
> > +                                                     MTHP_STAT_ANON_SWPOUT_FALLBACK);
>
> Same comment.

agreed.  Also, it doesn't cause any crash as we have memory in mthp_stats for
order-0.
i will totally remove  if (nr_pages > 0) and as we are certainly in large folios
cases.

>
> >  #endif
> >                                       if (!add_to_swap(folio))
> >                                               goto activate_locked_split;
>

Thanks
Barry

Re: [PATCH v4 0/2] mm: add per-order mTHP alloc and swpout counters

[Andrew Morton]

On Fri,  5 Apr 2024 23:27:02 +1300 Barry Song <21cnbao@gmail.com> wrote:

> The patchset introduces a framework to facilitate mTHP counters, starting
> with the allocation and swap-out counters. Currently, only four new nodes
> are appended to the stats directory for each mTHP size.
> 
> /sys/kernel/mm/transparent_hugepage/hugepages-<size>/stats
> 	anon_alloc
> 	anon_alloc_fallback
> 	anon_swpout
> 	anon_swpout_fallback

Please let's get these documented? 
Documentation/admin-guide/mm/transhuge.rst seems to be the place, and
under Documentation/ABI.  

Re: [PATCH v4 0/2] mm: add per-order mTHP alloc and swpout counters

[Barry Song]

On Sat, Apr 6, 2024 at 9:11 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Fri,  5 Apr 2024 23:27:02 +1300 Barry Song <21cnbao@gmail.com> wrote:
>
> > The patchset introduces a framework to facilitate mTHP counters, starting
> > with the allocation and swap-out counters. Currently, only four new nodes
> > are appended to the stats directory for each mTHP size.
> >
> > /sys/kernel/mm/transparent_hugepage/hugepages-<size>/stats
> >       anon_alloc
> >       anon_alloc_fallback
> >       anon_swpout
> >       anon_swpout_fallback
>
> Please let's get these documented?

Sure, I will do it.

Furthermore, after careful consideration, I suggest renaming anon_alloc to
anon_fault_alloc and anon_alloc_fallback to anon_fault_fallback. This
renaming is consistent with the naming convention observed in
thp_fault_alloc and thp_fault_fallback in vmstat. Additionally, this
modification will facilitate the handling of large folio swap-ins.

/sys/kernel/mm/transparent_hugepage/hugepages-<size>/stats
           anon_fault_alloc,
           anon_fault_fallback,
           anon_swpin_alloc,
           anon_swpin_fallback,

Otherwise, without this renaming, it would be difficult to
differentiate fault_alloc
from swpin_alloc.

Hi David, Ryan,
I would also appreciate your input on the modification I am considering.

> Documentation/admin-guide/mm/transhuge.rst seems to be the place, and
> under Documentation/ABI.

Thanks
Barry

Re: [PATCH v4 0/2] mm: add per-order mTHP alloc and swpout counters

[Ryan Roberts]

On 05/04/2024 23:04, Barry Song wrote:
> On Sat, Apr 6, 2024 at 9:11 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>>
>> On Fri,  5 Apr 2024 23:27:02 +1300 Barry Song <21cnbao@gmail.com> wrote:
>>
>>> The patchset introduces a framework to facilitate mTHP counters, starting
>>> with the allocation and swap-out counters. Currently, only four new nodes
>>> are appended to the stats directory for each mTHP size.
>>>
>>> /sys/kernel/mm/transparent_hugepage/hugepages-<size>/stats
>>>       anon_alloc
>>>       anon_alloc_fallback
>>>       anon_swpout
>>>       anon_swpout_fallback
>>
>> Please let's get these documented?
> 
> Sure, I will do it.
> 
> Furthermore, after careful consideration, I suggest renaming anon_alloc to
> anon_fault_alloc and anon_alloc_fallback to anon_fault_fallback. This
> renaming is consistent with the naming convention observed in
> thp_fault_alloc and thp_fault_fallback in vmstat. Additionally, this
> modification will facilitate the handling of large folio swap-ins.
> 
> /sys/kernel/mm/transparent_hugepage/hugepages-<size>/stats
>            anon_fault_alloc,
>            anon_fault_fallback,
>            anon_swpin_alloc,
>            anon_swpin_fallback,
> 
> Otherwise, without this renaming, it would be difficult to
> differentiate fault_alloc
> from swpin_alloc.
> 
> Hi David, Ryan,
> I would also appreciate your input on the modification I am considering.

Yes, this makes sense to me!

> 
>> Documentation/admin-guide/mm/transhuge.rst seems to be the place, and
>> under Documentation/ABI.
> 
> Thanks
> Barry