Re: [PATCH resend 5/8] sched: cgroup cookie API for core scheduling

[Josh Don <joshdon@google.com>]

On Tue, Mar 30, 2021 at 2:29 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > On Wed, Mar 24, 2021 at 05:40:17PM -0400, Joel Fernandes (Google) wrote:
> > > +
> > > +   if (!tg->core_tagged && val) {
> > > +           /* Tag is being set. Check ancestors and descendants. */
> > > +           if (cpu_core_get_group_cookie(tg) ||
> > > +               cpu_core_check_descendants(tg, true /* tag */)) {
> > > +                   ret = -EBUSY;
> > > +                   goto out_unlock;
> > > +           }
> >
> > So the desired semantics is to only allow a single tag on any upwards
> > path? Isn't that in conflict with the cgroup requirements?
> >
> > TJ?

I carried this requirement over from the previous iteration, but I
don't see a reason why we can't just dump this and have each task use
the group cookie of its closest tagged ancestor. Joel, is there any
context here I'm missing?

FWIW I also just realized that cpu_core_check_descendants() is busted
as it recurses only on one child.

> > > +   } else if (tg->core_tagged && !val) {
> > > +           /* Tag is being reset. Check descendants. */
> > > +           if (cpu_core_check_descendants(tg, true /* tag */)) {
> >
> > I'm struggling to understand this. If, per the above, you cannot set
> > when either a parent is already set or a child is set, then how can a
> > child be set to refuse clearing?

Yes this is superfluous with the above semantics.