在 2026/10/14 22:05, Waiman Long 写道:
> When prstate_housekeeping_conflict() is called to perform housekeeping
> check with cpumask changes, the cpumask changes are passed in via addmask
> and delmask. With this arrangement, the exclusive CPUs owned by the child
> partitions and not affected by the cpumask change, if any, are excluded
> in the analysis. If the current cpuset is an isolated partition with
> root partition children, it is possible the cpumask change will still
> leave active housekeeping CPUs but then no housekeeping CPUs will be
> left when the child partitions are disabled or invalidated.
>
> As a first step to protect against this possibility, all the exclusive
> CPUs of the cpuset should be considered to be owned by the current cpuset
> when doing housekeeping check. As a result, there is no need to pass in
> separate add_mask and del_mask parameters. They are combined into a single
> new_cpus parameter. The parent_prs parameter can be removed as well.
>
> With this change, the prstate_housekeeping_conflict() call in
> update_parent_effective_cpumask() with partcmd_update and newmask
> is now duplicative and can be removed. The housekeeping check in
> remote_cpus_update(), however, will still be useful as this function can
> be called directly from update_cpumasks_hier() without going through
> partition_cpus_change(). The update_parent_effective_cpumask() call
> from update_cpumasks_hier() is partcmd_update with no cpumask which
> still have the housekeeping check and so is covered.
>
> Fixes: 103b08709e8a ("cgroup/cpuset: Fail if isolated and nohz_full don't
> leave any housekeeping")
> Signed-off-by: Waiman Long <[email protected]>
> ---
> kernel/cgroup/cpuset.c | 77 ++++++++++++++++--------------------------
> 1 file changed, 30 insertions(+), 47 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 2b9aefc0edb8..b2684c98d8c0 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1355,26 +1355,20 @@ static void partition_xcpus_del(int old_prs, struct
> cpuset *parent,
> /*
> * prstate_housekeeping_conflict - check for partition & housekeeping
> conflicts
> * @new_prs: new partition root state to be checked
> - * @parent_prs: Parent partition root state
> - * @add_cpus: additional CPUs to be added to current cpuset
> - * @del_cpus: CPUs to be removed from current cpuset, can be NULL
> + * @new_cpus: cpu mask to be checked
> * Return: true if there is conflict, false otherwise
> *
> * There are two different housekeeping conflicts to be checked:
> - * 1) If new_prs is PRS_ROOT, none of the @add_cpus can be a boot-time
> isolated
> - * CPU. IOW, the whole @add_cpus must be a subset of HK_TYPE_DOMAIN_BOOT.
> + * 1) If new_prs is PRS_ROOT, none of the @new_cpus can be a boot-time
> isolated
> + * CPU. IOW, the whole @new_cpus must be a subset of HK_TYPE_DOMAIN_BOOT.
> * 2) If nohz_full is enabled and we have isolated CPUs, their combination
> must
> - * still leave housekeeping CPUs. This check is only needed if new_prs
> - * differs from parent_prs and one of them is PRS_ISOLATED.
> + * still leave housekeeping CPUs.
> */
> -static bool prstate_housekeeping_conflict(int new_prs, int parent_prs,
> - struct cpumask *add_cpus,
> - struct cpumask *del_cpus)
> +static bool prstate_housekeeping_conflict(int new_prs, struct cpumask
> *new_cpus)
> {
> cpumask_var_t full_hk_cpus;
> int res;
>
> - WARN_ON_ONCE(parent_prs <= 0);
> if (!new_prs)
> return false;
>
> @@ -1393,30 +1387,18 @@ static bool prstate_housekeeping_conflict(int
> new_prs, int parent_prs,
> new_prs = -new_prs;
>
> if (housekeeping_enabled(HK_TYPE_DOMAIN_BOOT) && (new_prs == PRS_ROOT)
> &&
> - !cpumask_subset(add_cpus,
> housekeeping_cpumask(HK_TYPE_DOMAIN_BOOT)))
> + !cpumask_subset(new_cpus,
> housekeeping_cpumask(HK_TYPE_DOMAIN_BOOT)))
> return true;
>
> /*
> * Exit 2nd check early and return false if
> * 1) there is no boot time isolated nohz_full CPU
> - * 2) new_prs is PRS_ROOT, parent_prs is PRS_ISOLATED and no CPU will
> - * be returned back to the parent.
> + * 2) @new_prs is not PRS_ISOLATED
> + * 3) @new_cpus is a subset of isolated_cpus
> */
> if (!housekeeping_enabled(HK_TYPE_KERNEL_NOISE) ||
> - (!del_cpus && (new_prs == PRS_ROOT) && (parent_prs ==
> PRS_ISOLATED)))
> - return false;
> -
> - /*
> - * Now make sure that @add_cpus contains new CPUs to be isolated and
> - * @del_cpus contains isolated CPUs to be un-isolated.
> - * With the guards above, @add_cpus can't be NULL after the swap.
> - */
> - if (parent_prs == PRS_ISOLATED)
> - swap(add_cpus, del_cpus);
> -
> - if (del_cpus &&
> - (cpumask_first_and_and(del_cpus,
> housekeeping_cpumask(HK_TYPE_KERNEL_NOISE),
> - cpu_active_mask) < nr_cpu_ids))
> + (new_prs != PRS_ISOLATED) ||
> + cpumask_subset(new_cpus, isolated_cpus))
> return false;
>
> if (!alloc_cpumask_var(&full_hk_cpus, GFP_KERNEL))
> @@ -1426,7 +1408,7 @@ static bool prstate_housekeeping_conflict(int new_prs,
> int parent_prs,
> housekeeping_cpumask(HK_TYPE_DOMAIN));
> cpumask_andnot(full_hk_cpus, full_hk_cpus, isolated_cpus);
> cpumask_and(full_hk_cpus, full_hk_cpus, cpu_active_mask);
> - res = !cpumask_weight_andnot(full_hk_cpus, add_cpus);
> + res = !cpumask_weight_andnot(full_hk_cpus, new_cpus);
>
One of my test cases regressed in v3 compared to v2. CPUs 0-15 are online,
with nohz_full=2-15 and no isolcpus:
# cd /sys/fs/cgroup
# echo +cpuset > cgroup.subtree_control
# mkdir A
# echo 0 > A/cpuset.cpus
# echo isolated > A/cpuset.cpus.partition
# cat cpuset.cpus.isolated
0
# echo 1 > A/cpuset.cpus
# cat A/cpuset.cpus.partition
isolated invalid (partition config conflicts with housekeeping setup)
The last write should release CPU 0 for housekeeping, leaving A as a valid
isolated partition on CPU 1. It looks like the check uses the state before
the update, sees only CPU 1 available for housekeeping, and incorrectly
invalidates A.
thanks,
Guopeng
> free_cpumask_var(full_hk_cpus);
> return res;
> @@ -1613,7 +1595,7 @@ static int remote_partition_enable(struct cpuset *cs,
> int new_prs,
> return PERR_INVCPUS;
> if (cpumask_intersects(tmp->new_cpus, subpartitions_cpus))
> return PERR_NOCPUS;
> - if (prstate_housekeeping_conflict(new_prs, PRS_ROOT, tmp->new_cpus,
> NULL))
> + if (prstate_housekeeping_conflict(new_prs, tmp->new_cpus))
> return PERR_HKEEPING;
>
> spin_lock_irq(&callback_lock);
> @@ -1715,8 +1697,7 @@ static void remote_cpus_update(struct cpuset *cs,
> struct cpumask *xcpus,
> else if (cpumask_intersects(tmp->addmask, subpartitions_cpus) ||
> cpumask_subset(top_cpuset.effective_cpus,
> tmp->addmask))
> WRITE_ONCE(cs->prs_err, PERR_NOCPUS);
> - else if (prstate_housekeeping_conflict(prs, PRS_ROOT,
> - tmp->addmask,
> tmp->delmask))
> + else if (prstate_housekeeping_conflict(prs, excpus))
> WRITE_ONCE(cs->prs_err, PERR_HKEEPING);
> if (cs->prs_err)
> goto invalidate;
> @@ -1796,7 +1777,6 @@ static int update_parent_effective_cpumask(struct
> cpuset *cs, int cmd,
> int old_prs, new_prs;
> int part_error = PERR_NONE; /* Partition error? */
> struct cpumask *xcpus = user_xcpus(cs);
> - int parent_prs = parent->partition_root_state;
> bool nocpu;
>
> lockdep_assert_cpuset_lock_held();
> @@ -1858,7 +1838,7 @@ static int update_parent_effective_cpumask(struct
> cpuset *cs, int cmd,
> if (cpumask_empty(xcpus))
> return PERR_INVCPUS;
>
> - if (prstate_housekeeping_conflict(new_prs, parent_prs, xcpus,
> NULL))
> + if (prstate_housekeeping_conflict(new_prs, xcpus))
> return PERR_HKEEPING;
>
> if (tasks_nocpu_error(parent, cs, xcpus))
> @@ -1930,14 +1910,6 @@ static int update_parent_effective_cpumask(struct
> cpuset *cs, int cmd,
> parent->effective_xcpus);
> }
>
> - /*
> - * Check for housekeeping conflicts
> - */
> - if (is_partition_valid(cs) &&
> - prstate_housekeeping_conflict(old_prs, parent_prs,
> - tmp->delmask, tmp->addmask))
> - part_error = PERR_HKEEPING;
> -
> /*
> * The new CPUs to be removed from parent's effective CPUs
> * must be present.
> @@ -2430,6 +2402,21 @@ static void partition_cpus_change(struct cpuset *cs,
> struct cpuset *trialcs,
> return;
>
> prs_err = validate_partition(cs, trialcs);
> + if (!prs_err && !cpumask_empty(trialcs->effective_xcpus)) {
> + int parent_prs = is_remote_partition(cs)
> + ? PRS_ROOT : parent_cs(cs)->partition_root_state;
> + /*
> + * Check for housekeeping CPUs conflict assuming that all
> + * the exclusive CPUs belong to the current cpuset. We rely
> + * on the fact that effective_xcpus has been set up by
> + * compute_trial_excpus() which is called previously in
> + * update_cpumask() and update_exclusive_cpumask().
> + */
> + if (parent_prs > 0 &&
> + prstate_housekeeping_conflict(trialcs->partition_root_state,
> + trialcs->effective_xcpus))
> + prs_err = PERR_HKEEPING;
> + }
> if (prs_err) {
> WRITE_ONCE(cs->prs_err, prs_err);
> trialcs->prs_err = prs_err;
> @@ -2922,15 +2909,11 @@ static int update_prstate(struct cpuset *cs, int
> new_prs)
> err = remote_partition_enable(cs, new_prs, &tmpmask);
> }
> } else if (old_prs && new_prs) {
> - int parent_prs = is_remote_partition(cs)
> - ? PRS_ROOT : parent->partition_root_state;
> -
> /*
> * A change in load balance state only, no change in cpumasks.
> * Need to update isolated_cpus.
> */
> - if (prstate_housekeeping_conflict(new_prs, parent_prs,
> - cs->effective_xcpus, NULL))
> + if (prstate_housekeeping_conflict(new_prs, cs->effective_xcpus))
> err = PERR_HKEEPING;
> else
> isolcpus_updated = true;
