lkml.org 
[lkml]   [2021]   [Dec]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v9 1/7] cgroup/cpuset: Don't let child cpusets restrict parent in default hierarchy
From

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

\
 
 \ /
  Last update: 2021-12-15 19:01    [W:0.507 / U:0.940 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site