From ff841a06c844b0556b434d67cfc43f4fda56ae7b Mon Sep 17 00:00:00 2001 From: Yosry Ahmed Date: Fri, 22 Sep 2023 17:57:39 +0000 Subject: mm: memcg: refactor page state unit helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patch series "mm: memcg: fix tracking of pending stats updates values", v2. While working on adjacent code [1], I realized that the values passed into memcg_rstat_updated() to keep track of the magnitude of pending updates is consistent. It is mostly in pages, but sometimes it can be in bytes or KBs. Fix that. Patch 1 reworks memcg_page_state_unit() so that we can reuse it in patch 2 to check and normalize the units of state updates. [1]https://lore.kernel.org/lkml/20230921081057.3440885-1-yosryahmed@google.com/ This patch (of 2): memcg_page_state_unit() is currently used to identify the unit of a memcg state item so that all stats in memory.stat are in bytes. However, it lies about the units of WORKINGSET_* stats. These stats actually represent pages, but we present them to userspace as a scalar number of events. In retrospect, maybe those stats should have been memcg "events" rather than memcg "state". In preparation for using memcg_page_state_unit() for other purposes that need to know the truthful units of different stat items, break it down into two helpers: - memcg_page_state_unit() retuns the actual unit of the item. - memcg_page_state_output_unit() returns the unit used for output. Use the latter instead of the former in memcg_page_state_output() and lruvec_page_state_output(). While we are at it, let's show cgroup v1 some love and add memcg_page_state_local_output() for consistency. No functional change intended. Link: https://lkml.kernel.org/r/20230922175741.635002-1-yosryahmed@google.com Link: https://lkml.kernel.org/r/20230922175741.635002-2-yosryahmed@google.com Signed-off-by: Yosry Ahmed Acked-by: Johannes Weiner Cc: Michal Hocko Cc: Michal Koutný Cc: Muchun Song Cc: Roman Gushchin Cc: Shakeel Butt Signed-off-by: Andrew Morton --- mm/memcontrol.c | 44 +++++++++++++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 11 deletions(-) (limited to 'mm') diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 8713bc796c77..7e65d8c2e685 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -1535,7 +1535,7 @@ static const struct memory_stat memory_stats[] = { { "workingset_nodereclaim", WORKINGSET_NODERECLAIM }, }; -/* Translate stat items to the correct unit for memory.stat output */ +/* The actual unit of the state item, not the same as the output unit */ static int memcg_page_state_unit(int item) { switch (item) { @@ -1543,6 +1543,22 @@ static int memcg_page_state_unit(int item) case MEMCG_ZSWAP_B: case NR_SLAB_RECLAIMABLE_B: case NR_SLAB_UNRECLAIMABLE_B: + return 1; + case NR_KERNEL_STACK_KB: + return SZ_1K; + default: + return PAGE_SIZE; + } +} + +/* Translate stat items to the correct unit for memory.stat output */ +static int memcg_page_state_output_unit(int item) +{ + /* + * Workingset state is actually in pages, but we export it to userspace + * as a scalar count of events, so special case it here. + */ + switch (item) { case WORKINGSET_REFAULT_ANON: case WORKINGSET_REFAULT_FILE: case WORKINGSET_ACTIVATE_ANON: @@ -1551,17 +1567,23 @@ static int memcg_page_state_unit(int item) case WORKINGSET_RESTORE_FILE: case WORKINGSET_NODERECLAIM: return 1; - case NR_KERNEL_STACK_KB: - return SZ_1K; default: - return PAGE_SIZE; + return memcg_page_state_unit(item); } } 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); + return memcg_page_state(memcg, item) * + memcg_page_state_output_unit(item); +} + +static inline unsigned long memcg_page_state_local_output( + struct mem_cgroup *memcg, int item) +{ + return memcg_page_state_local(memcg, item) * + memcg_page_state_output_unit(item); } static void memcg_stat_format(struct mem_cgroup *memcg, struct seq_buf *s) @@ -4113,9 +4135,8 @@ static void memcg1_stat_format(struct mem_cgroup *memcg, struct seq_buf *s) for (i = 0; i < ARRAY_SIZE(memcg1_stats); i++) { unsigned long nr; - nr = memcg_page_state_local(memcg, memcg1_stats[i]); - seq_buf_printf(s, "%s %lu\n", memcg1_stat_names[i], - nr * memcg_page_state_unit(memcg1_stats[i])); + nr = memcg_page_state_local_output(memcg, memcg1_stats[i]); + seq_buf_printf(s, "%s %lu\n", memcg1_stat_names[i], nr); } for (i = 0; i < ARRAY_SIZE(memcg1_events); i++) @@ -4141,9 +4162,9 @@ static void memcg1_stat_format(struct mem_cgroup *memcg, struct seq_buf *s) for (i = 0; i < ARRAY_SIZE(memcg1_stats); i++) { unsigned long nr; - nr = memcg_page_state(memcg, memcg1_stats[i]); + nr = memcg_page_state_output(memcg, memcg1_stats[i]); seq_buf_printf(s, "total_%s %llu\n", memcg1_stat_names[i], - (u64)nr * memcg_page_state_unit(memcg1_stats[i])); + (u64)nr); } for (i = 0; i < ARRAY_SIZE(memcg1_events); i++) @@ -6625,7 +6646,8 @@ static int memory_stat_show(struct seq_file *m, void *v) static inline unsigned long lruvec_page_state_output(struct lruvec *lruvec, int item) { - return lruvec_page_state(lruvec, item) * memcg_page_state_unit(item); + return lruvec_page_state(lruvec, item) * + memcg_page_state_output_unit(item); } static int memory_numa_stat_show(struct seq_file *m, void *v) -- cgit v1.2.3