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

[Shakeel Butt <shakeelb@google.com>]

On Fri, May 6, 2022 at 1:50 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> On Fri, May 06, 2022 at 08:56:31AM -0700, Shakeel Butt wrote:
> > On Fri, May 6, 2022 at 7:57 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
> > >
> > > On Thu, May 05, 2022 at 05:13:30AM -0700, Ganesan Rajagopal wrote:
> > > > v1 memcg exports memcg->watermark as "memory.mem_usage_in_bytes" in
> > > > sysfs. This is missing for v2 memcg though "memory.current" is exported.
> > > > There is no other easy way of getting this information in Linux.
> > > > getrsuage() returns ru_maxrss but that's the max RSS of a single process
> > > > instead of the aggregated max RSS of all the processes. Hence, expose
> > > > memcg->watermark as "memory.watermark" for v2 memcg.
> > > >
> > > > Signed-off-by: Ganesan Rajagopal <rganesan@arista.com>
> > >
> > > This wasn't initially added to cgroup2 because its usefulness is very
> > > specific: it's (mostly) useless on limited cgroups, on long-running
> > > cgroups, and on cgroups that are recycled for multiple jobs. And I
> > > expect these categories apply to the majority of cgroup usecases.
> > >
> > > However, for the situation where you want to measure the footprint of
> > > a short-lived, unlimited one-off cgroup, there really is no good
> > > alternative. And it's a legitimate usecase. It doesn't cost much to
> > > maintain this info. So I think we should go ahead with this patch.
> > >
> > > But please add a blurb to Documentation/admin-guide/cgroup-v2.rst.
> >
> > No objection from me. I do have two points: (1) watermark is not a
> > good name for this interface, maybe max_usage or something.
>
> How about memory.peak? It'd be nice to avoid underscores.
>
> > (2) a way to reset (i.e. write to it, reset it).
>
> We used to have that with cgroup1, but it gets weird in modern cgroup
> environments when there can be multiple consumers. One of them resets
> the stat for their own purpose, now the others have no idea what
> sample time frame they are looking at.
>
> It'd be more robust if we just said "peak usage since birth of
> cgroup". If you want to sample new work, create a new cgroup.

SGTM for both.