| Message ID | 20211217154854.41409-1-mkoutny@suse.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d068eebbd4822b6c14a7ea375dfe53ca5c69c776 |
| Headers | show |
| Series | cgroup/cpuset: Make child cpusets restrict parents on v1 hierarchy | expand |
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>
On Fri, Dec 17, 2021 at 04:48:54PM +0100, 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> Applied to cgroup/for-5.17-fixes. Thanks.
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,
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.