*sigh*, +tj
On Tue, Mar 30, 2021 at 11:23:10AM +0200, Peter Zijlstra wrote: > On Wed, Mar 24, 2021 at 05:40:17PM -0400, Joel Fernandes (Google) wrote: > > From: Josh Don <[email protected]> > > > > This adds the API to set/get the cookie for a given cgroup. This > > interface lives at cgroup/cpu.core_tag. > > > > The cgroup interface can be used to toggle a unique cookie value for all > > descendent tasks, preventing these tasks from sharing with any others. > > See Documentation/admin-guide/hw-vuln/core-scheduling.rst for a full > > rundown of both this and the per-task API. > > I refuse to read RST. Life's too short for that. > > > +u64 cpu_core_tag_read_u64(struct cgroup_subsys_state *css, > > + struct cftype *cft) > > +{ > > + return !!css_tg(css)->core_tagged; > > +} > > + > > +int cpu_core_tag_write_u64(struct cgroup_subsys_state *css, struct cftype > > *cft, > > + u64 val) > > +{ > > + static DEFINE_MUTEX(sched_core_group_mutex); > > + struct task_group *tg = css_tg(css); > > + struct cgroup_subsys_state *css_tmp; > > + struct task_struct *p; > > + unsigned long group_cookie; > > + int ret = 0; > > + > > + if (val > 1) > > + return -ERANGE; > > + > > + if (!static_branch_likely(&sched_smt_present)) > > + return -EINVAL; > > + > > + mutex_lock(&sched_core_group_mutex); > > + > > + 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? > > > + } 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? > > > + ret = -EBUSY; > > + goto out_unlock; > > + } > > + } else { > > + goto out_unlock; > > + } > >
