Messages in this thread | | | Date | Fri, 17 Dec 2021 11:34:09 -0500 | Subject | Re: [PATCH] cgroup/cpuset: Make child cpusets restrict parents on v1 hierarchy | From | Waiman Long <> |
| |
On 12/17/21 10:48, Michal Koutný wrote: > The commit 1f1562fcd04a ("cgroup/cpuset: Don't let child cpusets > restrict parent in default hierarchy") inteded to relax the check only > on the default hierarchy (or v2 mode) but it dropped the check in v1 > too. > > This patch returns and separates the legacy-only validations so that > they can be considered only in the v1 mode, which should enforce the old > constraints for the sake of compatibility. > > Fixes: 1f1562fcd04a ("cgroup/cpuset: Don't let child cpusets restrict parent in default hierarchy") > Suggested-by: Waiman Long <longman@redhat.com> > Signed-off-by: Michal Koutný <mkoutny@suse.com> > --- > kernel/cgroup/cpuset.c | 52 ++++++++++++++++++++++++++++++++---------- > 1 file changed, 40 insertions(+), 12 deletions(-) > > This is formatted as a separate patch fixing the already queued change in > for-5.17 but it can be eventually squashed into the referenced commit AFAIAC. > > diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c > index 0dd7d853ed17..ce6929ddc0b0 100644 > --- a/kernel/cgroup/cpuset.c > +++ b/kernel/cgroup/cpuset.c > @@ -590,6 +590,35 @@ static inline void free_cpuset(struct cpuset *cs) > kfree(cs); > } > > +/* > + * validate_change_legacy() - Validate conditions specific to legacy (v1) > + * behavior. > + */ > +static int validate_change_legacy(struct cpuset *cur, struct cpuset *trial) > +{ > + struct cgroup_subsys_state *css; > + struct cpuset *c, *par; > + int ret; > + > + WARN_ON_ONCE(!rcu_read_lock_held()); > + > + /* Each of our child cpusets must be a subset of us */ > + ret = -EBUSY; > + cpuset_for_each_child(c, css, cur) > + if (!is_cpuset_subset(c, trial)) > + goto out; > + > + /* On legacy hierarchy, we must be a subset of our parent cpuset. */ > + ret = -EACCES; > + par = parent_cs(cur); > + if (par && !is_cpuset_subset(trial, par)) > + goto out; > + > + ret = 0; > +out: > + return ret; > +} > + > /* > * validate_change() - Used to validate that any proposed cpuset change > * follows the structural rules for cpusets. > @@ -614,20 +643,21 @@ static int validate_change(struct cpuset *cur, struct cpuset *trial) > { > struct cgroup_subsys_state *css; > struct cpuset *c, *par; > - int ret; > - > - /* The checks don't apply to root cpuset */ > - if (cur == &top_cpuset) > - return 0; > + int ret = 0; > > rcu_read_lock(); > - par = parent_cs(cur); > > - /* On legacy hierarchy, we must be a subset of our parent cpuset. */ > - ret = -EACCES; > - if (!is_in_v2_mode() && !is_cpuset_subset(trial, par)) > + if (!is_in_v2_mode()) > + ret = validate_change_legacy(cur, trial); > + if (ret) > + goto out; > + > + /* Remaining checks don't apply to root cpuset */ > + if (cur == &top_cpuset) > goto out; > > + par = parent_cs(cur); > + > /* > * If either I or some sibling (!= me) is exclusive, we can't > * overlap > @@ -1175,9 +1205,7 @@ enum subparts_cmd { > * > * Because of the implicit cpu exclusive nature of a partition root, > * cpumask changes that violates the cpu exclusivity rule will not be > - * permitted when checked by validate_change(). The validate_change() > - * function will also prevent any changes to the cpu list if it is not > - * a superset of children's cpu lists. > + * permitted when checked by validate_change(). > */ > static int update_parent_subparts_cpumask(struct cpuset *cpuset, int cmd, > struct cpumask *newmask,
Thanks for addressing this issue.
Reviewed-by: Waiman Long <longman@redhat.com>
| |