On Tue, Apr 23, 2024 at 10:44:07AM -0700, Shakeel Butt wrote: > On Tue, Apr 23, 2024 at 09:58:44AM -0400, Johannes Weiner wrote: > > On Mon, Apr 22, 2024 at 10:18:23PM -0700, Shakeel Butt wrote: > > > At the moment the memcg stats are sized based on the size of enum > > > node_stat_item but not all fields in node_stat_item corresponds to memcg > > > stats. So, rearrage the contents of node_stat_item such that all the > > > memcg specific stats are at the top and then the later patches will make > > > sure that the memcg code will not waste space for non-memcg stats. > > > > > > Signed-off-by: Shakeel Butt <shakeel.butt@xxxxxxxxx> > > > > This series is a great idea and the savings speak for themselves. > > > > But rearranging and splitting vmstats along the memcg-nomemcg line > > seems like an undue burden on the non-memcg codebase and interface. > > > > - It messes with user-visible /proc/vmstat ordering, and sets things > > up to do so on an ongoing basis as stats are added to memcg. > > > > - It also separates related stats (like the workingset ones) in > > /proc/vmstat when memcg only accounts a subset. > > > > Would it make more sense to have a translation table inside memcg? > > Like we have with memcg1_events. > > Thanks for taking a look. I will look into the translation table > approach. The reason I went with this approach was that I am in parallel > looking into rearranging fields of important MM structs and also enums > to improve cache locality. For example, the field NR_SWAPCACHE is always > accessed together with NR_FILE_PAGES, so it makes sense to have them on > same cacheline. So, is the rearrangement of vmstats a big NO or a little > bit here and there is fine unlike what I did with this patch? I'm curious what other folks think. The cache optimization is a stronger argument, IMO, because it directly benefits the users of /proc/vmstat. And it would be fairly self contained inside the node_stat_item enum - "ordered for cache". I was more hesitant about imposing a memcg requirement on the generic vmstat ordering.
