Skip to contentSkip to conversation

mm: shmem: Rename mTHP shmem counters

PATCH Applied Ryan Roberts 4 participants 4 messages Jul 10, 2024

Subsystems (5)
lore
Share & tools
lore

Ryan Roberts

author reviewer
#0 lore raw

PATCH v2 1/1 mm: shmem: Rename mTHP shmem counters

The legacy PMD-sized THP counters at /proc/vmstat include thp_file_alloc, thp_file_fallback and thp_file_fallback_charge, which rather confusingly refer to shmem THP and do not include any other types of file pages. This is inconsistent since in most other places in the kernel, THP counters are explicitly separated for anon, shmem and file flavours. However, we are stuck with it since it constitutes a user ABI.

Recently, commit 66f44583f9b6 ("mm: shmem: add mTHP counters for anonymous shmem") added equivalent mTHP stats for shmem, keeping the same "file_" prefix in the names. But in future, we may want to add extra stats to cover actual file pages, at which point, it would all become very confusing.

So let's take the opportunity to rename these new counters "shmem_" before the change makes it upstream and the ABI becomes immutable. While we are at it, let's improve the documentation for the legacy counters to make it clear that they count shmem pages only.

---

Hi All,

Applies on top of yesterday's mm-unstable (2073cda629a4) and tested with mm selftests; no regressions observed.

The backstory here is that I'd like to introduce some counters for regular file folio allocations to observe how often large folio allocation succeeds, but these shmem counters are named "file" which is going to make things confusing. So hoping to solve that before commit 66f44583f9b6 ("mm: shmem: add mTHP counters for anonymous shmem") goes upstream (it is currently in mm-stable).

Changes since v1 [1]
====================
- Updated documentation for existing legacy "file_" counters to make it clear they only count shmem pages.

[1] https://lore.kernel.org/linux-mm/[email protected]/

Thanks,
Ryan

 Documentation/admin-guide/mm/transhuge.rst | 29 ++++++++++++----------
 include/linux/huge_mm.h                    |  6 ++---
 mm/huge_memory.c                           | 12 ++++-----
 mm/shmem.c                                 |  8 +++---
 4 files changed, 29 insertions(+), 26 deletions(-)
Documentation/admin-guide/mm/transhuge.rst +16 -13
index 747c811ee8f1..3528daa1f239 100644
@@ -412,20 +412,23 @@ thp_collapse_alloc_failed
 	the allocation.
 
 thp_file_alloc
-	is incremented every time a file huge page is successfully
-	allocated.
+	is incremented every time a shmem huge page is successfully
+	allocated (Note that despite being named after "file", the counter
+	measures only shmem).
 
 thp_file_fallback
-	is incremented if a file huge page is attempted to be allocated
-	but fails and instead falls back to using small pages.
+	is incremented if a shmem huge page is attempted to be allocated
+	but fails and instead falls back to using small pages. (Note that
+	despite being named after "file", the counter measures only shmem).
 
 thp_file_fallback_charge
-	is incremented if a file huge page cannot be charged and instead
+	is incremented if a shmem huge page cannot be charged and instead
 	falls back to using small pages even though the allocation was
-	successful.
+	successful. (Note that despite being named after "file", the
+	counter measures only shmem).
 
 thp_file_mapped
-	is incremented every time a file huge page is mapped into
+	is incremented every time a file or shmem huge page is mapped into
 	user address space.
 
 thp_split_page
@@ -496,16 +499,16 @@ swpout_fallback
 	Usually because failed to allocate some continuous swap space
 	for the huge page.
 
-file_alloc
-	is incremented every time a file huge page is successfully
+shmem_alloc
+	is incremented every time a shmem huge page is successfully
 	allocated.
 
-file_fallback
-	is incremented if a file huge page is attempted to be allocated
+shmem_fallback
+	is incremented if a shmem huge page is attempted to be allocated
 	but fails and instead falls back to using small pages.
 
-file_fallback_charge
-	is incremented if a file huge page cannot be charged and instead
+shmem_fallback_charge
+	is incremented if a shmem huge page cannot be charged and instead
 	falls back to using small pages even though the allocation was
 	successful.
 
include/linux/huge_mm.h +3 -3
index acb6ac24a07e..cff002be83eb 100644
@@ -269,9 +269,9 @@ enum mthp_stat_item {
 	MTHP_STAT_ANON_FAULT_FALLBACK_CHARGE,
 	MTHP_STAT_SWPOUT,
 	MTHP_STAT_SWPOUT_FALLBACK,
-	MTHP_STAT_FILE_ALLOC,
-	MTHP_STAT_FILE_FALLBACK,
-	MTHP_STAT_FILE_FALLBACK_CHARGE,
+	MTHP_STAT_SHMEM_ALLOC,
+	MTHP_STAT_SHMEM_FALLBACK,
+	MTHP_STAT_SHMEM_FALLBACK_CHARGE,
 	MTHP_STAT_SPLIT,
 	MTHP_STAT_SPLIT_FAILED,
 	MTHP_STAT_SPLIT_DEFERRED,
mm/huge_memory.c +6 -6
index 9ec64aa2be94..f9696c94e211 100644
@@ -568,9 +568,9 @@ DEFINE_MTHP_STAT_ATTR(anon_fault_fallback, MTHP_STAT_ANON_FAULT_FALLBACK);
 DEFINE_MTHP_STAT_ATTR(anon_fault_fallback_charge, MTHP_STAT_ANON_FAULT_FALLBACK_CHARGE);
 DEFINE_MTHP_STAT_ATTR(swpout, MTHP_STAT_SWPOUT);
 DEFINE_MTHP_STAT_ATTR(swpout_fallback, MTHP_STAT_SWPOUT_FALLBACK);
-DEFINE_MTHP_STAT_ATTR(file_alloc, MTHP_STAT_FILE_ALLOC);
-DEFINE_MTHP_STAT_ATTR(file_fallback, MTHP_STAT_FILE_FALLBACK);
-DEFINE_MTHP_STAT_ATTR(file_fallback_charge, MTHP_STAT_FILE_FALLBACK_CHARGE);
+DEFINE_MTHP_STAT_ATTR(shmem_alloc, MTHP_STAT_SHMEM_ALLOC);
+DEFINE_MTHP_STAT_ATTR(shmem_fallback, MTHP_STAT_SHMEM_FALLBACK);
+DEFINE_MTHP_STAT_ATTR(shmem_fallback_charge, MTHP_STAT_SHMEM_FALLBACK_CHARGE);
 DEFINE_MTHP_STAT_ATTR(split, MTHP_STAT_SPLIT);
 DEFINE_MTHP_STAT_ATTR(split_failed, MTHP_STAT_SPLIT_FAILED);
 DEFINE_MTHP_STAT_ATTR(split_deferred, MTHP_STAT_SPLIT_DEFERRED);
@@ -581,9 +581,9 @@ static struct attribute *stats_attrs[] = {
 	&anon_fault_fallback_charge_attr.attr,
 	&swpout_attr.attr,
 	&swpout_fallback_attr.attr,
-	&file_alloc_attr.attr,
-	&file_fallback_attr.attr,
-	&file_fallback_charge_attr.attr,
+	&shmem_alloc_attr.attr,
+	&shmem_fallback_attr.attr,
+	&shmem_fallback_charge_attr.attr,
 	&split_attr.attr,
 	&split_failed_attr.attr,
 	&split_deferred_attr.attr,
mm/shmem.c +4 -4
index 921d59c3d669..f24dfbd387ba 100644
@@ -1777,7 +1777,7 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
 			if (pages == HPAGE_PMD_NR)
 				count_vm_event(THP_FILE_FALLBACK);
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-			count_mthp_stat(order, MTHP_STAT_FILE_FALLBACK);
+			count_mthp_stat(order, MTHP_STAT_SHMEM_FALLBACK);
 #endif
 			order = next_order(&suitable_orders, order);
 		}
@@ -1804,8 +1804,8 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
 				count_vm_event(THP_FILE_FALLBACK_CHARGE);
 			}
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-			count_mthp_stat(folio_order(folio), MTHP_STAT_FILE_FALLBACK);
-			count_mthp_stat(folio_order(folio), MTHP_STAT_FILE_FALLBACK_CHARGE);
+			count_mthp_stat(folio_order(folio), MTHP_STAT_SHMEM_FALLBACK);
+			count_mthp_stat(folio_order(folio), MTHP_STAT_SHMEM_FALLBACK_CHARGE);
 #endif
 		}
 		goto unlock;
@@ -2181,7 +2181,7 @@ static int shmem_get_folio_gfp(struct inode *inode, pgoff_t index,
 			if (folio_test_pmd_mappable(folio))
 				count_vm_event(THP_FILE_ALLOC);
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-			count_mthp_stat(folio_order(folio), MTHP_STAT_FILE_ALLOC);
+			count_mthp_stat(folio_order(folio), MTHP_STAT_SHMEM_ALLOC);
 #endif
 			goto alloced;
 		}

--
2.43.0

reviewer
#1 lore raw
Quoted 41 lines · Ryan Roberts

The legacy PMD-sized THP counters at /proc/vmstat include thp_file_alloc, thp_file_fallback and thp_file_fallback_charge, which rather confusingly refer to shmem THP and do not include any other types of file pages. This is inconsistent since in most other places in the kernel, THP counters are explicitly separated for anon, shmem and file flavours. However, we are stuck with it since it constitutes a user ABI.

Recently, commit 66f44583f9b6 ("mm: shmem: add mTHP counters for anonymous shmem") added equivalent mTHP stats for shmem, keeping the same "file_" prefix in the names. But in future, we may want to add extra stats to cover actual file pages, at which point, it would all become very confusing.

So let's take the opportunity to rename these new counters "shmem_" before the change makes it upstream and the ABI becomes immutable. While we are at it, let's improve the documentation for the legacy counters to make it clear that they count shmem pages only.

---

Hi All,

Applies on top of yesterday's mm-unstable (2073cda629a4) and tested with mm selftests; no regressions observed.

The backstory here is that I'd like to introduce some counters for regular file folio allocations to observe how often large folio allocation succeeds, but these shmem counters are named "file" which is going to make things confusing. So hoping to solve that before commit 66f44583f9b6 ("mm: shmem: add mTHP counters for anonymous shmem") goes upstream (it is currently in mm-stable).

Changes since v1 [1]
====================
- Updated documentation for existing legacy "file_" counters to make it clear they only count shmem pages.

[1] https://lore.kernel.org/linux-mm/[email protected]/

Quoted 9 lines

Thanks,
Ryan

 Documentation/admin-guide/mm/transhuge.rst | 29 ++++++++++++----------
 include/linux/huge_mm.h                    |  6 ++---
 mm/huge_memory.c                           | 12 ++++-----
 mm/shmem.c                                 |  8 +++---
 4 files changed, 29 insertions(+), 26 deletions(-)

LGTM. Reviewed-by: Zi Yan <[email protected]>

--
Best Regards,
Yan, Zi

reviewer Reviewed-by on v2 1/1
#2 lore raw
In reply to Ryan Roberts
Quoted 14 lines · Ryan Roberts

The legacy PMD-sized THP counters at /proc/vmstat include thp_file_alloc, thp_file_fallback and thp_file_fallback_charge, which rather confusingly refer to shmem THP and do not include any other types of file pages. This is inconsistent since in most other places in the kernel, THP counters are explicitly separated for anon, shmem and file flavours. However, we are stuck with it since it constitutes a user ABI.

Recently, commit 66f44583f9b6 ("mm: shmem: add mTHP counters for anonymous shmem") added equivalent mTHP stats for shmem, keeping the same "file_" prefix in the names. But in future, we may want to add extra stats to cover actual file pages, at which point, it would all become very confusing.

Quoted 8 lines

So let's take the opportunity to rename these new counters "shmem_" before the change makes it upstream and the ABI becomes immutable. While we are at it, let's improve the documentation for the legacy counters to make it clear that they count shmem pages only.

Quoted 168 lines

---

Hi All,

Applies on top of yesterday's mm-unstable (2073cda629a4) and tested with mm selftests; no regressions observed.

The backstory here is that I'd like to introduce some counters for regular file folio allocations to observe how often large folio allocation succeeds, but these shmem counters are named "file" which is going to make things confusing. So hoping to solve that before commit 66f44583f9b6 ("mm: shmem: add mTHP counters for anonymous shmem") goes upstream (it is currently in mm-stable).

Changes since v1 [1]
====================
- Updated documentation for existing legacy "file_" counters to make it clear they only count shmem pages.

[1] https://lore.kernel.org/linux-mm/[email protected]/

Thanks,
Ryan

 Documentation/admin-guide/mm/transhuge.rst | 29 ++++++++++++----------
 include/linux/huge_mm.h                    |  6 ++---
 mm/huge_memory.c                           | 12 ++++-----
 mm/shmem.c                                 |  8 +++---
 4 files changed, 29 insertions(+), 26 deletions(-)
Documentation/admin-guide/mm/transhuge.rst +16 -13
index 747c811ee8f1..3528daa1f239 100644
@@ -412,20 +412,23 @@ thp_collapse_alloc_failed
        the allocation.
 
 thp_file_alloc
-       is incremented every time a file huge page is successfully
-       allocated.
+       is incremented every time a shmem huge page is successfully
+       allocated (Note that despite being named after "file", the counter
+       measures only shmem).
 
 thp_file_fallback
-       is incremented if a file huge page is attempted to be allocated
-       but fails and instead falls back to using small pages.
+       is incremented if a shmem huge page is attempted to be allocated
+       but fails and instead falls back to using small pages. (Note that
+       despite being named after "file", the counter measures only shmem).
 
 thp_file_fallback_charge
-       is incremented if a file huge page cannot be charged and instead
+       is incremented if a shmem huge page cannot be charged and instead
        falls back to using small pages even though the allocation was
-       successful.
+       successful. (Note that despite being named after "file", the
+       counter measures only shmem).
 
 thp_file_mapped
-       is incremented every time a file huge page is mapped into
+       is incremented every time a file or shmem huge page is mapped into
        user address space.
 
 thp_split_page
@@ -496,16 +499,16 @@ swpout_fallback
        Usually because failed to allocate some continuous swap space
        for the huge page.
 
-file_alloc
-       is incremented every time a file huge page is successfully
+shmem_alloc
+       is incremented every time a shmem huge page is successfully
        allocated.
 
-file_fallback
-       is incremented if a file huge page is attempted to be allocated
+shmem_fallback
+       is incremented if a shmem huge page is attempted to be allocated
        but fails and instead falls back to using small pages.
 
-file_fallback_charge
-       is incremented if a file huge page cannot be charged and instead
+shmem_fallback_charge
+       is incremented if a shmem huge page cannot be charged and instead
        falls back to using small pages even though the allocation was
        successful.
 
include/linux/huge_mm.h +3 -3
index acb6ac24a07e..cff002be83eb 100644
@@ -269,9 +269,9 @@ enum mthp_stat_item {
        MTHP_STAT_ANON_FAULT_FALLBACK_CHARGE,
        MTHP_STAT_SWPOUT,
        MTHP_STAT_SWPOUT_FALLBACK,
-       MTHP_STAT_FILE_ALLOC,
-       MTHP_STAT_FILE_FALLBACK,
-       MTHP_STAT_FILE_FALLBACK_CHARGE,
+       MTHP_STAT_SHMEM_ALLOC,
+       MTHP_STAT_SHMEM_FALLBACK,
+       MTHP_STAT_SHMEM_FALLBACK_CHARGE,
        MTHP_STAT_SPLIT,
        MTHP_STAT_SPLIT_FAILED,
        MTHP_STAT_SPLIT_DEFERRED,
mm/huge_memory.c +6 -6
index 9ec64aa2be94..f9696c94e211 100644
@@ -568,9 +568,9 @@ DEFINE_MTHP_STAT_ATTR(anon_fault_fallback, MTHP_STAT_ANON_FAULT_FALLBACK);
 DEFINE_MTHP_STAT_ATTR(anon_fault_fallback_charge, MTHP_STAT_ANON_FAULT_FALLBACK_CHARGE);
 DEFINE_MTHP_STAT_ATTR(swpout, MTHP_STAT_SWPOUT);
 DEFINE_MTHP_STAT_ATTR(swpout_fallback, MTHP_STAT_SWPOUT_FALLBACK);
-DEFINE_MTHP_STAT_ATTR(file_alloc, MTHP_STAT_FILE_ALLOC);
-DEFINE_MTHP_STAT_ATTR(file_fallback, MTHP_STAT_FILE_FALLBACK);
-DEFINE_MTHP_STAT_ATTR(file_fallback_charge, MTHP_STAT_FILE_FALLBACK_CHARGE);
+DEFINE_MTHP_STAT_ATTR(shmem_alloc, MTHP_STAT_SHMEM_ALLOC);
+DEFINE_MTHP_STAT_ATTR(shmem_fallback, MTHP_STAT_SHMEM_FALLBACK);
+DEFINE_MTHP_STAT_ATTR(shmem_fallback_charge, MTHP_STAT_SHMEM_FALLBACK_CHARGE);
 DEFINE_MTHP_STAT_ATTR(split, MTHP_STAT_SPLIT);
 DEFINE_MTHP_STAT_ATTR(split_failed, MTHP_STAT_SPLIT_FAILED);
 DEFINE_MTHP_STAT_ATTR(split_deferred, MTHP_STAT_SPLIT_DEFERRED);
@@ -581,9 +581,9 @@ static struct attribute *stats_attrs[] = {
        &anon_fault_fallback_charge_attr.attr,
        &swpout_attr.attr,
        &swpout_fallback_attr.attr,
-       &file_alloc_attr.attr,
-       &file_fallback_attr.attr,
-       &file_fallback_charge_attr.attr,
+       &shmem_alloc_attr.attr,
+       &shmem_fallback_attr.attr,
+       &shmem_fallback_charge_attr.attr,
        &split_attr.attr,
        &split_failed_attr.attr,
        &split_deferred_attr.attr,
mm/shmem.c +4 -5
index 921d59c3d669..f24dfbd387ba 100644
@@ -1777,7 +1777,7 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
                        if (pages == HPAGE_PMD_NR)
                                count_vm_event(THP_FILE_FALLBACK);
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-                       count_mthp_stat(order, MTHP_STAT_FILE_FALLBACK);
+                       count_mthp_stat(order, MTHP_STAT_SHMEM_FALLBACK);
 #endif
                        order = next_order(&suitable_orders, order);
                }
@@ -1804,8 +1804,8 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
                                count_vm_event(THP_FILE_FALLBACK_CHARGE);
                        }
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-                       count_mthp_stat(folio_order(folio), MTHP_STAT_FILE_FALLBACK);
-                       count_mthp_stat(folio_order(folio), MTHP_STAT_FILE_FALLBACK_CHARGE);
+                       count_mthp_stat(folio_order(folio), MTHP_STAT_SHMEM_FALLBACK);
+                       count_mthp_stat(folio_order(folio), MTHP_STAT_SHMEM_FALLBACK_CHARGE);
 #endif
                }
                goto unlock;
@@ -2181,7 +2181,7 @@ static int shmem_get_folio_gfp(struct inode *inode, pgoff_t index,
                        if (folio_test_pmd_mappable(folio))
                                count_vm_event(THP_FILE_ALLOC);
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-                       count_mthp_stat(folio_order(folio), MTHP_STAT_FILE_ALLOC);
+                       count_mthp_stat(folio_order(folio), MTHP_STAT_SHMEM_ALLOC);
 #endif
                        goto alloced;
                }
--
Quoted 2 lines

2.43.0

Thanks
Barry

Acked-by on v2 1/1
#3 lore raw
In reply to Ryan Roberts
Quoted 18 lines · Ryan Roberts

The legacy PMD-sized THP counters at /proc/vmstat include thp_file_alloc, thp_file_fallback and thp_file_fallback_charge, which rather confusingly refer to shmem THP and do not include any other types of file pages. This is inconsistent since in most other places in the kernel, THP counters are explicitly separated for anon, shmem and file flavours. However, we are stuck with it since it constitutes a user ABI.

Recently, commit 66f44583f9b6 ("mm: shmem: add mTHP counters for anonymous shmem") added equivalent mTHP stats for shmem, keeping the same "file_" prefix in the names. But in future, we may want to add extra stats to cover actual file pages, at which point, it would all become very confusing.

So let's take the opportunity to rename these new counters "shmem_" before the change makes it upstream and the ABI becomes immutable. While we are at it, let's improve the documentation for the legacy counters to make it clear that they count shmem pages only.

Quoted 4 lines

---

Signature · 3 lines
Cheers,

David / dhildenb