Messages in this thread | | | Date | Mon, 6 Jun 2022 21:29:58 +0800 | Subject | Re: [PATCH v2] mm: memcontrol: add {pgscan,pgsteal}_{kswapd,direct} items in memory.stat of cgroup v2 | From | Qi Zheng <> |
| |
On 2022/6/6 8:03 PM, Michal Hocko wrote: > On Sat 04-06-22 16:22:09, 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. > > I do not remember why we have chosen to report cumulative stats rather > than the direct and kswapd parts. Looking back when Roman has introduced > those (http://lkml.kernel.org/r/1494530183-30808-1-git-send-email-guro@fb.com) > I do not see any discussion around that. So it was likely just not > a priority. > > I have just one question. Say we even decide to have a per memcg kswapd > in some form, would we report that into the same counter?
IMO, I would like it can be reported into the same counter.
> >> 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> > > In any case > Acked-by: Michal Hocko <mhocko@suse.com>
Thanks.
> > One nit below > [...] >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c >> index 0d3fe0a0c75a..fd78c4d6bbc7 100644 >> --- a/mm/memcontrol.c >> +++ b/mm/memcontrol.c >> @@ -1460,6 +1460,28 @@ static inline unsigned long memcg_page_state_output(struct mem_cgroup *memcg, >> return memcg_page_state(memcg, item) * memcg_page_state_unit(item); >> } >> > > I would just add the following for clarity
OK, will do.
> > /* 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 >> +}; >
-- Thanks, Qi
| |