On 12/15/21 07:23, Michal Koutný wrote:
On Mon, Dec 13, 2021 at 10:41:23AM -1000, Tejun Heo tj@kernel.org wrote:
To address this issue, the check is now removed for the default hierarchy to free parent cpusets from being restricted by child cpusets. The check will still apply for legacy hierarchy.
I'm trying to find whether something in update_cpumasks_hier() ensures the constraint is checkd on the legacy hierarchy but it seems to me this baby was thrown out with the bathwater. How is the legacy check still applied?
Yes, you are right. I did remove the check for legacy hierarchy too.
Applied to cgroup/for-5.17.
It comes out a bit more complex if I want to achieve both variants in the below followup:
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c index 0dd7d853ed17..8b6e06f504f6 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))
I think you still need to guard it with "!is_in_v2_mode()".
if (!is_in_v2_mode()) { ret = validate_change_legacy(cur, trial); if (ret) goto out; }
- ret = validate_change_legacy(cur, trial);
- if (ret)
goto out;- /* Remaining checks don't apply to root cpuset */
- ret = 0;
- if (cur == &top_cpuset) goto out;
- par = parent_cs(cur);
- /*
- If either I or some sibling (!= me) is exclusive, we can't
- overlap
Cheers, Longman