Re: [PATCH v2] mm/memcontrol: Export memcg->watermark via sysfs for v2 memcg

[Ganesan Rajagopal <rganesan@arista.com>]

On Wed, May 11, 2022 at 11:19 PM Michal Koutný <mkoutny@suse.com> wrote:
>
> Hi.
>
> On Fri, May 06, 2022 at 10:09:16PM -0700, Ganesan Rajagopal <rganesan@arista.com> wrote:
> > We run a lot of automated tests when building our software and run into
> > OOM scenarios when the tests run unbounded. v1 memcg exports
> > memcg->watermark as "memory.max_usage_in_bytes" in sysfs. We use this
> > metric to heuristically limit the number of tests that can run in
> > parallel based on per test historical data.
> >
> > This metric is currently not exported for v2 memcg and there is no
> > other easy way of getting this information. getrusage() syscall returns
> > "ru_maxrss" which can be used as an approximation but that's the max
> > RSS of a single child process across all children instead of the
> > aggregated max for all child processes. The only work around is to
> > periodically poll "memory.current" but that's not practical for
> > short-lived one-off cgroups.
> >
> > Hence, expose memcg->watermark as "memory.peak" for v2 memcg.
>
> It'll save some future indirections if the commit messages includes the
> argument about multiple readers and purposeful irresetability.

Good point. The patch has already been picked up for mm-unstable. I don't
know what's the process in this situation. Should I post a "[PATCH v3]"
with an updated commit message?

>
> >
> > Signed-off-by: Ganesan Rajagopal <rganesan@arista.com>
> > ---
> >  Documentation/admin-guide/cgroup-v2.rst |  7 +++++++
> >  mm/memcontrol.c                         | 13 +++++++++++++
> >  2 files changed, 20 insertions(+)
>
> Besides that it looks useful and correct, feel free to add
> Reviewed-by: Michal Koutný <mkoutny@suse.com>

Thank you.

Ganesan