[PATCH v3 0/7] Convert all THP vmstat counters to pages

[PATCH v3 0/7] Convert all THP vmstat counters to pages

[Muchun Song]

Hi,

This patch series is aimed to convert all THP vmstat counters to pages.

The unit of some vmstat counters are pages, some are bytes, some are
HPAGE_PMD_NR, and some are KiB. When we want to expose these vmstat
counters to the userspace, we have to know the unit of the vmstat counters
is which one. When the unit is bytes or kB, both clearly distinguishable
by the B/KB suffix. But for the THP vmstat counters, We may make mistakes.

For example, the below is some bug fix for the THP vmstat counters:
  - 7de2e9f195b9 ("mm: memcontrol: correct the NR_ANON_THPS counter of hierarchical memcg")
  - Not committed which is the first commit in this series ("mm: memcontrol: fix NR_ANON_THPS account")

This patch series can make the code clear. And make all the unit of the THP
vmstat counters in pages. Finally, the unit of the vmstat counters are
pages, kB and bytes. The B/KB suffix can tell us that the unit is bytes
or kB. The rest which is without suffix are pages.

As Hugh said, "It does need to be recognized that, with these changes, every
THP stats update overflows the per-cpu counter, resorting to atomic global
updates.

But this change is consistent with 4.7's 8f182270dfec ("mm/swap.c: flush
lru pvecs on compound page arrival"): we accepted greater overhead for
greater accuracy back then, so I think it's okay to do so for THP stats."

This was inspired by Johannes and Roman. Thanks to them.

Changes in v2 -> v3:
  - Change the series subject from "Convert all vmstat counters to pages or bytes"
    to "Convert all THP vmstat counters to pages".
  - Remove convert of KB to B.

Changes in v1 -> v2:
  - Change the series subject from "Convert all THP vmstat counters to pages"
    to "Convert all vmstat counters to pages or bytes".
  - Convert NR_KERNEL_SCS_KB account to bytes.
  - Convert vmstat slab counters to bytes.
  - Remove {global_}node_page_state_pages.

Muchun Song (7):
  mm: memcontrol: fix NR_ANON_THPS account
  mm: memcontrol: convert NR_ANON_THPS account to pages
  mm: memcontrol: convert NR_FILE_THPS account to pages
  mm: memcontrol: convert NR_SHMEM_THPS account to pages
  mm: memcontrol: convert NR_SHMEM_PMDMAPPED account to pages
  mm: memcontrol: convert NR_FILE_PMDMAPPED account to pages
  mm: memcontrol: make the slab calculation consistent

 drivers/base/node.c |  15 ++----
 fs/proc/meminfo.c   |  10 ++--
 mm/filemap.c        |   4 +-
 mm/huge_memory.c    |   9 ++--
 mm/khugepaged.c     |   4 +-
 mm/memcontrol.c     | 139 ++++++++++++++++++++++++++--------------------------
 mm/page_alloc.c     |   7 ++-
 mm/rmap.c           |  19 ++++---
 mm/shmem.c          |   3 +-
 9 files changed, 107 insertions(+), 103 deletions(-)

-- 
2.11.0

[PATCH v3 1/7] mm: memcontrol: fix NR_ANON_THPS account

[Muchun Song]

The unit of NR_ANON_THPS is HPAGE_PMD_NR already. So it should inc/dec
by one rather than nr_pages.

Fixes: 468c398233da ("mm: memcontrol: switch to native NR_ANON_THPS counter")
Signed-off-by: Muchun Song <songmuchun@bytedance.com>
Acked-by: Michal Hocko <mhocko@suse.com>
---
 mm/memcontrol.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index b80328f52fb4..8818bf64d6fe 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5653,10 +5653,8 @@ static int mem_cgroup_move_account(struct page *page,
 			__mod_lruvec_state(from_vec, NR_ANON_MAPPED, -nr_pages);
 			__mod_lruvec_state(to_vec, NR_ANON_MAPPED, nr_pages);
 			if (PageTransHuge(page)) {
-				__mod_lruvec_state(from_vec, NR_ANON_THPS,
-						   -nr_pages);
-				__mod_lruvec_state(to_vec, NR_ANON_THPS,
-						   nr_pages);
+				__dec_lruvec_state(from_vec, NR_ANON_THPS);
+				__inc_lruvec_state(to_vec, NR_ANON_THPS);
 			}
 
 		}
-- 
2.11.0

Re: [PATCH v3 1/7] mm: memcontrol: fix NR_ANON_THPS account

[Roman Gushchin]

On Tue, Dec 08, 2020 at 12:18:41PM +0800, Muchun Song wrote:
> The unit of NR_ANON_THPS is HPAGE_PMD_NR already. So it should inc/dec
> by one rather than nr_pages.
> 
> Fixes: 468c398233da ("mm: memcontrol: switch to native NR_ANON_THPS counter")
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> Acked-by: Michal Hocko <mhocko@suse.com>

Reviewed-by: Roman Gushchin <guro@fb.com>

Thanks!


> ---
>  mm/memcontrol.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index b80328f52fb4..8818bf64d6fe 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -5653,10 +5653,8 @@ static int mem_cgroup_move_account(struct page *page,
>  			__mod_lruvec_state(from_vec, NR_ANON_MAPPED, -nr_pages);
>  			__mod_lruvec_state(to_vec, NR_ANON_MAPPED, nr_pages);
>  			if (PageTransHuge(page)) {
> -				__mod_lruvec_state(from_vec, NR_ANON_THPS,
> -						   -nr_pages);
> -				__mod_lruvec_state(to_vec, NR_ANON_THPS,
> -						   nr_pages);
> +				__dec_lruvec_state(from_vec, NR_ANON_THPS);
> +				__inc_lruvec_state(to_vec, NR_ANON_THPS);
>  			}
>  
>  		}
> -- 
> 2.11.0
> 

[PATCH v3 2/7] mm: memcontrol: convert NR_ANON_THPS account to pages

[Muchun Song]

The unit of NR_ANON_THPS is HPAGE_PMD_NR. Convert the NR_ANON_THPS
account to pages.

Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
 drivers/base/node.c |  3 +--
 fs/proc/meminfo.c   |  2 +-
 mm/huge_memory.c    |  3 ++-
 mm/memcontrol.c     | 20 ++++++--------------
 mm/page_alloc.c     |  2 +-
 mm/rmap.c           |  7 ++++---
 6 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/drivers/base/node.c b/drivers/base/node.c
index 04f71c7bc3f8..ec35cb567940 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -461,8 +461,7 @@ static ssize_t node_read_meminfo(struct device *dev,
 			     nid, K(sunreclaimable)
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 			     ,
-			     nid, K(node_page_state(pgdat, NR_ANON_THPS) *
-				    HPAGE_PMD_NR),
+			     nid, K(node_page_state(pgdat, NR_ANON_THPS)),
 			     nid, K(node_page_state(pgdat, NR_SHMEM_THPS) *
 				    HPAGE_PMD_NR),
 			     nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED) *
diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
index d6fc74619625..a635c8a84ddf 100644
--- a/fs/proc/meminfo.c
+++ b/fs/proc/meminfo.c
@@ -129,7 +129,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 	show_val_kb(m, "AnonHugePages:  ",
-		    global_node_page_state(NR_ANON_THPS) * HPAGE_PMD_NR);
+		    global_node_page_state(NR_ANON_THPS));
 	show_val_kb(m, "ShmemHugePages: ",
 		    global_node_page_state(NR_SHMEM_THPS) * HPAGE_PMD_NR);
 	show_val_kb(m, "ShmemPmdMapped: ",
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 10dd3cae5f53..66ec454120de 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2178,7 +2178,8 @@ static void __split_huge_pmd_locked(struct vm_area_struct *vma, pmd_t *pmd,
 		lock_page_memcg(page);
 		if (atomic_add_negative(-1, compound_mapcount_ptr(page))) {
 			/* Last compound_mapcount is gone. */
-			__dec_lruvec_page_state(page, NR_ANON_THPS);
+			__mod_lruvec_page_state(page, NR_ANON_THPS,
+						-HPAGE_PMD_NR);
 			if (TestClearPageDoubleMap(page)) {
 				/* No need in mapcount reference anymore */
 				for (i = 0; i < HPAGE_PMD_NR; i++)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 8818bf64d6fe..b18e25a5cdf3 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1532,7 +1532,7 @@ static struct memory_stat memory_stats[] = {
 	 * on some architectures, the macro of HPAGE_PMD_SIZE is not
 	 * constant(e.g. powerpc).
 	 */
-	{ "anon_thp", 0, NR_ANON_THPS },
+	{ "anon_thp", PAGE_SIZE, NR_ANON_THPS },
 	{ "file_thp", 0, NR_FILE_THPS },
 	{ "shmem_thp", 0, NR_SHMEM_THPS },
 #endif
@@ -1565,8 +1565,7 @@ static int __init memory_stats_init(void)
 
 	for (i = 0; i < ARRAY_SIZE(memory_stats); i++) {
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-		if (memory_stats[i].idx == NR_ANON_THPS ||
-		    memory_stats[i].idx == NR_FILE_THPS ||
+		if (memory_stats[i].idx == NR_FILE_THPS ||
 		    memory_stats[i].idx == NR_SHMEM_THPS)
 			memory_stats[i].ratio = HPAGE_PMD_SIZE;
 #endif
@@ -4088,10 +4087,6 @@ static int memcg_stat_show(struct seq_file *m, void *v)
 		if (memcg1_stats[i] == MEMCG_SWAP && !do_memsw_account())
 			continue;
 		nr = memcg_page_state_local(memcg, memcg1_stats[i]);
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
-		if (memcg1_stats[i] == NR_ANON_THPS)
-			nr *= HPAGE_PMD_NR;
-#endif
 		seq_printf(m, "%s %lu\n", memcg1_stat_names[i], nr * PAGE_SIZE);
 	}
 
@@ -4122,10 +4117,6 @@ static int memcg_stat_show(struct seq_file *m, void *v)
 		if (memcg1_stats[i] == MEMCG_SWAP && !do_memsw_account())
 			continue;
 		nr = memcg_page_state(memcg, memcg1_stats[i]);
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
-		if (memcg1_stats[i] == NR_ANON_THPS)
-			nr *= HPAGE_PMD_NR;
-#endif
 		seq_printf(m, "total_%s %llu\n", memcg1_stat_names[i],
 						(u64)nr * PAGE_SIZE);
 	}
@@ -5653,10 +5644,11 @@ static int mem_cgroup_move_account(struct page *page,
 			__mod_lruvec_state(from_vec, NR_ANON_MAPPED, -nr_pages);
 			__mod_lruvec_state(to_vec, NR_ANON_MAPPED, nr_pages);
 			if (PageTransHuge(page)) {
-				__dec_lruvec_state(from_vec, NR_ANON_THPS);
-				__inc_lruvec_state(to_vec, NR_ANON_THPS);
+				__mod_lruvec_state(from_vec, NR_ANON_THPS,
+						   -nr_pages);
+				__mod_lruvec_state(to_vec, NR_ANON_THPS,
+						   nr_pages);
 			}
-
 		}
 	} else {
 		__mod_lruvec_state(from_vec, NR_FILE_PAGES, -nr_pages);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 469e28f95ce7..1700f52b7869 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5580,7 +5580,7 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
 			K(node_page_state(pgdat, NR_SHMEM_THPS) * HPAGE_PMD_NR),
 			K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED)
 					* HPAGE_PMD_NR),
-			K(node_page_state(pgdat, NR_ANON_THPS) * HPAGE_PMD_NR),
+			K(node_page_state(pgdat, NR_ANON_THPS)),
 #endif
 			K(node_page_state(pgdat, NR_WRITEBACK_TEMP)),
 			node_page_state(pgdat, NR_KERNEL_STACK_KB),
diff --git a/mm/rmap.c b/mm/rmap.c
index 08c56aaf72eb..f59e92e26b61 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1144,7 +1144,8 @@ void do_page_add_anon_rmap(struct page *page,
 		 * disabled.
 		 */
 		if (compound)
-			__inc_lruvec_page_state(page, NR_ANON_THPS);
+			__mod_lruvec_page_state(page, NR_ANON_THPS,
+						HPAGE_PMD_NR);
 		__mod_lruvec_page_state(page, NR_ANON_MAPPED, nr);
 	}
 
@@ -1186,7 +1187,7 @@ void page_add_new_anon_rmap(struct page *page,
 		if (hpage_pincount_available(page))
 			atomic_set(compound_pincount_ptr(page), 0);
 
-		__inc_lruvec_page_state(page, NR_ANON_THPS);
+		__mod_lruvec_page_state(page, NR_ANON_THPS, HPAGE_PMD_NR);
 	} else {
 		/* Anon THP always mapped first with PMD */
 		VM_BUG_ON_PAGE(PageTransCompound(page), page);
@@ -1292,7 +1293,7 @@ static void page_remove_anon_compound_rmap(struct page *page)
 	if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
 		return;
 
-	__dec_lruvec_page_state(page, NR_ANON_THPS);
+	__mod_lruvec_page_state(page, NR_ANON_THPS, -HPAGE_PMD_NR);
 
 	if (TestClearPageDoubleMap(page)) {
 		/*
-- 
2.11.0

Re: [PATCH v3 2/7] mm: memcontrol: convert NR_ANON_THPS account to pages

[Michal Hocko]

On Tue 08-12-20 12:18:42, Muchun Song wrote:
> The unit of NR_ANON_THPS is HPAGE_PMD_NR. Convert the NR_ANON_THPS
> account to pages.

This changelog could benefit from some improvements. First of all you
should be clear about the motivation. I believe the previous feedback
was also to explicitly mention what effect this has on the pcp
accounting flushing.

> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
>  drivers/base/node.c |  3 +--
>  fs/proc/meminfo.c   |  2 +-
>  mm/huge_memory.c    |  3 ++-
>  mm/memcontrol.c     | 20 ++++++--------------
>  mm/page_alloc.c     |  2 +-
>  mm/rmap.c           |  7 ++++---
>  6 files changed, 15 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index 04f71c7bc3f8..ec35cb567940 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -461,8 +461,7 @@ static ssize_t node_read_meminfo(struct device *dev,
>  			     nid, K(sunreclaimable)
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>  			     ,
> -			     nid, K(node_page_state(pgdat, NR_ANON_THPS) *
> -				    HPAGE_PMD_NR),
> +			     nid, K(node_page_state(pgdat, NR_ANON_THPS)),
>  			     nid, K(node_page_state(pgdat, NR_SHMEM_THPS) *
>  				    HPAGE_PMD_NR),
>  			     nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED) *
> diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
> index d6fc74619625..a635c8a84ddf 100644
> --- a/fs/proc/meminfo.c
> +++ b/fs/proc/meminfo.c
> @@ -129,7 +129,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
>  
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>  	show_val_kb(m, "AnonHugePages:  ",
> -		    global_node_page_state(NR_ANON_THPS) * HPAGE_PMD_NR);
> +		    global_node_page_state(NR_ANON_THPS));
>  	show_val_kb(m, "ShmemHugePages: ",
>  		    global_node_page_state(NR_SHMEM_THPS) * HPAGE_PMD_NR);
>  	show_val_kb(m, "ShmemPmdMapped: ",
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 10dd3cae5f53..66ec454120de 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2178,7 +2178,8 @@ static void __split_huge_pmd_locked(struct vm_area_struct *vma, pmd_t *pmd,
>  		lock_page_memcg(page);
>  		if (atomic_add_negative(-1, compound_mapcount_ptr(page))) {
>  			/* Last compound_mapcount is gone. */
> -			__dec_lruvec_page_state(page, NR_ANON_THPS);
> +			__mod_lruvec_page_state(page, NR_ANON_THPS,
> +						-HPAGE_PMD_NR);
>  			if (TestClearPageDoubleMap(page)) {
>  				/* No need in mapcount reference anymore */
>  				for (i = 0; i < HPAGE_PMD_NR; i++)
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 8818bf64d6fe..b18e25a5cdf3 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1532,7 +1532,7 @@ static struct memory_stat memory_stats[] = {
>  	 * on some architectures, the macro of HPAGE_PMD_SIZE is not
>  	 * constant(e.g. powerpc).
>  	 */
> -	{ "anon_thp", 0, NR_ANON_THPS },
> +	{ "anon_thp", PAGE_SIZE, NR_ANON_THPS },
>  	{ "file_thp", 0, NR_FILE_THPS },
>  	{ "shmem_thp", 0, NR_SHMEM_THPS },
>  #endif
> @@ -1565,8 +1565,7 @@ static int __init memory_stats_init(void)
>  
>  	for (i = 0; i < ARRAY_SIZE(memory_stats); i++) {
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -		if (memory_stats[i].idx == NR_ANON_THPS ||
> -		    memory_stats[i].idx == NR_FILE_THPS ||
> +		if (memory_stats[i].idx == NR_FILE_THPS ||
>  		    memory_stats[i].idx == NR_SHMEM_THPS)
>  			memory_stats[i].ratio = HPAGE_PMD_SIZE;
>  #endif
> @@ -4088,10 +4087,6 @@ static int memcg_stat_show(struct seq_file *m, void *v)
>  		if (memcg1_stats[i] == MEMCG_SWAP && !do_memsw_account())
>  			continue;
>  		nr = memcg_page_state_local(memcg, memcg1_stats[i]);
> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -		if (memcg1_stats[i] == NR_ANON_THPS)
> -			nr *= HPAGE_PMD_NR;
> -#endif
>  		seq_printf(m, "%s %lu\n", memcg1_stat_names[i], nr * PAGE_SIZE);
>  	}
>  
> @@ -4122,10 +4117,6 @@ static int memcg_stat_show(struct seq_file *m, void *v)
>  		if (memcg1_stats[i] == MEMCG_SWAP && !do_memsw_account())
>  			continue;
>  		nr = memcg_page_state(memcg, memcg1_stats[i]);
> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -		if (memcg1_stats[i] == NR_ANON_THPS)
> -			nr *= HPAGE_PMD_NR;
> -#endif
>  		seq_printf(m, "total_%s %llu\n", memcg1_stat_names[i],
>  						(u64)nr * PAGE_SIZE);
>  	}
> @@ -5653,10 +5644,11 @@ static int mem_cgroup_move_account(struct page *page,
>  			__mod_lruvec_state(from_vec, NR_ANON_MAPPED, -nr_pages);
>  			__mod_lruvec_state(to_vec, NR_ANON_MAPPED, nr_pages);
>  			if (PageTransHuge(page)) {
> -				__dec_lruvec_state(from_vec, NR_ANON_THPS);
> -				__inc_lruvec_state(to_vec, NR_ANON_THPS);
> +				__mod_lruvec_state(from_vec, NR_ANON_THPS,
> +						   -nr_pages);
> +				__mod_lruvec_state(to_vec, NR_ANON_THPS,
> +						   nr_pages);
>  			}
> -
>  		}
>  	} else {
>  		__mod_lruvec_state(from_vec, NR_FILE_PAGES, -nr_pages);
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 469e28f95ce7..1700f52b7869 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5580,7 +5580,7 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
>  			K(node_page_state(pgdat, NR_SHMEM_THPS) * HPAGE_PMD_NR),
>  			K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED)
>  					* HPAGE_PMD_NR),
> -			K(node_page_state(pgdat, NR_ANON_THPS) * HPAGE_PMD_NR),
> +			K(node_page_state(pgdat, NR_ANON_THPS)),
>  #endif
>  			K(node_page_state(pgdat, NR_WRITEBACK_TEMP)),
>  			node_page_state(pgdat, NR_KERNEL_STACK_KB),
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 08c56aaf72eb..f59e92e26b61 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1144,7 +1144,8 @@ void do_page_add_anon_rmap(struct page *page,
>  		 * disabled.
>  		 */
>  		if (compound)
> -			__inc_lruvec_page_state(page, NR_ANON_THPS);
> +			__mod_lruvec_page_state(page, NR_ANON_THPS,
> +						HPAGE_PMD_NR);
>  		__mod_lruvec_page_state(page, NR_ANON_MAPPED, nr);
>  	}
>  
> @@ -1186,7 +1187,7 @@ void page_add_new_anon_rmap(struct page *page,
>  		if (hpage_pincount_available(page))
>  			atomic_set(compound_pincount_ptr(page), 0);
>  
> -		__inc_lruvec_page_state(page, NR_ANON_THPS);
> +		__mod_lruvec_page_state(page, NR_ANON_THPS, HPAGE_PMD_NR);
>  	} else {
>  		/* Anon THP always mapped first with PMD */
>  		VM_BUG_ON_PAGE(PageTransCompound(page), page);
> @@ -1292,7 +1293,7 @@ static void page_remove_anon_compound_rmap(struct page *page)
>  	if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
>  		return;
>  
> -	__dec_lruvec_page_state(page, NR_ANON_THPS);
> +	__mod_lruvec_page_state(page, NR_ANON_THPS, -HPAGE_PMD_NR);
>  
>  	if (TestClearPageDoubleMap(page)) {
>  		/*
> -- 
> 2.11.0

-- 
Michal Hocko
SUSE Labs

Re: [External] Re: [PATCH v3 2/7] mm: memcontrol: convert NR_ANON_THPS account to pages

[Muchun Song]

On Tue, Dec 15, 2020 at 9:30 PM Michal Hocko <mhocko@suse.com> wrote:
>
> On Tue 08-12-20 12:18:42, Muchun Song wrote:
> > The unit of NR_ANON_THPS is HPAGE_PMD_NR. Convert the NR_ANON_THPS
> > account to pages.
>
> This changelog could benefit from some improvements. First of all you
> should be clear about the motivation. I believe the previous feedback
> was also to explicitly mention what effect this has on the pcp
> accounting flushing.

Thank you. Will update.

>
> > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> > ---
> >  drivers/base/node.c |  3 +--
> >  fs/proc/meminfo.c   |  2 +-
> >  mm/huge_memory.c    |  3 ++-
> >  mm/memcontrol.c     | 20 ++++++--------------
> >  mm/page_alloc.c     |  2 +-
> >  mm/rmap.c           |  7 ++++---
> >  6 files changed, 15 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/base/node.c b/drivers/base/node.c
> > index 04f71c7bc3f8..ec35cb567940 100644
> > --- a/drivers/base/node.c
> > +++ b/drivers/base/node.c
> > @@ -461,8 +461,7 @@ static ssize_t node_read_meminfo(struct device *dev,
> >                            nid, K(sunreclaimable)
> >  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> >                            ,
> > -                          nid, K(node_page_state(pgdat, NR_ANON_THPS) *
> > -                                 HPAGE_PMD_NR),
> > +                          nid, K(node_page_state(pgdat, NR_ANON_THPS)),
> >                            nid, K(node_page_state(pgdat, NR_SHMEM_THPS) *
> >                                   HPAGE_PMD_NR),
> >                            nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED) *
> > diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
> > index d6fc74619625..a635c8a84ddf 100644
> > --- a/fs/proc/meminfo.c
> > +++ b/fs/proc/meminfo.c
> > @@ -129,7 +129,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
> >
> >  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> >       show_val_kb(m, "AnonHugePages:  ",
> > -                 global_node_page_state(NR_ANON_THPS) * HPAGE_PMD_NR);
> > +                 global_node_page_state(NR_ANON_THPS));
> >       show_val_kb(m, "ShmemHugePages: ",
> >                   global_node_page_state(NR_SHMEM_THPS) * HPAGE_PMD_NR);
> >       show_val_kb(m, "ShmemPmdMapped: ",
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 10dd3cae5f53..66ec454120de 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -2178,7 +2178,8 @@ static void __split_huge_pmd_locked(struct vm_area_struct *vma, pmd_t *pmd,
> >               lock_page_memcg(page);
> >               if (atomic_add_negative(-1, compound_mapcount_ptr(page))) {
> >                       /* Last compound_mapcount is gone. */
> > -                     __dec_lruvec_page_state(page, NR_ANON_THPS);
> > +                     __mod_lruvec_page_state(page, NR_ANON_THPS,
> > +                                             -HPAGE_PMD_NR);
> >                       if (TestClearPageDoubleMap(page)) {
> >                               /* No need in mapcount reference anymore */
> >                               for (i = 0; i < HPAGE_PMD_NR; i++)
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 8818bf64d6fe..b18e25a5cdf3 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -1532,7 +1532,7 @@ static struct memory_stat memory_stats[] = {
> >        * on some architectures, the macro of HPAGE_PMD_SIZE is not
> >        * constant(e.g. powerpc).
> >        */
> > -     { "anon_thp", 0, NR_ANON_THPS },
> > +     { "anon_thp", PAGE_SIZE, NR_ANON_THPS },
> >       { "file_thp", 0, NR_FILE_THPS },
> >       { "shmem_thp", 0, NR_SHMEM_THPS },
> >  #endif
> > @@ -1565,8 +1565,7 @@ static int __init memory_stats_init(void)
> >
> >       for (i = 0; i < ARRAY_SIZE(memory_stats); i++) {
> >  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> > -             if (memory_stats[i].idx == NR_ANON_THPS ||
> > -                 memory_stats[i].idx == NR_FILE_THPS ||
> > +             if (memory_stats[i].idx == NR_FILE_THPS ||
> >                   memory_stats[i].idx == NR_SHMEM_THPS)
> >                       memory_stats[i].ratio = HPAGE_PMD_SIZE;
> >  #endif
> > @@ -4088,10 +4087,6 @@ static int memcg_stat_show(struct seq_file *m, void *v)
> >               if (memcg1_stats[i] == MEMCG_SWAP && !do_memsw_account())
> >                       continue;
> >               nr = memcg_page_state_local(memcg, memcg1_stats[i]);
> > -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> > -             if (memcg1_stats[i] == NR_ANON_THPS)
> > -                     nr *= HPAGE_PMD_NR;
> > -#endif
> >               seq_printf(m, "%s %lu\n", memcg1_stat_names[i], nr * PAGE_SIZE);
> >       }
> >
> > @@ -4122,10 +4117,6 @@ static int memcg_stat_show(struct seq_file *m, void *v)
> >               if (memcg1_stats[i] == MEMCG_SWAP && !do_memsw_account())
> >                       continue;
> >               nr = memcg_page_state(memcg, memcg1_stats[i]);
> > -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> > -             if (memcg1_stats[i] == NR_ANON_THPS)
> > -                     nr *= HPAGE_PMD_NR;
> > -#endif
> >               seq_printf(m, "total_%s %llu\n", memcg1_stat_names[i],
> >                                               (u64)nr * PAGE_SIZE);
> >       }
> > @@ -5653,10 +5644,11 @@ static int mem_cgroup_move_account(struct page *page,
> >                       __mod_lruvec_state(from_vec, NR_ANON_MAPPED, -nr_pages);
> >                       __mod_lruvec_state(to_vec, NR_ANON_MAPPED, nr_pages);
> >                       if (PageTransHuge(page)) {
> > -                             __dec_lruvec_state(from_vec, NR_ANON_THPS);
> > -                             __inc_lruvec_state(to_vec, NR_ANON_THPS);
> > +                             __mod_lruvec_state(from_vec, NR_ANON_THPS,
> > +                                                -nr_pages);
> > +                             __mod_lruvec_state(to_vec, NR_ANON_THPS,
> > +                                                nr_pages);
> >                       }
> > -
> >               }
> >       } else {
> >               __mod_lruvec_state(from_vec, NR_FILE_PAGES, -nr_pages);
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 469e28f95ce7..1700f52b7869 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -5580,7 +5580,7 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
> >                       K(node_page_state(pgdat, NR_SHMEM_THPS) * HPAGE_PMD_NR),
> >                       K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED)
> >                                       * HPAGE_PMD_NR),
> > -                     K(node_page_state(pgdat, NR_ANON_THPS) * HPAGE_PMD_NR),
> > +                     K(node_page_state(pgdat, NR_ANON_THPS)),
> >  #endif
> >                       K(node_page_state(pgdat, NR_WRITEBACK_TEMP)),
> >                       node_page_state(pgdat, NR_KERNEL_STACK_KB),
> > diff --git a/mm/rmap.c b/mm/rmap.c
> > index 08c56aaf72eb..f59e92e26b61 100644
> > --- a/mm/rmap.c
> > +++ b/mm/rmap.c
> > @@ -1144,7 +1144,8 @@ void do_page_add_anon_rmap(struct page *page,
> >                * disabled.
> >                */
> >               if (compound)
> > -                     __inc_lruvec_page_state(page, NR_ANON_THPS);
> > +                     __mod_lruvec_page_state(page, NR_ANON_THPS,
> > +                                             HPAGE_PMD_NR);
> >               __mod_lruvec_page_state(page, NR_ANON_MAPPED, nr);
> >       }
> >
> > @@ -1186,7 +1187,7 @@ void page_add_new_anon_rmap(struct page *page,
> >               if (hpage_pincount_available(page))
> >                       atomic_set(compound_pincount_ptr(page), 0);
> >
> > -             __inc_lruvec_page_state(page, NR_ANON_THPS);
> > +             __mod_lruvec_page_state(page, NR_ANON_THPS, HPAGE_PMD_NR);
> >       } else {
> >               /* Anon THP always mapped first with PMD */
> >               VM_BUG_ON_PAGE(PageTransCompound(page), page);
> > @@ -1292,7 +1293,7 @@ static void page_remove_anon_compound_rmap(struct page *page)
> >       if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
> >               return;
> >
> > -     __dec_lruvec_page_state(page, NR_ANON_THPS);
> > +     __mod_lruvec_page_state(page, NR_ANON_THPS, -HPAGE_PMD_NR);
> >
> >       if (TestClearPageDoubleMap(page)) {
> >               /*
> > --
> > 2.11.0
>
> --
> Michal Hocko
> SUSE Labs



--
Yours,
Muchun

[PATCH v3 3/7] mm: memcontrol: convert NR_FILE_THPS account to pages

[Muchun Song]

The unit of NR_FILE_THPS is HPAGE_PMD_NR. Converrt NR_FILE_THPS
account to pages.

Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
 drivers/base/node.c | 3 +--
 fs/proc/meminfo.c   | 2 +-
 mm/filemap.c        | 2 +-
 mm/huge_memory.c    | 3 ++-
 mm/khugepaged.c     | 2 +-
 mm/memcontrol.c     | 5 ++---
 6 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/base/node.c b/drivers/base/node.c
index ec35cb567940..0f752d0fce6f 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -466,8 +466,7 @@ static ssize_t node_read_meminfo(struct device *dev,
 				    HPAGE_PMD_NR),
 			     nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED) *
 				    HPAGE_PMD_NR),
-			     nid, K(node_page_state(pgdat, NR_FILE_THPS) *
-				    HPAGE_PMD_NR),
+			     nid, K(node_page_state(pgdat, NR_FILE_THPS)),
 			     nid, K(node_page_state(pgdat, NR_FILE_PMDMAPPED) *
 				    HPAGE_PMD_NR)
 #endif
diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
index a635c8a84ddf..7ea4679880c8 100644
--- a/fs/proc/meminfo.c
+++ b/fs/proc/meminfo.c
@@ -135,7 +135,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
 	show_val_kb(m, "ShmemPmdMapped: ",
 		    global_node_page_state(NR_SHMEM_PMDMAPPED) * HPAGE_PMD_NR);
 	show_val_kb(m, "FileHugePages:  ",
-		    global_node_page_state(NR_FILE_THPS) * HPAGE_PMD_NR);
+		    global_node_page_state(NR_FILE_THPS));
 	show_val_kb(m, "FilePmdMapped:  ",
 		    global_node_page_state(NR_FILE_PMDMAPPED) * HPAGE_PMD_NR);
 #endif
diff --git a/mm/filemap.c b/mm/filemap.c
index 78090ee08ac2..9cc8b3ac9eac 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -207,7 +207,7 @@ static void unaccount_page_cache_page(struct address_space *mapping,
 		if (PageTransHuge(page))
 			__dec_lruvec_page_state(page, NR_SHMEM_THPS);
 	} else if (PageTransHuge(page)) {
-		__dec_lruvec_page_state(page, NR_FILE_THPS);
+		__mod_lruvec_page_state(page, NR_FILE_THPS, -HPAGE_PMD_NR);
 		filemap_nr_thps_dec(mapping);
 	}
 
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 66ec454120de..1e24165fa53a 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2748,7 +2748,8 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
 			if (PageSwapBacked(head))
 				__dec_lruvec_page_state(head, NR_SHMEM_THPS);
 			else
-				__dec_lruvec_page_state(head, NR_FILE_THPS);
+				__mod_lruvec_page_state(head, NR_FILE_THPS,
+							-HPAGE_PMD_NR);
 		}
 
 		__split_huge_page(page, list, end);
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 494d3cb0b58a..76b3e064a72a 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1869,7 +1869,7 @@ static void collapse_file(struct mm_struct *mm,
 	if (is_shmem)
 		__inc_lruvec_page_state(new_page, NR_SHMEM_THPS);
 	else {
-		__inc_lruvec_page_state(new_page, NR_FILE_THPS);
+		__mod_lruvec_page_state(new_page, NR_FILE_THPS, HPAGE_PMD_NR);
 		filemap_nr_thps_inc(mapping);
 	}
 
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index b18e25a5cdf3..04985c8c6a0a 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1533,7 +1533,7 @@ static struct memory_stat memory_stats[] = {
 	 * constant(e.g. powerpc).
 	 */
 	{ "anon_thp", PAGE_SIZE, NR_ANON_THPS },
-	{ "file_thp", 0, NR_FILE_THPS },
+	{ "file_thp", PAGE_SIZE, NR_FILE_THPS },
 	{ "shmem_thp", 0, NR_SHMEM_THPS },
 #endif
 	{ "inactive_anon", PAGE_SIZE, NR_INACTIVE_ANON },
@@ -1565,8 +1565,7 @@ static int __init memory_stats_init(void)
 
 	for (i = 0; i < ARRAY_SIZE(memory_stats); i++) {
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-		if (memory_stats[i].idx == NR_FILE_THPS ||
-		    memory_stats[i].idx == NR_SHMEM_THPS)
+		if (memory_stats[i].idx == NR_SHMEM_THPS)
 			memory_stats[i].ratio = HPAGE_PMD_SIZE;
 #endif
 		VM_BUG_ON(!memory_stats[i].ratio);
-- 
2.11.0

[PATCH v3 4/7] mm: memcontrol: convert NR_SHMEM_THPS account to pages

[Muchun Song]

The unit of NR_SHMEM_THPS is HPAGE_PMD_NR. Convert NR_SHMEM_THPS
account to pages

Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
 drivers/base/node.c |  3 +--
 fs/proc/meminfo.c   |  2 +-
 mm/filemap.c        |  2 +-
 mm/huge_memory.c    |  3 ++-
 mm/khugepaged.c     |  2 +-
 mm/memcontrol.c     | 26 ++------------------------
 mm/page_alloc.c     |  2 +-
 mm/shmem.c          |  3 ++-
 8 files changed, 11 insertions(+), 32 deletions(-)

diff --git a/drivers/base/node.c b/drivers/base/node.c
index 0f752d0fce6f..a7f298ae693a 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -462,8 +462,7 @@ static ssize_t node_read_meminfo(struct device *dev,
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 			     ,
 			     nid, K(node_page_state(pgdat, NR_ANON_THPS)),
-			     nid, K(node_page_state(pgdat, NR_SHMEM_THPS) *
-				    HPAGE_PMD_NR),
+			     nid, K(node_page_state(pgdat, NR_SHMEM_THPS)),
 			     nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED) *
 				    HPAGE_PMD_NR),
 			     nid, K(node_page_state(pgdat, NR_FILE_THPS)),
diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
index 7ea4679880c8..cfb107eaa3e6 100644
--- a/fs/proc/meminfo.c
+++ b/fs/proc/meminfo.c
@@ -131,7 +131,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
 	show_val_kb(m, "AnonHugePages:  ",
 		    global_node_page_state(NR_ANON_THPS));
 	show_val_kb(m, "ShmemHugePages: ",
-		    global_node_page_state(NR_SHMEM_THPS) * HPAGE_PMD_NR);
+		    global_node_page_state(NR_SHMEM_THPS));
 	show_val_kb(m, "ShmemPmdMapped: ",
 		    global_node_page_state(NR_SHMEM_PMDMAPPED) * HPAGE_PMD_NR);
 	show_val_kb(m, "FileHugePages:  ",
diff --git a/mm/filemap.c b/mm/filemap.c
index 9cc8b3ac9eac..c653717b92b6 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -205,7 +205,7 @@ static void unaccount_page_cache_page(struct address_space *mapping,
 	if (PageSwapBacked(page)) {
 		__mod_lruvec_page_state(page, NR_SHMEM, -nr);
 		if (PageTransHuge(page))
-			__dec_lruvec_page_state(page, NR_SHMEM_THPS);
+			__mod_lruvec_page_state(page, NR_SHMEM_THPS, -HPAGE_PMD_NR);
 	} else if (PageTransHuge(page)) {
 		__mod_lruvec_page_state(page, NR_FILE_THPS, -HPAGE_PMD_NR);
 		filemap_nr_thps_dec(mapping);
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 1e24165fa53a..ac552807492e 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2746,7 +2746,8 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
 		spin_unlock(&ds_queue->split_queue_lock);
 		if (mapping) {
 			if (PageSwapBacked(head))
-				__dec_lruvec_page_state(head, NR_SHMEM_THPS);
+				__mod_lruvec_page_state(head, NR_SHMEM_THPS,
+							-HPAGE_PMD_NR);
 			else
 				__mod_lruvec_page_state(head, NR_FILE_THPS,
 							-HPAGE_PMD_NR);
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 76b3e064a72a..2f1c22331188 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1867,7 +1867,7 @@ static void collapse_file(struct mm_struct *mm,
 	}
 
 	if (is_shmem)
-		__inc_lruvec_page_state(new_page, NR_SHMEM_THPS);
+		__mod_lruvec_page_state(new_page, NR_SHMEM_THPS, HPAGE_PMD_NR);
 	else {
 		__mod_lruvec_page_state(new_page, NR_FILE_THPS, HPAGE_PMD_NR);
 		filemap_nr_thps_inc(mapping);
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 04985c8c6a0a..a40797a27f87 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1515,7 +1515,7 @@ struct memory_stat {
 	unsigned int idx;
 };
 
-static struct memory_stat memory_stats[] = {
+static const struct memory_stat memory_stats[] = {
 	{ "anon", PAGE_SIZE, NR_ANON_MAPPED },
 	{ "file", PAGE_SIZE, NR_FILE_PAGES },
 	{ "kernel_stack", 1024, NR_KERNEL_STACK_KB },
@@ -1527,14 +1527,9 @@ static struct memory_stat memory_stats[] = {
 	{ "file_dirty", PAGE_SIZE, NR_FILE_DIRTY },
 	{ "file_writeback", PAGE_SIZE, NR_WRITEBACK },
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-	/*
-	 * The ratio will be initialized in memory_stats_init(). Because
-	 * on some architectures, the macro of HPAGE_PMD_SIZE is not
-	 * constant(e.g. powerpc).
-	 */
 	{ "anon_thp", PAGE_SIZE, NR_ANON_THPS },
 	{ "file_thp", PAGE_SIZE, NR_FILE_THPS },
-	{ "shmem_thp", 0, NR_SHMEM_THPS },
+	{ "shmem_thp", PAGE_SIZE, NR_SHMEM_THPS },
 #endif
 	{ "inactive_anon", PAGE_SIZE, NR_INACTIVE_ANON },
 	{ "active_anon", PAGE_SIZE, NR_ACTIVE_ANON },
@@ -1559,23 +1554,6 @@ static struct memory_stat memory_stats[] = {
 	{ "workingset_nodereclaim", 1, WORKINGSET_NODERECLAIM },
 };
 
-static int __init memory_stats_init(void)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(memory_stats); i++) {
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
-		if (memory_stats[i].idx == NR_SHMEM_THPS)
-			memory_stats[i].ratio = HPAGE_PMD_SIZE;
-#endif
-		VM_BUG_ON(!memory_stats[i].ratio);
-		VM_BUG_ON(memory_stats[i].idx >= MEMCG_NR_STAT);
-	}
-
-	return 0;
-}
-pure_initcall(memory_stats_init);
-
 static char *memory_stat_format(struct mem_cgroup *memcg)
 {
 	struct seq_buf s;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 1700f52b7869..720fb5a220b6 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5577,7 +5577,7 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
 			K(node_page_state(pgdat, NR_WRITEBACK)),
 			K(node_page_state(pgdat, NR_SHMEM)),
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-			K(node_page_state(pgdat, NR_SHMEM_THPS) * HPAGE_PMD_NR),
+			K(node_page_state(pgdat, NR_SHMEM_THPS)),
 			K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED)
 					* HPAGE_PMD_NR),
 			K(node_page_state(pgdat, NR_ANON_THPS)),
diff --git a/mm/shmem.c b/mm/shmem.c
index 53d84d2c9fe5..b2ca3beabc19 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -713,7 +713,8 @@ static int shmem_add_to_page_cache(struct page *page,
 		}
 		if (PageTransHuge(page)) {
 			count_vm_event(THP_FILE_ALLOC);
-			__inc_lruvec_page_state(page, NR_SHMEM_THPS);
+			__mod_lruvec_page_state(page, NR_SHMEM_THPS,
+						HPAGE_PMD_NR);
 		}
 		mapping->nrpages += nr;
 		__mod_lruvec_page_state(page, NR_FILE_PAGES, nr);
-- 
2.11.0

[PATCH v3 5/7] mm: memcontrol: convert NR_SHMEM_PMDMAPPED account to pages

[Muchun Song]

The unit of NR_SHMEM_PMDMAPPED is HPAGE_PMD_NR. Convert NR_SHMEM_PMDMAPPED
account to pages.

Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
 drivers/base/node.c | 3 +--
 fs/proc/meminfo.c   | 2 +-
 mm/page_alloc.c     | 3 +--
 mm/rmap.c           | 6 ++++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/base/node.c b/drivers/base/node.c
index a7f298ae693a..45293084bb19 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -463,8 +463,7 @@ static ssize_t node_read_meminfo(struct device *dev,
 			     ,
 			     nid, K(node_page_state(pgdat, NR_ANON_THPS)),
 			     nid, K(node_page_state(pgdat, NR_SHMEM_THPS)),
-			     nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED) *
-				    HPAGE_PMD_NR),
+			     nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED)),
 			     nid, K(node_page_state(pgdat, NR_FILE_THPS)),
 			     nid, K(node_page_state(pgdat, NR_FILE_PMDMAPPED) *
 				    HPAGE_PMD_NR)
diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
index cfb107eaa3e6..c61f440570f9 100644
--- a/fs/proc/meminfo.c
+++ b/fs/proc/meminfo.c
@@ -133,7 +133,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
 	show_val_kb(m, "ShmemHugePages: ",
 		    global_node_page_state(NR_SHMEM_THPS));
 	show_val_kb(m, "ShmemPmdMapped: ",
-		    global_node_page_state(NR_SHMEM_PMDMAPPED) * HPAGE_PMD_NR);
+		    global_node_page_state(NR_SHMEM_PMDMAPPED));
 	show_val_kb(m, "FileHugePages:  ",
 		    global_node_page_state(NR_FILE_THPS));
 	show_val_kb(m, "FilePmdMapped:  ",
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 720fb5a220b6..575fbfeea4b5 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5578,8 +5578,7 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
 			K(node_page_state(pgdat, NR_SHMEM)),
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 			K(node_page_state(pgdat, NR_SHMEM_THPS)),
-			K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED)
-					* HPAGE_PMD_NR),
+			K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED)),
 			K(node_page_state(pgdat, NR_ANON_THPS)),
 #endif
 			K(node_page_state(pgdat, NR_WRITEBACK_TEMP)),
diff --git a/mm/rmap.c b/mm/rmap.c
index f59e92e26b61..3089ad6bf468 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1219,7 +1219,8 @@ void page_add_file_rmap(struct page *page, bool compound)
 		if (!atomic_inc_and_test(compound_mapcount_ptr(page)))
 			goto out;
 		if (PageSwapBacked(page))
-			__inc_node_page_state(page, NR_SHMEM_PMDMAPPED);
+			__mod_lruvec_page_state(page, NR_SHMEM_PMDMAPPED,
+						HPAGE_PMD_NR);
 		else
 			__inc_node_page_state(page, NR_FILE_PMDMAPPED);
 	} else {
@@ -1260,7 +1261,8 @@ static void page_remove_file_rmap(struct page *page, bool compound)
 		if (!atomic_add_negative(-1, compound_mapcount_ptr(page)))
 			return;
 		if (PageSwapBacked(page))
-			__dec_node_page_state(page, NR_SHMEM_PMDMAPPED);
+			__mod_lruvec_page_state(page, NR_SHMEM_PMDMAPPED,
+						-HPAGE_PMD_NR);
 		else
 			__dec_node_page_state(page, NR_FILE_PMDMAPPED);
 	} else {
-- 
2.11.0

[PATCH v3 6/7] mm: memcontrol: convert NR_FILE_PMDMAPPED account to pages

[Muchun Song]

The unit of NR_FILE_PMDMAPPED is HPAGE_PMD_NR. Convert NR_FILE_PMDMAPPED
account to pages.

Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
 drivers/base/node.c | 3 +--
 fs/proc/meminfo.c   | 2 +-
 mm/rmap.c           | 6 ++++--
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/base/node.c b/drivers/base/node.c
index 45293084bb19..04505beca104 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -465,8 +465,7 @@ static ssize_t node_read_meminfo(struct device *dev,
 			     nid, K(node_page_state(pgdat, NR_SHMEM_THPS)),
 			     nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED)),
 			     nid, K(node_page_state(pgdat, NR_FILE_THPS)),
-			     nid, K(node_page_state(pgdat, NR_FILE_PMDMAPPED) *
-				    HPAGE_PMD_NR)
+			     nid, K(node_page_state(pgdat, NR_FILE_PMDMAPPED))
 #endif
 			    );
 	len += hugetlb_report_node_meminfo(buf, len, nid);
diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
index c61f440570f9..6fa761c9cc78 100644
--- a/fs/proc/meminfo.c
+++ b/fs/proc/meminfo.c
@@ -137,7 +137,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
 	show_val_kb(m, "FileHugePages:  ",
 		    global_node_page_state(NR_FILE_THPS));
 	show_val_kb(m, "FilePmdMapped:  ",
-		    global_node_page_state(NR_FILE_PMDMAPPED) * HPAGE_PMD_NR);
+		    global_node_page_state(NR_FILE_PMDMAPPED));
 #endif
 
 #ifdef CONFIG_CMA
diff --git a/mm/rmap.c b/mm/rmap.c
index 3089ad6bf468..e383c5619501 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1222,7 +1222,8 @@ void page_add_file_rmap(struct page *page, bool compound)
 			__mod_lruvec_page_state(page, NR_SHMEM_PMDMAPPED,
 						HPAGE_PMD_NR);
 		else
-			__inc_node_page_state(page, NR_FILE_PMDMAPPED);
+			__mod_lruvec_page_state(page, NR_FILE_PMDMAPPED,
+						HPAGE_PMD_NR);
 	} else {
 		if (PageTransCompound(page) && page_mapping(page)) {
 			VM_WARN_ON_ONCE(!PageLocked(page));
@@ -1264,7 +1265,8 @@ static void page_remove_file_rmap(struct page *page, bool compound)
 			__mod_lruvec_page_state(page, NR_SHMEM_PMDMAPPED,
 						-HPAGE_PMD_NR);
 		else
-			__dec_node_page_state(page, NR_FILE_PMDMAPPED);
+			__mod_lruvec_page_state(page, NR_FILE_PMDMAPPED,
+						-HPAGE_PMD_NR);
 	} else {
 		if (!atomic_add_negative(-1, &page->_mapcount))
 			return;
-- 
2.11.0

[PATCH v3 7/7] mm: memcontrol: make the slab calculation consistent

[Muchun Song]

Although the ratio of the slab is one, we also should read the ratio
from the related memory_stats instead of hard-coding. And the local
variable of size is already the value of slab_unreclaimable. So we
do not need to read again.

We can drop the ratio in struct memory_stat. This can make the code
clean and simple. And get rid of the awkward mix of static and runtime
initialization of the memory_stats table.

Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
 mm/memcontrol.c | 112 ++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 73 insertions(+), 39 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index a40797a27f87..841ea37cc123 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1511,49 +1511,78 @@ static bool mem_cgroup_wait_acct_move(struct mem_cgroup *memcg)
 
 struct memory_stat {
 	const char *name;
-	unsigned int ratio;
 	unsigned int idx;
 };
 
 static const struct memory_stat memory_stats[] = {
-	{ "anon", PAGE_SIZE, NR_ANON_MAPPED },
-	{ "file", PAGE_SIZE, NR_FILE_PAGES },
-	{ "kernel_stack", 1024, NR_KERNEL_STACK_KB },
-	{ "pagetables", PAGE_SIZE, NR_PAGETABLE },
-	{ "percpu", 1, MEMCG_PERCPU_B },
-	{ "sock", PAGE_SIZE, MEMCG_SOCK },
-	{ "shmem", PAGE_SIZE, NR_SHMEM },
-	{ "file_mapped", PAGE_SIZE, NR_FILE_MAPPED },
-	{ "file_dirty", PAGE_SIZE, NR_FILE_DIRTY },
-	{ "file_writeback", PAGE_SIZE, NR_WRITEBACK },
+	{ "anon",			NR_ANON_MAPPED			},
+	{ "file",			NR_FILE_PAGES			},
+	{ "kernel_stack",		NR_KERNEL_STACK_KB		},
+	{ "pagetables",			NR_PAGETABLE			},
+	{ "percpu",			MEMCG_PERCPU_B			},
+	{ "sock",			MEMCG_SOCK			},
+	{ "shmem",			NR_SHMEM			},
+	{ "file_mapped",		NR_FILE_MAPPED			},
+	{ "file_dirty",			NR_FILE_DIRTY			},
+	{ "file_writeback",		NR_WRITEBACK			},
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-	{ "anon_thp", PAGE_SIZE, NR_ANON_THPS },
-	{ "file_thp", PAGE_SIZE, NR_FILE_THPS },
-	{ "shmem_thp", PAGE_SIZE, NR_SHMEM_THPS },
+	{ "anon_thp",			NR_ANON_THPS			},
+	{ "file_thp",			NR_FILE_THPS			},
+	{ "shmem_thp",			NR_SHMEM_THPS			},
 #endif
-	{ "inactive_anon", PAGE_SIZE, NR_INACTIVE_ANON },
-	{ "active_anon", PAGE_SIZE, NR_ACTIVE_ANON },
-	{ "inactive_file", PAGE_SIZE, NR_INACTIVE_FILE },
-	{ "active_file", PAGE_SIZE, NR_ACTIVE_FILE },
-	{ "unevictable", PAGE_SIZE, NR_UNEVICTABLE },
-
-	/*
-	 * Note: The slab_reclaimable and slab_unreclaimable must be
-	 * together and slab_reclaimable must be in front.
-	 */
-	{ "slab_reclaimable", 1, NR_SLAB_RECLAIMABLE_B },
-	{ "slab_unreclaimable", 1, NR_SLAB_UNRECLAIMABLE_B },
+	{ "inactive_anon",		NR_INACTIVE_ANON		},
+	{ "active_anon",		NR_ACTIVE_ANON			},
+	{ "inactive_file",		NR_INACTIVE_FILE		},
+	{ "active_file",		NR_ACTIVE_FILE			},
+	{ "unevictable",		NR_UNEVICTABLE			},
+	{ "slab_reclaimable",		NR_SLAB_RECLAIMABLE_B		},
+	{ "slab_unreclaimable",		NR_SLAB_UNRECLAIMABLE_B		},
 
 	/* The memory events */
-	{ "workingset_refault_anon", 1, WORKINGSET_REFAULT_ANON },
-	{ "workingset_refault_file", 1, WORKINGSET_REFAULT_FILE },
-	{ "workingset_activate_anon", 1, WORKINGSET_ACTIVATE_ANON },
-	{ "workingset_activate_file", 1, WORKINGSET_ACTIVATE_FILE },
-	{ "workingset_restore_anon", 1, WORKINGSET_RESTORE_ANON },
-	{ "workingset_restore_file", 1, WORKINGSET_RESTORE_FILE },
-	{ "workingset_nodereclaim", 1, WORKINGSET_NODERECLAIM },
+	{ "workingset_refault_anon",	WORKINGSET_REFAULT_ANON		},
+	{ "workingset_refault_file",	WORKINGSET_REFAULT_FILE		},
+	{ "workingset_activate_anon",	WORKINGSET_ACTIVATE_ANON	},
+	{ "workingset_activate_file",	WORKINGSET_ACTIVATE_FILE	},
+	{ "workingset_restore_anon",	WORKINGSET_RESTORE_ANON		},
+	{ "workingset_restore_file",	WORKINGSET_RESTORE_FILE		},
+	{ "workingset_nodereclaim",	WORKINGSET_NODERECLAIM		},
 };
 
+/* Translate stat items to the correct unit for memory.stat output */
+static int memcg_page_state_unit(int item)
+{
+	int unit;
+
+	switch (item) {
+	case MEMCG_PERCPU_B:
+	case NR_SLAB_RECLAIMABLE_B:
+	case NR_SLAB_UNRECLAIMABLE_B:
+	case WORKINGSET_REFAULT_ANON:
+	case WORKINGSET_REFAULT_FILE:
+	case WORKINGSET_ACTIVATE_ANON:
+	case WORKINGSET_ACTIVATE_FILE:
+	case WORKINGSET_RESTORE_ANON:
+	case WORKINGSET_RESTORE_FILE:
+	case WORKINGSET_NODERECLAIM:
+		unit = 1;
+		break;
+	case NR_KERNEL_STACK_KB:
+		unit = SZ_1K;
+		break;
+	default:
+		unit = PAGE_SIZE;
+		break;
+	}
+
+	return unit;
+}
+
+static inline unsigned long memcg_page_state_output(struct mem_cgroup *memcg,
+						    int item)
+{
+	return memcg_page_state(memcg, item) * memcg_page_state_unit(item);
+}
+
 static char *memory_stat_format(struct mem_cgroup *memcg)
 {
 	struct seq_buf s;
@@ -1577,13 +1606,12 @@ static char *memory_stat_format(struct mem_cgroup *memcg)
 	for (i = 0; i < ARRAY_SIZE(memory_stats); i++) {
 		u64 size;
 
-		size = memcg_page_state(memcg, memory_stats[i].idx);
-		size *= memory_stats[i].ratio;
+		size = memcg_page_state_output(memcg, memory_stats[i].idx);
 		seq_buf_printf(&s, "%s %llu\n", memory_stats[i].name, size);
 
 		if (unlikely(memory_stats[i].idx == NR_SLAB_UNRECLAIMABLE_B)) {
-			size = memcg_page_state(memcg, NR_SLAB_RECLAIMABLE_B) +
-			       memcg_page_state(memcg, NR_SLAB_UNRECLAIMABLE_B);
+			size += memcg_page_state_output(memcg,
+							NR_SLAB_RECLAIMABLE_B);
 			seq_buf_printf(&s, "slab %llu\n", size);
 		}
 	}
@@ -6377,6 +6405,12 @@ static int memory_stat_show(struct seq_file *m, void *v)
 }
 
 #ifdef CONFIG_NUMA
+static inline unsigned long lruvec_page_state_output(struct lruvec *lruvec,
+						     int item)
+{
+	return lruvec_page_state(lruvec, item) * memcg_page_state_unit(item);
+}
+
 static int memory_numa_stat_show(struct seq_file *m, void *v)
 {
 	int i;
@@ -6394,8 +6428,8 @@ static int memory_numa_stat_show(struct seq_file *m, void *v)
 			struct lruvec *lruvec;
 
 			lruvec = mem_cgroup_lruvec(memcg, NODE_DATA(nid));
-			size = lruvec_page_state(lruvec, memory_stats[i].idx);
-			size *= memory_stats[i].ratio;
+			size = lruvec_page_state_output(lruvec,
+							memory_stats[i].idx);
 			seq_printf(m, " N%d=%llu", nid, size);
 		}
 		seq_putc(m, '\n');
-- 
2.11.0

Re: [PATCH v3 7/7] mm: memcontrol: make the slab calculation consistent

[Pankaj Gupta]

> Although the ratio of the slab is one, we also should read the ratio
> from the related memory_stats instead of hard-coding. And the local
> variable of size is already the value of slab_unreclaimable. So we
> do not need to read again.
>
> We can drop the ratio in struct memory_stat. This can make the code
> clean and simple. And get rid of the awkward mix of static and runtime
> initialization of the memory_stats table.
>
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
>  mm/memcontrol.c | 112 ++++++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 73 insertions(+), 39 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index a40797a27f87..841ea37cc123 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1511,49 +1511,78 @@ static bool mem_cgroup_wait_acct_move(struct mem_cgroup *memcg)
>
>  struct memory_stat {
>         const char *name;
> -       unsigned int ratio;
>         unsigned int idx;
>  };
>
>  static const struct memory_stat memory_stats[] = {
> -       { "anon", PAGE_SIZE, NR_ANON_MAPPED },
> -       { "file", PAGE_SIZE, NR_FILE_PAGES },
> -       { "kernel_stack", 1024, NR_KERNEL_STACK_KB },
> -       { "pagetables", PAGE_SIZE, NR_PAGETABLE },
> -       { "percpu", 1, MEMCG_PERCPU_B },
> -       { "sock", PAGE_SIZE, MEMCG_SOCK },
> -       { "shmem", PAGE_SIZE, NR_SHMEM },
> -       { "file_mapped", PAGE_SIZE, NR_FILE_MAPPED },
> -       { "file_dirty", PAGE_SIZE, NR_FILE_DIRTY },
> -       { "file_writeback", PAGE_SIZE, NR_WRITEBACK },
> +       { "anon",                       NR_ANON_MAPPED                  },
> +       { "file",                       NR_FILE_PAGES                   },
> +       { "kernel_stack",               NR_KERNEL_STACK_KB              },
> +       { "pagetables",                 NR_PAGETABLE                    },
> +       { "percpu",                     MEMCG_PERCPU_B                  },
> +       { "sock",                       MEMCG_SOCK                      },
> +       { "shmem",                      NR_SHMEM                        },
> +       { "file_mapped",                NR_FILE_MAPPED                  },
> +       { "file_dirty",                 NR_FILE_DIRTY                   },
> +       { "file_writeback",             NR_WRITEBACK                    },
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -       { "anon_thp", PAGE_SIZE, NR_ANON_THPS },
> -       { "file_thp", PAGE_SIZE, NR_FILE_THPS },
> -       { "shmem_thp", PAGE_SIZE, NR_SHMEM_THPS },
> +       { "anon_thp",                   NR_ANON_THPS                    },
> +       { "file_thp",                   NR_FILE_THPS                    },
> +       { "shmem_thp",                  NR_SHMEM_THPS                   },
>  #endif
> -       { "inactive_anon", PAGE_SIZE, NR_INACTIVE_ANON },
> -       { "active_anon", PAGE_SIZE, NR_ACTIVE_ANON },
> -       { "inactive_file", PAGE_SIZE, NR_INACTIVE_FILE },
> -       { "active_file", PAGE_SIZE, NR_ACTIVE_FILE },
> -       { "unevictable", PAGE_SIZE, NR_UNEVICTABLE },
> -
> -       /*
> -        * Note: The slab_reclaimable and slab_unreclaimable must be
> -        * together and slab_reclaimable must be in front.
> -        */
> -       { "slab_reclaimable", 1, NR_SLAB_RECLAIMABLE_B },
> -       { "slab_unreclaimable", 1, NR_SLAB_UNRECLAIMABLE_B },
> +       { "inactive_anon",              NR_INACTIVE_ANON                },
> +       { "active_anon",                NR_ACTIVE_ANON                  },
> +       { "inactive_file",              NR_INACTIVE_FILE                },
> +       { "active_file",                NR_ACTIVE_FILE                  },
> +       { "unevictable",                NR_UNEVICTABLE                  },
> +       { "slab_reclaimable",           NR_SLAB_RECLAIMABLE_B           },
> +       { "slab_unreclaimable",         NR_SLAB_UNRECLAIMABLE_B         },
>
>         /* The memory events */
> -       { "workingset_refault_anon", 1, WORKINGSET_REFAULT_ANON },
> -       { "workingset_refault_file", 1, WORKINGSET_REFAULT_FILE },
> -       { "workingset_activate_anon", 1, WORKINGSET_ACTIVATE_ANON },
> -       { "workingset_activate_file", 1, WORKINGSET_ACTIVATE_FILE },
> -       { "workingset_restore_anon", 1, WORKINGSET_RESTORE_ANON },
> -       { "workingset_restore_file", 1, WORKINGSET_RESTORE_FILE },
> -       { "workingset_nodereclaim", 1, WORKINGSET_NODERECLAIM },
> +       { "workingset_refault_anon",    WORKINGSET_REFAULT_ANON         },
> +       { "workingset_refault_file",    WORKINGSET_REFAULT_FILE         },
> +       { "workingset_activate_anon",   WORKINGSET_ACTIVATE_ANON        },
> +       { "workingset_activate_file",   WORKINGSET_ACTIVATE_FILE        },
> +       { "workingset_restore_anon",    WORKINGSET_RESTORE_ANON         },
> +       { "workingset_restore_file",    WORKINGSET_RESTORE_FILE         },
> +       { "workingset_nodereclaim",     WORKINGSET_NODERECLAIM          },
>  };
>
> +/* Translate stat items to the correct unit for memory.stat output */
> +static int memcg_page_state_unit(int item)
> +{
> +       int unit;
> +
> +       switch (item) {
> +       case MEMCG_PERCPU_B:
> +       case NR_SLAB_RECLAIMABLE_B:
> +       case NR_SLAB_UNRECLAIMABLE_B:
> +       case WORKINGSET_REFAULT_ANON:
> +       case WORKINGSET_REFAULT_FILE:
> +       case WORKINGSET_ACTIVATE_ANON:
> +       case WORKINGSET_ACTIVATE_FILE:
> +       case WORKINGSET_RESTORE_ANON:
> +       case WORKINGSET_RESTORE_FILE:
> +       case WORKINGSET_NODERECLAIM:
> +               unit = 1;
> +               break;
> +       case NR_KERNEL_STACK_KB:
> +               unit = SZ_1K;
> +               break;
> +       default:
> +               unit = PAGE_SIZE;
> +               break;
 break not needed here, or maybe we can return for every case,
that will avoid "unit" variable.

> +       }
> +
> +       return unit;
> +}
> +
> +static inline unsigned long memcg_page_state_output(struct mem_cgroup *memcg,
> +                                                   int item)
> +{
> +       return memcg_page_state(memcg, item) * memcg_page_state_unit(item);
> +}
> +
>  static char *memory_stat_format(struct mem_cgroup *memcg)
>  {
>         struct seq_buf s;
> @@ -1577,13 +1606,12 @@ static char *memory_stat_format(struct mem_cgroup *memcg)
>         for (i = 0; i < ARRAY_SIZE(memory_stats); i++) {
>                 u64 size;
>
> -               size = memcg_page_state(memcg, memory_stats[i].idx);
> -               size *= memory_stats[i].ratio;
> +               size = memcg_page_state_output(memcg, memory_stats[i].idx);
>                 seq_buf_printf(&s, "%s %llu\n", memory_stats[i].name, size);
>
>                 if (unlikely(memory_stats[i].idx == NR_SLAB_UNRECLAIMABLE_B)) {
> -                       size = memcg_page_state(memcg, NR_SLAB_RECLAIMABLE_B) +
> -                              memcg_page_state(memcg, NR_SLAB_UNRECLAIMABLE_B);
> +                       size +=
(memcg,
> +                                                       NR_SLAB_RECLAIMABLE_B);
>                         seq_buf_printf(&s, "slab %llu\n", size);
>                 }
>         }
> @@ -6377,6 +6405,12 @@ static int memory_stat_show(struct seq_file *m, void *v)
>  }
>
>  #ifdef CONFIG_NUMA
> +static inline unsigned long lruvec_page_state_output(struct lruvec *lruvec,
> +                                                    int item)
> +{
> +       return lruvec_page_state(lruvec, item) * memcg_page_state_unit(item);
> +}
> +
>  static int memory_numa_stat_show(struct seq_file *m, void *v)
>  {
>         int i;
> @@ -6394,8 +6428,8 @@ static int memory_numa_stat_show(struct seq_file *m, void *v)
>                         struct lruvec *lruvec;
>
>                         lruvec = mem_cgroup_lruvec(memcg, NODE_DATA(nid));
> -                       size = lruvec_page_state(lruvec, memory_stats[i].idx);
> -                       size *= memory_stats[i].ratio;
> +                       size = lruvec_page_state_output(lruvec,
> +                                                       memory_stats[i].idx);
>                         seq_printf(m, " N%d=%llu", nid, size);
>                 }
>                 seq_putc(m, '\n');
> --
> 2.11.0
>
>

Re: [External] Re: [PATCH v3 7/7] mm: memcontrol: make the slab calculation consistent

[Muchun Song]

On Tue, Dec 8, 2020 at 3:21 PM Pankaj Gupta
<pankaj.gupta.linux@gmail.com> wrote:
>
> > Although the ratio of the slab is one, we also should read the ratio
> > from the related memory_stats instead of hard-coding. And the local
> > variable of size is already the value of slab_unreclaimable. So we
> > do not need to read again.
> >
> > We can drop the ratio in struct memory_stat. This can make the code
> > clean and simple. And get rid of the awkward mix of static and runtime
> > initialization of the memory_stats table.
> >
> > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> > ---
> >  mm/memcontrol.c | 112 ++++++++++++++++++++++++++++++++++++--------------------
> >  1 file changed, 73 insertions(+), 39 deletions(-)
> >
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index a40797a27f87..841ea37cc123 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -1511,49 +1511,78 @@ static bool mem_cgroup_wait_acct_move(struct mem_cgroup *memcg)
> >
> >  struct memory_stat {
> >         const char *name;
> > -       unsigned int ratio;
> >         unsigned int idx;
> >  };
> >
> >  static const struct memory_stat memory_stats[] = {
> > -       { "anon", PAGE_SIZE, NR_ANON_MAPPED },
> > -       { "file", PAGE_SIZE, NR_FILE_PAGES },
> > -       { "kernel_stack", 1024, NR_KERNEL_STACK_KB },
> > -       { "pagetables", PAGE_SIZE, NR_PAGETABLE },
> > -       { "percpu", 1, MEMCG_PERCPU_B },
> > -       { "sock", PAGE_SIZE, MEMCG_SOCK },
> > -       { "shmem", PAGE_SIZE, NR_SHMEM },
> > -       { "file_mapped", PAGE_SIZE, NR_FILE_MAPPED },
> > -       { "file_dirty", PAGE_SIZE, NR_FILE_DIRTY },
> > -       { "file_writeback", PAGE_SIZE, NR_WRITEBACK },
> > +       { "anon",                       NR_ANON_MAPPED                  },
> > +       { "file",                       NR_FILE_PAGES                   },
> > +       { "kernel_stack",               NR_KERNEL_STACK_KB              },
> > +       { "pagetables",                 NR_PAGETABLE                    },
> > +       { "percpu",                     MEMCG_PERCPU_B                  },
> > +       { "sock",                       MEMCG_SOCK                      },
> > +       { "shmem",                      NR_SHMEM                        },
> > +       { "file_mapped",                NR_FILE_MAPPED                  },
> > +       { "file_dirty",                 NR_FILE_DIRTY                   },
> > +       { "file_writeback",             NR_WRITEBACK                    },
> >  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> > -       { "anon_thp", PAGE_SIZE, NR_ANON_THPS },
> > -       { "file_thp", PAGE_SIZE, NR_FILE_THPS },
> > -       { "shmem_thp", PAGE_SIZE, NR_SHMEM_THPS },
> > +       { "anon_thp",                   NR_ANON_THPS                    },
> > +       { "file_thp",                   NR_FILE_THPS                    },
> > +       { "shmem_thp",                  NR_SHMEM_THPS                   },
> >  #endif
> > -       { "inactive_anon", PAGE_SIZE, NR_INACTIVE_ANON },
> > -       { "active_anon", PAGE_SIZE, NR_ACTIVE_ANON },
> > -       { "inactive_file", PAGE_SIZE, NR_INACTIVE_FILE },
> > -       { "active_file", PAGE_SIZE, NR_ACTIVE_FILE },
> > -       { "unevictable", PAGE_SIZE, NR_UNEVICTABLE },
> > -
> > -       /*
> > -        * Note: The slab_reclaimable and slab_unreclaimable must be
> > -        * together and slab_reclaimable must be in front.
> > -        */
> > -       { "slab_reclaimable", 1, NR_SLAB_RECLAIMABLE_B },
> > -       { "slab_unreclaimable", 1, NR_SLAB_UNRECLAIMABLE_B },
> > +       { "inactive_anon",              NR_INACTIVE_ANON                },
> > +       { "active_anon",                NR_ACTIVE_ANON                  },
> > +       { "inactive_file",              NR_INACTIVE_FILE                },
> > +       { "active_file",                NR_ACTIVE_FILE                  },
> > +       { "unevictable",                NR_UNEVICTABLE                  },
> > +       { "slab_reclaimable",           NR_SLAB_RECLAIMABLE_B           },
> > +       { "slab_unreclaimable",         NR_SLAB_UNRECLAIMABLE_B         },
> >
> >         /* The memory events */
> > -       { "workingset_refault_anon", 1, WORKINGSET_REFAULT_ANON },
> > -       { "workingset_refault_file", 1, WORKINGSET_REFAULT_FILE },
> > -       { "workingset_activate_anon", 1, WORKINGSET_ACTIVATE_ANON },
> > -       { "workingset_activate_file", 1, WORKINGSET_ACTIVATE_FILE },
> > -       { "workingset_restore_anon", 1, WORKINGSET_RESTORE_ANON },
> > -       { "workingset_restore_file", 1, WORKINGSET_RESTORE_FILE },
> > -       { "workingset_nodereclaim", 1, WORKINGSET_NODERECLAIM },
> > +       { "workingset_refault_anon",    WORKINGSET_REFAULT_ANON         },
> > +       { "workingset_refault_file",    WORKINGSET_REFAULT_FILE         },
> > +       { "workingset_activate_anon",   WORKINGSET_ACTIVATE_ANON        },
> > +       { "workingset_activate_file",   WORKINGSET_ACTIVATE_FILE        },
> > +       { "workingset_restore_anon",    WORKINGSET_RESTORE_ANON         },
> > +       { "workingset_restore_file",    WORKINGSET_RESTORE_FILE         },
> > +       { "workingset_nodereclaim",     WORKINGSET_NODERECLAIM          },
> >  };
> >
> > +/* Translate stat items to the correct unit for memory.stat output */
> > +static int memcg_page_state_unit(int item)
> > +{
> > +       int unit;
> > +
> > +       switch (item) {
> > +       case MEMCG_PERCPU_B:
> > +       case NR_SLAB_RECLAIMABLE_B:
> > +       case NR_SLAB_UNRECLAIMABLE_B:
> > +       case WORKINGSET_REFAULT_ANON:
> > +       case WORKINGSET_REFAULT_FILE:
> > +       case WORKINGSET_ACTIVATE_ANON:
> > +       case WORKINGSET_ACTIVATE_FILE:
> > +       case WORKINGSET_RESTORE_ANON:
> > +       case WORKINGSET_RESTORE_FILE:
> > +       case WORKINGSET_NODERECLAIM:
> > +               unit = 1;
> > +               break;
> > +       case NR_KERNEL_STACK_KB:
> > +               unit = SZ_1K;
> > +               break;
> > +       default:
> > +               unit = PAGE_SIZE;
> > +               break;
>  break not needed here, or maybe we can return for every case,
> that will avoid "unit" variable.

Yeah, thanks.

>
> > +       }
> > +
> > +       return unit;
> > +}
> > +
> > +static inline unsigned long memcg_page_state_output(struct mem_cgroup *memcg,
> > +                                                   int item)
> > +{
> > +       return memcg_page_state(memcg, item) * memcg_page_state_unit(item);
> > +}
> > +
> >  static char *memory_stat_format(struct mem_cgroup *memcg)
> >  {
> >         struct seq_buf s;
> > @@ -1577,13 +1606,12 @@ static char *memory_stat_format(struct mem_cgroup *memcg)
> >         for (i = 0; i < ARRAY_SIZE(memory_stats); i++) {
> >                 u64 size;
> >
> > -               size = memcg_page_state(memcg, memory_stats[i].idx);
> > -               size *= memory_stats[i].ratio;
> > +               size = memcg_page_state_output(memcg, memory_stats[i].idx);
> >                 seq_buf_printf(&s, "%s %llu\n", memory_stats[i].name, size);
> >
> >                 if (unlikely(memory_stats[i].idx == NR_SLAB_UNRECLAIMABLE_B)) {
> > -                       size = memcg_page_state(memcg, NR_SLAB_RECLAIMABLE_B) +
> > -                              memcg_page_state(memcg, NR_SLAB_UNRECLAIMABLE_B);
> > +                       size +=
> (memcg,
> > +                                                       NR_SLAB_RECLAIMABLE_B);
> >                         seq_buf_printf(&s, "slab %llu\n", size);
> >                 }
> >         }
> > @@ -6377,6 +6405,12 @@ static int memory_stat_show(struct seq_file *m, void *v)
> >  }
> >
> >  #ifdef CONFIG_NUMA
> > +static inline unsigned long lruvec_page_state_output(struct lruvec *lruvec,
> > +                                                    int item)
> > +{
> > +       return lruvec_page_state(lruvec, item) * memcg_page_state_unit(item);
> > +}
> > +
> >  static int memory_numa_stat_show(struct seq_file *m, void *v)
> >  {
> >         int i;
> > @@ -6394,8 +6428,8 @@ static int memory_numa_stat_show(struct seq_file *m, void *v)
> >                         struct lruvec *lruvec;
> >
> >                         lruvec = mem_cgroup_lruvec(memcg, NODE_DATA(nid));
> > -                       size = lruvec_page_state(lruvec, memory_stats[i].idx);
> > -                       size *= memory_stats[i].ratio;
> > +                       size = lruvec_page_state_output(lruvec,
> > +                                                       memory_stats[i].idx);
> >                         seq_printf(m, " N%d=%llu", nid, size);
> >                 }
> >                 seq_putc(m, '\n');
> > --
> > 2.11.0
> >
> >



-- 
Yours,
Muchun

Re: [PATCH v3 7/7] mm: memcontrol: make the slab calculation consistent

[Michal Hocko]

On Tue 08-12-20 12:18:47, Muchun Song wrote:
> Although the ratio of the slab is one, we also should read the ratio
> from the related memory_stats instead of hard-coding. And the local
> variable of size is already the value of slab_unreclaimable. So we
> do not need to read again.
>
> We can drop the ratio in struct memory_stat. This can make the code
> clean and simple. And get rid of the awkward mix of static and runtime
> initialization of the memory_stats table.

This changelog doesn't explain, what is the problem, why do we care and
why the additional code is worthwile.

> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
>  mm/memcontrol.c | 112 ++++++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 73 insertions(+), 39 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index a40797a27f87..841ea37cc123 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1511,49 +1511,78 @@ static bool mem_cgroup_wait_acct_move(struct mem_cgroup *memcg)
>  
>  struct memory_stat {
>  	const char *name;
> -	unsigned int ratio;
>  	unsigned int idx;
>  };
>  
>  static const struct memory_stat memory_stats[] = {
> -	{ "anon", PAGE_SIZE, NR_ANON_MAPPED },
> -	{ "file", PAGE_SIZE, NR_FILE_PAGES },
> -	{ "kernel_stack", 1024, NR_KERNEL_STACK_KB },
> -	{ "pagetables", PAGE_SIZE, NR_PAGETABLE },
> -	{ "percpu", 1, MEMCG_PERCPU_B },
> -	{ "sock", PAGE_SIZE, MEMCG_SOCK },
> -	{ "shmem", PAGE_SIZE, NR_SHMEM },
> -	{ "file_mapped", PAGE_SIZE, NR_FILE_MAPPED },
> -	{ "file_dirty", PAGE_SIZE, NR_FILE_DIRTY },
> -	{ "file_writeback", PAGE_SIZE, NR_WRITEBACK },
> +	{ "anon",			NR_ANON_MAPPED			},
> +	{ "file",			NR_FILE_PAGES			},
> +	{ "kernel_stack",		NR_KERNEL_STACK_KB		},
> +	{ "pagetables",			NR_PAGETABLE			},
> +	{ "percpu",			MEMCG_PERCPU_B			},
> +	{ "sock",			MEMCG_SOCK			},
> +	{ "shmem",			NR_SHMEM			},
> +	{ "file_mapped",		NR_FILE_MAPPED			},
> +	{ "file_dirty",			NR_FILE_DIRTY			},
> +	{ "file_writeback",		NR_WRITEBACK			},
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -	{ "anon_thp", PAGE_SIZE, NR_ANON_THPS },
> -	{ "file_thp", PAGE_SIZE, NR_FILE_THPS },
> -	{ "shmem_thp", PAGE_SIZE, NR_SHMEM_THPS },
> +	{ "anon_thp",			NR_ANON_THPS			},
> +	{ "file_thp",			NR_FILE_THPS			},
> +	{ "shmem_thp",			NR_SHMEM_THPS			},
>  #endif
> -	{ "inactive_anon", PAGE_SIZE, NR_INACTIVE_ANON },
> -	{ "active_anon", PAGE_SIZE, NR_ACTIVE_ANON },
> -	{ "inactive_file", PAGE_SIZE, NR_INACTIVE_FILE },
> -	{ "active_file", PAGE_SIZE, NR_ACTIVE_FILE },
> -	{ "unevictable", PAGE_SIZE, NR_UNEVICTABLE },
> -
> -	/*
> -	 * Note: The slab_reclaimable and slab_unreclaimable must be
> -	 * together and slab_reclaimable must be in front.
> -	 */
> -	{ "slab_reclaimable", 1, NR_SLAB_RECLAIMABLE_B },
> -	{ "slab_unreclaimable", 1, NR_SLAB_UNRECLAIMABLE_B },
> +	{ "inactive_anon",		NR_INACTIVE_ANON		},
> +	{ "active_anon",		NR_ACTIVE_ANON			},
> +	{ "inactive_file",		NR_INACTIVE_FILE		},
> +	{ "active_file",		NR_ACTIVE_FILE			},
> +	{ "unevictable",		NR_UNEVICTABLE			},
> +	{ "slab_reclaimable",		NR_SLAB_RECLAIMABLE_B		},
> +	{ "slab_unreclaimable",		NR_SLAB_UNRECLAIMABLE_B		},
>  
>  	/* The memory events */
> -	{ "workingset_refault_anon", 1, WORKINGSET_REFAULT_ANON },
> -	{ "workingset_refault_file", 1, WORKINGSET_REFAULT_FILE },
> -	{ "workingset_activate_anon", 1, WORKINGSET_ACTIVATE_ANON },
> -	{ "workingset_activate_file", 1, WORKINGSET_ACTIVATE_FILE },
> -	{ "workingset_restore_anon", 1, WORKINGSET_RESTORE_ANON },
> -	{ "workingset_restore_file", 1, WORKINGSET_RESTORE_FILE },
> -	{ "workingset_nodereclaim", 1, WORKINGSET_NODERECLAIM },
> +	{ "workingset_refault_anon",	WORKINGSET_REFAULT_ANON		},
> +	{ "workingset_refault_file",	WORKINGSET_REFAULT_FILE		},
> +	{ "workingset_activate_anon",	WORKINGSET_ACTIVATE_ANON	},
> +	{ "workingset_activate_file",	WORKINGSET_ACTIVATE_FILE	},
> +	{ "workingset_restore_anon",	WORKINGSET_RESTORE_ANON		},
> +	{ "workingset_restore_file",	WORKINGSET_RESTORE_FILE		},
> +	{ "workingset_nodereclaim",	WORKINGSET_NODERECLAIM		},
>  };
>  
> +/* Translate stat items to the correct unit for memory.stat output */
> +static int memcg_page_state_unit(int item)
> +{
> +	int unit;
> +
> +	switch (item) {
> +	case MEMCG_PERCPU_B:
> +	case NR_SLAB_RECLAIMABLE_B:
> +	case NR_SLAB_UNRECLAIMABLE_B:
> +	case WORKINGSET_REFAULT_ANON:
> +	case WORKINGSET_REFAULT_FILE:
> +	case WORKINGSET_ACTIVATE_ANON:
> +	case WORKINGSET_ACTIVATE_FILE:
> +	case WORKINGSET_RESTORE_ANON:
> +	case WORKINGSET_RESTORE_FILE:
> +	case WORKINGSET_NODERECLAIM:
> +		unit = 1;
> +		break;
> +	case NR_KERNEL_STACK_KB:
> +		unit = SZ_1K;
> +		break;
> +	default:
> +		unit = PAGE_SIZE;
> +		break;
> +	}
> +
> +	return unit;
> +}
> +
> +static inline unsigned long memcg_page_state_output(struct mem_cgroup *memcg,
> +						    int item)
> +{
> +	return memcg_page_state(memcg, item) * memcg_page_state_unit(item);
> +}
> +
>  static char *memory_stat_format(struct mem_cgroup *memcg)
>  {
>  	struct seq_buf s;
> @@ -1577,13 +1606,12 @@ static char *memory_stat_format(struct mem_cgroup *memcg)
>  	for (i = 0; i < ARRAY_SIZE(memory_stats); i++) {
>  		u64 size;
>  
> -		size = memcg_page_state(memcg, memory_stats[i].idx);
> -		size *= memory_stats[i].ratio;
> +		size = memcg_page_state_output(memcg, memory_stats[i].idx);
>  		seq_buf_printf(&s, "%s %llu\n", memory_stats[i].name, size);
>  
>  		if (unlikely(memory_stats[i].idx == NR_SLAB_UNRECLAIMABLE_B)) {
> -			size = memcg_page_state(memcg, NR_SLAB_RECLAIMABLE_B) +
> -			       memcg_page_state(memcg, NR_SLAB_UNRECLAIMABLE_B);
> +			size += memcg_page_state_output(memcg,
> +							NR_SLAB_RECLAIMABLE_B);
>  			seq_buf_printf(&s, "slab %llu\n", size);
>  		}
>  	}
> @@ -6377,6 +6405,12 @@ static int memory_stat_show(struct seq_file *m, void *v)
>  }
>  
>  #ifdef CONFIG_NUMA
> +static inline unsigned long lruvec_page_state_output(struct lruvec *lruvec,
> +						     int item)
> +{
> +	return lruvec_page_state(lruvec, item) * memcg_page_state_unit(item);
> +}
> +
>  static int memory_numa_stat_show(struct seq_file *m, void *v)
>  {
>  	int i;
> @@ -6394,8 +6428,8 @@ static int memory_numa_stat_show(struct seq_file *m, void *v)
>  			struct lruvec *lruvec;
>  
>  			lruvec = mem_cgroup_lruvec(memcg, NODE_DATA(nid));
> -			size = lruvec_page_state(lruvec, memory_stats[i].idx);
> -			size *= memory_stats[i].ratio;
> +			size = lruvec_page_state_output(lruvec,
> +							memory_stats[i].idx);
>  			seq_printf(m, " N%d=%llu", nid, size);
>  		}
>  		seq_putc(m, '\n');
> -- 
> 2.11.0

-- 
Michal Hocko
SUSE Labs

Re: [External] Re: [PATCH v3 7/7] mm: memcontrol: make the slab calculation consistent

[Muchun Song]

On Tue, Dec 15, 2020 at 9:35 PM Michal Hocko <mhocko@suse.com> wrote:
>
> On Tue 08-12-20 12:18:47, Muchun Song wrote:
> > Although the ratio of the slab is one, we also should read the ratio
> > from the related memory_stats instead of hard-coding. And the local
> > variable of size is already the value of slab_unreclaimable. So we
> > do not need to read again.
> >
> > We can drop the ratio in struct memory_stat. This can make the code
> > clean and simple. And get rid of the awkward mix of static and runtime
> > initialization of the memory_stats table.
>
> This changelog doesn't explain, what is the problem, why do we care and
> why the additional code is worthwile.

Thank you. Will update the commit log for more clear.

>
> > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> > ---
> >  mm/memcontrol.c | 112 ++++++++++++++++++++++++++++++++++++--------------------
> >  1 file changed, 73 insertions(+), 39 deletions(-)
> >
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index a40797a27f87..841ea37cc123 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -1511,49 +1511,78 @@ static bool mem_cgroup_wait_acct_move(struct mem_cgroup *memcg)
> >
> >  struct memory_stat {
> >       const char *name;
> > -     unsigned int ratio;
> >       unsigned int idx;
> >  };
> >
> >  static const struct memory_stat memory_stats[] = {
> > -     { "anon", PAGE_SIZE, NR_ANON_MAPPED },
> > -     { "file", PAGE_SIZE, NR_FILE_PAGES },
> > -     { "kernel_stack", 1024, NR_KERNEL_STACK_KB },
> > -     { "pagetables", PAGE_SIZE, NR_PAGETABLE },
> > -     { "percpu", 1, MEMCG_PERCPU_B },
> > -     { "sock", PAGE_SIZE, MEMCG_SOCK },
> > -     { "shmem", PAGE_SIZE, NR_SHMEM },
> > -     { "file_mapped", PAGE_SIZE, NR_FILE_MAPPED },
> > -     { "file_dirty", PAGE_SIZE, NR_FILE_DIRTY },
> > -     { "file_writeback", PAGE_SIZE, NR_WRITEBACK },
> > +     { "anon",                       NR_ANON_MAPPED                  },
> > +     { "file",                       NR_FILE_PAGES                   },
> > +     { "kernel_stack",               NR_KERNEL_STACK_KB              },
> > +     { "pagetables",                 NR_PAGETABLE                    },
> > +     { "percpu",                     MEMCG_PERCPU_B                  },
> > +     { "sock",                       MEMCG_SOCK                      },
> > +     { "shmem",                      NR_SHMEM                        },
> > +     { "file_mapped",                NR_FILE_MAPPED                  },
> > +     { "file_dirty",                 NR_FILE_DIRTY                   },
> > +     { "file_writeback",             NR_WRITEBACK                    },
> >  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> > -     { "anon_thp", PAGE_SIZE, NR_ANON_THPS },
> > -     { "file_thp", PAGE_SIZE, NR_FILE_THPS },
> > -     { "shmem_thp", PAGE_SIZE, NR_SHMEM_THPS },
> > +     { "anon_thp",                   NR_ANON_THPS                    },
> > +     { "file_thp",                   NR_FILE_THPS                    },
> > +     { "shmem_thp",                  NR_SHMEM_THPS                   },
> >  #endif
> > -     { "inactive_anon", PAGE_SIZE, NR_INACTIVE_ANON },
> > -     { "active_anon", PAGE_SIZE, NR_ACTIVE_ANON },
> > -     { "inactive_file", PAGE_SIZE, NR_INACTIVE_FILE },
> > -     { "active_file", PAGE_SIZE, NR_ACTIVE_FILE },
> > -     { "unevictable", PAGE_SIZE, NR_UNEVICTABLE },
> > -
> > -     /*
> > -      * Note: The slab_reclaimable and slab_unreclaimable must be
> > -      * together and slab_reclaimable must be in front.
> > -      */
> > -     { "slab_reclaimable", 1, NR_SLAB_RECLAIMABLE_B },
> > -     { "slab_unreclaimable", 1, NR_SLAB_UNRECLAIMABLE_B },
> > +     { "inactive_anon",              NR_INACTIVE_ANON                },
> > +     { "active_anon",                NR_ACTIVE_ANON                  },
> > +     { "inactive_file",              NR_INACTIVE_FILE                },
> > +     { "active_file",                NR_ACTIVE_FILE                  },
> > +     { "unevictable",                NR_UNEVICTABLE                  },
> > +     { "slab_reclaimable",           NR_SLAB_RECLAIMABLE_B           },
> > +     { "slab_unreclaimable",         NR_SLAB_UNRECLAIMABLE_B         },
> >
> >       /* The memory events */
> > -     { "workingset_refault_anon", 1, WORKINGSET_REFAULT_ANON },
> > -     { "workingset_refault_file", 1, WORKINGSET_REFAULT_FILE },
> > -     { "workingset_activate_anon", 1, WORKINGSET_ACTIVATE_ANON },
> > -     { "workingset_activate_file", 1, WORKINGSET_ACTIVATE_FILE },
> > -     { "workingset_restore_anon", 1, WORKINGSET_RESTORE_ANON },
> > -     { "workingset_restore_file", 1, WORKINGSET_RESTORE_FILE },
> > -     { "workingset_nodereclaim", 1, WORKINGSET_NODERECLAIM },
> > +     { "workingset_refault_anon",    WORKINGSET_REFAULT_ANON         },
> > +     { "workingset_refault_file",    WORKINGSET_REFAULT_FILE         },
> > +     { "workingset_activate_anon",   WORKINGSET_ACTIVATE_ANON        },
> > +     { "workingset_activate_file",   WORKINGSET_ACTIVATE_FILE        },
> > +     { "workingset_restore_anon",    WORKINGSET_RESTORE_ANON         },
> > +     { "workingset_restore_file",    WORKINGSET_RESTORE_FILE         },
> > +     { "workingset_nodereclaim",     WORKINGSET_NODERECLAIM          },
> >  };
> >
> > +/* Translate stat items to the correct unit for memory.stat output */
> > +static int memcg_page_state_unit(int item)
> > +{
> > +     int unit;
> > +
> > +     switch (item) {
> > +     case MEMCG_PERCPU_B:
> > +     case NR_SLAB_RECLAIMABLE_B:
> > +     case NR_SLAB_UNRECLAIMABLE_B:
> > +     case WORKINGSET_REFAULT_ANON:
> > +     case WORKINGSET_REFAULT_FILE:
> > +     case WORKINGSET_ACTIVATE_ANON:
> > +     case WORKINGSET_ACTIVATE_FILE:
> > +     case WORKINGSET_RESTORE_ANON:
> > +     case WORKINGSET_RESTORE_FILE:
> > +     case WORKINGSET_NODERECLAIM:
> > +             unit = 1;
> > +             break;
> > +     case NR_KERNEL_STACK_KB:
> > +             unit = SZ_1K;
> > +             break;
> > +     default:
> > +             unit = PAGE_SIZE;
> > +             break;
> > +     }
> > +
> > +     return unit;
> > +}
> > +
> > +static inline unsigned long memcg_page_state_output(struct mem_cgroup *memcg,
> > +                                                 int item)
> > +{
> > +     return memcg_page_state(memcg, item) * memcg_page_state_unit(item);
> > +}
> > +
> >  static char *memory_stat_format(struct mem_cgroup *memcg)
> >  {
> >       struct seq_buf s;
> > @@ -1577,13 +1606,12 @@ static char *memory_stat_format(struct mem_cgroup *memcg)
> >       for (i = 0; i < ARRAY_SIZE(memory_stats); i++) {
> >               u64 size;
> >
> > -             size = memcg_page_state(memcg, memory_stats[i].idx);
> > -             size *= memory_stats[i].ratio;
> > +             size = memcg_page_state_output(memcg, memory_stats[i].idx);
> >               seq_buf_printf(&s, "%s %llu\n", memory_stats[i].name, size);
> >
> >               if (unlikely(memory_stats[i].idx == NR_SLAB_UNRECLAIMABLE_B)) {
> > -                     size = memcg_page_state(memcg, NR_SLAB_RECLAIMABLE_B) +
> > -                            memcg_page_state(memcg, NR_SLAB_UNRECLAIMABLE_B);
> > +                     size += memcg_page_state_output(memcg,
> > +                                                     NR_SLAB_RECLAIMABLE_B);
> >                       seq_buf_printf(&s, "slab %llu\n", size);
> >               }
> >       }
> > @@ -6377,6 +6405,12 @@ static int memory_stat_show(struct seq_file *m, void *v)
> >  }
> >
> >  #ifdef CONFIG_NUMA
> > +static inline unsigned long lruvec_page_state_output(struct lruvec *lruvec,
> > +                                                  int item)
> > +{
> > +     return lruvec_page_state(lruvec, item) * memcg_page_state_unit(item);
> > +}
> > +
> >  static int memory_numa_stat_show(struct seq_file *m, void *v)
> >  {
> >       int i;
> > @@ -6394,8 +6428,8 @@ static int memory_numa_stat_show(struct seq_file *m, void *v)
> >                       struct lruvec *lruvec;
> >
> >                       lruvec = mem_cgroup_lruvec(memcg, NODE_DATA(nid));
> > -                     size = lruvec_page_state(lruvec, memory_stats[i].idx);
> > -                     size *= memory_stats[i].ratio;
> > +                     size = lruvec_page_state_output(lruvec,
> > +                                                     memory_stats[i].idx);
> >                       seq_printf(m, " N%d=%llu", nid, size);
> >               }
> >               seq_putc(m, '\n');
> > --
> > 2.11.0
>
> --
> Michal Hocko
> SUSE Labs



-- 
Yours,
Muchun