Re: [RFC PATCH] cgroup: use root_mem_cgroup as css when current is not enabled

[Zhaoyang Huang]

On Sun, Aug 14, 2022 at 2:40 PM Zhaoyang Huang <huangzhaoyang@xxxxxxxxx> wrote:
>
> On Sat, Aug 13, 2022 at 3:06 AM Tejun Heo <tj@xxxxxxxxxx> wrote:
> >
> > On Fri, Aug 12, 2022 at 06:09:26PM +0800, zhaoyang.huang wrote:
> > > From: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx>
> > >
> > > Memory charged on group B abserved on belowing v2 hierarchy where we just would
> > > like to only have group E's memory be controlled and B's descendants compete freely
> > > for memory. This should be the consequences of unified hierarchy. Solve this by
> > > have the cgroup without valid memory css alloced use root_mem_cgroup instead of
> > > its ancestor's.
> > >
> > >  A(subtree_control = memory) - B(subtree_control = NULL) - C()
> > >                                                          \ D()
> > >                            - E(subtree_control = memory) - F()
> > >                                                          \ G()
> > >
> > > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx>
> > > ---
> > >  kernel/cgroup/cgroup.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> > > index 1779ccd..b29b3f6 100644
> > > --- a/kernel/cgroup/cgroup.c
> > > +++ b/kernel/cgroup/cgroup.c
> > > @@ -533,6 +533,14 @@ static struct cgroup_subsys_state *cgroup_e_css_by_mask(struct cgroup *cgrp,
> > >        * can't test the csses directly.  Test ss_mask.
> > >        */
> > >       while (!(cgroup_ss_mask(cgrp) & (1 << ss->id))) {
> > > +             /*
> > > +              * charging to the parent cgroup which hasn't distribute
> > > +              * memory control to its descendants doesn't make sense
> > > +              * especially on cgroup v2, where the parent could be configured
> > > +              * to use memory controller as its sibling want to use it
> > > +              */
> > > +             if (memory_cgrp_id == ss->id)
> > > +                     return &root_mem_cgroup->css;
> >
> > This is gonna be a hard nack. A given cgroup always encompasses all the
> > resources consumed in its self-including subtree.
> >
> > Thanks.
> IMHO, I would like to say if it makes more sense as "A given cgroup
> always encompasses all the resources consumed in its ENABLED
> self-including subtree." Otherwise, how should I couple with the
> scenarios I raised in the commit message which I prefer parts of the
> subtrees compete for "memory" while others are free for it. The free
> here is not only without "min/low/high watermarks" but also not
> charged to their own LRU.
I would like to state more why these make sense. Memory cgroup is a
little bit different to other cgroups, that is, memcg will have real
physical resources attached, say pages. From perspective of memory
reclaiming, it is odd to find that pages under free memcgs are charged
to separate LRUs but without any management(no watermark control) and
perhaps affect workingset mechanism by LRU reason. Furthermore, v2
should grant the groups with the right to reject the subsys which
introduced by sibling enable, which could be deemed as v2's
inconvenient. The memcg could also apply subtree_control to enroll all
memory back whenever it want.
> >
> > --
> > tejun