lkml.org 
[lkml]   [2021]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] cgroup/cpuset: Make child cpusets restrict parents on v1 hierarchy
From
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>

\
 
 \ /
  Last update: 2021-12-17 17:34    [W:0.103 / U:0.112 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site