On Mon, Sep 25, 2023 at 6:50 AM Michal Hocko <mhocko@xxxxxxxx> wrote: > > On Fri 22-09-23 17:57:38, Yosry Ahmed wrote: > > 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. > > What kind of practical difference does this change make? Is it worth > additional code? As explained in patch 2's commit message, the value passed into memcg_rstat_updated() is used for the "flush only if not worth it" heuristic. As we have discussed in different threads in the past few weeks, unnecessary flushes can cause increased global lock contention and/or latency. Byte-sized paths (percpu, slab, zswap, ..) feed bytes into the heuristic, but those are interpreted as pages, which means we will flush earlier than we should. This was noticed by code inspection. How much does this matter in practice? I would say it depends on the workload: how many percpu/slab allocations are being made vs. how many flushes are requested. On a system with 100 cpus, 25M of stat updates are needed for a flush usually, but ~6K of slab/percpu updates will also (mistakenly) cause a flush. > > > 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@xxxxxxxxxx/ > > > > v1 -> v2: > > - Rebased on top of mm-unstable. > > > > Yosry Ahmed (2): > > mm: memcg: refactor page state unit helpers > > mm: memcg: normalize the value passed into memcg_rstat_updated() > > > > mm/memcontrol.c | 64 +++++++++++++++++++++++++++++++++++++++---------- > > 1 file changed, 51 insertions(+), 13 deletions(-) > > > > -- > > 2.42.0.515.g380fc7ccd1-goog > > -- > Michal Hocko > SUSE Labs
