Re: [PATCH v3] mm: memcg: use rstat for non-hierarchical stats

[Yosry Ahmed]

On Wed, Aug 2, 2023 at 12:40 AM Michal Hocko <mhocko@xxxxxxxx> wrote:
>
> On Tue 01-08-23 10:29:39, Yosry Ahmed wrote:
> > On Tue, Aug 1, 2023 at 9:39 AM Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote:
> [...]
> > > > Have you measured any potential regression for cgroup v2 which collects
> > > > all this data without ever using it (AFAICS)?
> > >
> > > I did not. I did not expect noticeable regressions given that all the
> > > extra work is done during flushing, which should mostly be done by the
> > > asynchronous worker, but can also happen in the stats reading context.
> > > Let me run the same script on cgroup v2 just in case and report back.
> >
> > A few runs on mm-unstable with this patch:
> >
> > # time cat /sys/fs/cgroup/cg*/memory.stat > /dev/null
>
> Is this really representative test to make? I would have expected the
> overhead would be mostly in mem_cgroup_css_rstat_flush (if it is visible
> at all of course). This would be more likely visible in all cpus busy
> situation (you can try heavy parallel kernel build from tmpfs for
> example).


I see. You are more worried about asynchronous flushing eating cpu
time rather than the synchronous flushing being slower. In fact, my
test is actually not representative at all because probably most of
the cgroups either do not have updates or the asynchronous flusher got
to them first.

Let me try a workload that is more parallel & cpu intensive and report
back. I am thinking of parallel reclaim/refault loops since both
reclaim and refault paths invoke stat updates and stat flushing.

>
> [...]
>
> > It looks like there are no regressions on cgroup v2 when reading the
> > stats. Please let me know if you want me to send a new version with
> > the cgroup v2 results as well in the commit log -- or I can just send
> > a new commit log. Whatever is easier for Andrew.
>
> Updating the changelog should be good enough.
> --
> Michal Hocko
> SUSE Labs