diff mbox series

[v3] mm: memcontrol: add {pgscan,pgsteal}_{kswapd,direct} items in memory.stat of cgroup v2

Message ID 20220606154028.55030-1-zhengqi.arch@bytedance.com (mailing list archive)
State New
Headers show
Series [v3] mm: memcontrol: add {pgscan,pgsteal}_{kswapd,direct} items in memory.stat of cgroup v2 | expand

Commit Message

Qi Zheng June 6, 2022, 3:40 p.m. UTC
There are already statistics of {pgscan,pgsteal}_kswapd and
{pgscan,pgsteal}_direct of memcg event here, but now only the
sum of the two is displayed in memory.stat of cgroup v2.

In order to obtain more accurate information during monitoring
and debugging, and to align with the display in /proc/vmstat,
it better to display {pgscan,pgsteal}_kswapd and
{pgscan,pgsteal}_direct separately.

Also, for forward compatibility, we still display pgscan and
pgsteal items so that it won't break existing applications.

Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Acked-by: Roman Gushchin <roman.gushchin@linux.dev>
Acked-by: Muchun Song <songmuchun@bytedance.com>
Acked-by: Shakeel Butt <shakeelb@google.com>
Acked-by: Michal Hocko <mhocko@suse.com>
---
Changelog in v2 -> v3:
 - add comment for memcg_vm_event_stat (suggested by Michal)
 - collect Acked-bys

Changelog in v1 -> v2:
 - keep pgscan and pgsteal items for forward compatibility, thanks to Shakeel
 - update commit log
 - collect Acked-bys

 Documentation/admin-guide/cgroup-v2.rst | 12 ++++++
 mm/memcontrol.c                         | 55 ++++++++++++-------------
 2 files changed, 39 insertions(+), 28 deletions(-)

Comments

Johannes Weiner June 6, 2022, 5:34 p.m. UTC | #1
On Mon, Jun 06, 2022 at 11:40:28PM +0800, Qi Zheng wrote:
> There are already statistics of {pgscan,pgsteal}_kswapd and
> {pgscan,pgsteal}_direct of memcg event here, but now only the
> sum of the two is displayed in memory.stat of cgroup v2.
> 
> In order to obtain more accurate information during monitoring
> and debugging, and to align with the display in /proc/vmstat,
> it better to display {pgscan,pgsteal}_kswapd and
> {pgscan,pgsteal}_direct separately.
> 
> Also, for forward compatibility, we still display pgscan and
> pgsteal items so that it won't break existing applications.
> 
> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> Acked-by: Roman Gushchin <roman.gushchin@linux.dev>
> Acked-by: Muchun Song <songmuchun@bytedance.com>
> Acked-by: Shakeel Butt <shakeelb@google.com>
> Acked-by: Michal Hocko <mhocko@suse.com>

No objection to keeping pgscan and pgsteal, but can you please fix the
doc to present the items in the same order as memory.stat has them?

> @@ -1445,9 +1445,21 @@ PAGE_SIZE multiple when read back.
>  	  pgscan (npn)
>  		Amount of scanned pages (in an inactive LRU list)
>  
> +	  pgscan_kswapd (npn)
> +		Amount of scanned pages by kswapd (in an inactive LRU list)
> +
> +	  pgscan_direct (npn)
> +		Amount of scanned pages directly  (in an inactive LRU list)
> +
>  	  pgsteal (npn)
>  		Amount of reclaimed pages
>  
> +	  pgsteal_kswapd (npn)
> +		Amount of reclaimed pages by kswapd
> +
> +	  pgsteal_direct (npn)
> +		Amount of reclaimed pages directly
> +
>  	  pgactivate (npn)
>  		Amount of pages moved to the active LRU list

vs:

> @@ -1495,41 +1518,17 @@ static char *memory_stat_format(struct mem_cgroup *memcg)
>  	}
>  
>  	/* Accumulated memory events */
> -
> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGFAULT),
> -		       memcg_events(memcg, PGFAULT));
> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGMAJFAULT),
> -		       memcg_events(memcg, PGMAJFAULT));
> -	seq_buf_printf(&s, "%s %lu\n",  vm_event_name(PGREFILL),
> -		       memcg_events(memcg, PGREFILL));
>  	seq_buf_printf(&s, "pgscan %lu\n",
>  		       memcg_events(memcg, PGSCAN_KSWAPD) +
>  		       memcg_events(memcg, PGSCAN_DIRECT));
>  	seq_buf_printf(&s, "pgsteal %lu\n",
>  		       memcg_events(memcg, PGSTEAL_KSWAPD) +
>  		       memcg_events(memcg, PGSTEAL_DIRECT));
> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGACTIVATE),
> -		       memcg_events(memcg, PGACTIVATE));
> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGDEACTIVATE),
> -		       memcg_events(memcg, PGDEACTIVATE));
> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGLAZYFREE),
> -		       memcg_events(memcg, PGLAZYFREE));
> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGLAZYFREED),
> -		       memcg_events(memcg, PGLAZYFREED));
> -
> -#if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_ZSWAP)
> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(ZSWPIN),
> -		       memcg_events(memcg, ZSWPIN));
> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(ZSWPOUT),
> -		       memcg_events(memcg, ZSWPOUT));
> -#endif
>  
> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(THP_FAULT_ALLOC),
> -		       memcg_events(memcg, THP_FAULT_ALLOC));
> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(THP_COLLAPSE_ALLOC),
> -		       memcg_events(memcg, THP_COLLAPSE_ALLOC));
> -#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
> +	for (i = 0; i < ARRAY_SIZE(memcg_vm_event_stat); i++)
> +		seq_buf_printf(&s, "%s %lu\n",
> +			       vm_event_name(memcg_vm_event_stat[i]),
> +			       memcg_events(memcg, memcg_vm_event_stat[i]));

Thanks
Qi Zheng June 7, 2022, 2:18 a.m. UTC | #2
On 2022/6/7 1:34 AM, Johannes Weiner wrote:
> On Mon, Jun 06, 2022 at 11:40:28PM +0800, Qi Zheng wrote:
>> There are already statistics of {pgscan,pgsteal}_kswapd and
>> {pgscan,pgsteal}_direct of memcg event here, but now only the
>> sum of the two is displayed in memory.stat of cgroup v2.
>>
>> In order to obtain more accurate information during monitoring
>> and debugging, and to align with the display in /proc/vmstat,
>> it better to display {pgscan,pgsteal}_kswapd and
>> {pgscan,pgsteal}_direct separately.
>>
>> Also, for forward compatibility, we still display pgscan and
>> pgsteal items so that it won't break existing applications.
>>
>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
>> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
>> Acked-by: Roman Gushchin <roman.gushchin@linux.dev>
>> Acked-by: Muchun Song <songmuchun@bytedance.com>
>> Acked-by: Shakeel Butt <shakeelb@google.com>
>> Acked-by: Michal Hocko <mhocko@suse.com>
> 
> No objection to keeping pgscan and pgsteal, but can you please fix the
> doc to present the items in the same order as memory.stat has them?

Sure, will fix.

Thanks,
Qi

> 
>> @@ -1445,9 +1445,21 @@ PAGE_SIZE multiple when read back.
>>   	  pgscan (npn)
>>   		Amount of scanned pages (in an inactive LRU list)
>>   
>> +	  pgscan_kswapd (npn)
>> +		Amount of scanned pages by kswapd (in an inactive LRU list)
>> +
>> +	  pgscan_direct (npn)
>> +		Amount of scanned pages directly  (in an inactive LRU list)
>> +
>>   	  pgsteal (npn)
>>   		Amount of reclaimed pages
>>   
>> +	  pgsteal_kswapd (npn)
>> +		Amount of reclaimed pages by kswapd
>> +
>> +	  pgsteal_direct (npn)
>> +		Amount of reclaimed pages directly
>> +
>>   	  pgactivate (npn)
>>   		Amount of pages moved to the active LRU list
> 
> vs:
> 
>> @@ -1495,41 +1518,17 @@ static char *memory_stat_format(struct mem_cgroup *memcg)
>>   	}
>>   
>>   	/* Accumulated memory events */
>> -
>> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGFAULT),
>> -		       memcg_events(memcg, PGFAULT));
>> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGMAJFAULT),
>> -		       memcg_events(memcg, PGMAJFAULT));
>> -	seq_buf_printf(&s, "%s %lu\n",  vm_event_name(PGREFILL),
>> -		       memcg_events(memcg, PGREFILL));
>>   	seq_buf_printf(&s, "pgscan %lu\n",
>>   		       memcg_events(memcg, PGSCAN_KSWAPD) +
>>   		       memcg_events(memcg, PGSCAN_DIRECT));
>>   	seq_buf_printf(&s, "pgsteal %lu\n",
>>   		       memcg_events(memcg, PGSTEAL_KSWAPD) +
>>   		       memcg_events(memcg, PGSTEAL_DIRECT));
>> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGACTIVATE),
>> -		       memcg_events(memcg, PGACTIVATE));
>> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGDEACTIVATE),
>> -		       memcg_events(memcg, PGDEACTIVATE));
>> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGLAZYFREE),
>> -		       memcg_events(memcg, PGLAZYFREE));
>> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGLAZYFREED),
>> -		       memcg_events(memcg, PGLAZYFREED));
>> -
>> -#if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_ZSWAP)
>> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(ZSWPIN),
>> -		       memcg_events(memcg, ZSWPIN));
>> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(ZSWPOUT),
>> -		       memcg_events(memcg, ZSWPOUT));
>> -#endif
>>   
>> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(THP_FAULT_ALLOC),
>> -		       memcg_events(memcg, THP_FAULT_ALLOC));
>> -	seq_buf_printf(&s, "%s %lu\n", vm_event_name(THP_COLLAPSE_ALLOC),
>> -		       memcg_events(memcg, THP_COLLAPSE_ALLOC));
>> -#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>> +	for (i = 0; i < ARRAY_SIZE(memcg_vm_event_stat); i++)
>> +		seq_buf_printf(&s, "%s %lu\n",
>> +			       vm_event_name(memcg_vm_event_stat[i]),
>> +			       memcg_events(memcg, memcg_vm_event_stat[i]));
> 
> Thanks
diff mbox series

Patch

diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index 176298f2f4de..b2b55e7360d8 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -1445,9 +1445,21 @@  PAGE_SIZE multiple when read back.
 	  pgscan (npn)
 		Amount of scanned pages (in an inactive LRU list)
 
+	  pgscan_kswapd (npn)
+		Amount of scanned pages by kswapd (in an inactive LRU list)
+
+	  pgscan_direct (npn)
+		Amount of scanned pages directly  (in an inactive LRU list)
+
 	  pgsteal (npn)
 		Amount of reclaimed pages
 
+	  pgsteal_kswapd (npn)
+		Amount of reclaimed pages by kswapd
+
+	  pgsteal_direct (npn)
+		Amount of reclaimed pages directly
+
 	  pgactivate (npn)
 		Amount of pages moved to the active LRU list
 
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 0d3fe0a0c75a..27cebaa53472 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1460,6 +1460,29 @@  static inline unsigned long memcg_page_state_output(struct mem_cgroup *memcg,
 	return memcg_page_state(memcg, item) * memcg_page_state_unit(item);
 }
 
+/* Subset of vm_event_item to report for memcg event stats */
+static const unsigned int memcg_vm_event_stat[] = {
+	PGSCAN_KSWAPD,
+	PGSCAN_DIRECT,
+	PGSTEAL_KSWAPD,
+	PGSTEAL_DIRECT,
+	PGFAULT,
+	PGMAJFAULT,
+	PGREFILL,
+	PGACTIVATE,
+	PGDEACTIVATE,
+	PGLAZYFREE,
+	PGLAZYFREED,
+#if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_ZSWAP)
+	ZSWPIN,
+	ZSWPOUT,
+#endif
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+	THP_FAULT_ALLOC,
+	THP_COLLAPSE_ALLOC,
+#endif
+};
+
 static char *memory_stat_format(struct mem_cgroup *memcg)
 {
 	struct seq_buf s;
@@ -1495,41 +1518,17 @@  static char *memory_stat_format(struct mem_cgroup *memcg)
 	}
 
 	/* Accumulated memory events */
-
-	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGFAULT),
-		       memcg_events(memcg, PGFAULT));
-	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGMAJFAULT),
-		       memcg_events(memcg, PGMAJFAULT));
-	seq_buf_printf(&s, "%s %lu\n",  vm_event_name(PGREFILL),
-		       memcg_events(memcg, PGREFILL));
 	seq_buf_printf(&s, "pgscan %lu\n",
 		       memcg_events(memcg, PGSCAN_KSWAPD) +
 		       memcg_events(memcg, PGSCAN_DIRECT));
 	seq_buf_printf(&s, "pgsteal %lu\n",
 		       memcg_events(memcg, PGSTEAL_KSWAPD) +
 		       memcg_events(memcg, PGSTEAL_DIRECT));
-	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGACTIVATE),
-		       memcg_events(memcg, PGACTIVATE));
-	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGDEACTIVATE),
-		       memcg_events(memcg, PGDEACTIVATE));
-	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGLAZYFREE),
-		       memcg_events(memcg, PGLAZYFREE));
-	seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGLAZYFREED),
-		       memcg_events(memcg, PGLAZYFREED));
-
-#if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_ZSWAP)
-	seq_buf_printf(&s, "%s %lu\n", vm_event_name(ZSWPIN),
-		       memcg_events(memcg, ZSWPIN));
-	seq_buf_printf(&s, "%s %lu\n", vm_event_name(ZSWPOUT),
-		       memcg_events(memcg, ZSWPOUT));
-#endif
 
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
-	seq_buf_printf(&s, "%s %lu\n", vm_event_name(THP_FAULT_ALLOC),
-		       memcg_events(memcg, THP_FAULT_ALLOC));
-	seq_buf_printf(&s, "%s %lu\n", vm_event_name(THP_COLLAPSE_ALLOC),
-		       memcg_events(memcg, THP_COLLAPSE_ALLOC));
-#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
+	for (i = 0; i < ARRAY_SIZE(memcg_vm_event_stat); i++)
+		seq_buf_printf(&s, "%s %lu\n",
+			       vm_event_name(memcg_vm_event_stat[i]),
+			       memcg_events(memcg, memcg_vm_event_stat[i]));
 
 	/* The above should easily fit into one page */
 	WARN_ON_ONCE(seq_buf_has_overflowed(&s));