[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9168ffab-b0a8-4024-a1f4-966b9f95c953@redhat.com>
Date: Sun, 19 Oct 2025 22:39:22 -0400
From: Waiman Long <llong@...hat.com>
To: Chen Ridong <chenridong@...weicloud.com>, tj@...nel.org,
hannes@...xchg.org, mkoutny@...e.com
Cc: cgups@...r.kernel.org, linux-kernel@...r.kernel.org,
lujialin4@...wei.com, chenridong@...wei.com
Subject: Re: [PATCH -next RFC 03/16] cpuset: factor out partition_enable()
function
On 9/28/25 3:12 AM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@...wei.com>
>
> Extract the core partition enablement logic into a dedicated
> partition_enable() function. This refactoring centralizes updates to key
> cpuset data structures including remote_sibling, effective_xcpus,
> partition_root_state, and prs_err.
>
> The function handles the complete partition enablement workflow:
> - Adding exclusive CPUs via partition_xcpus_add()
> - Managing remote sibling relationships
> - Synchronizing effective exclusive CPUs mask
> - Updating partition state and error status
> - Triggering required scheduler domain rebuilds
>
> This creates a coherent interface for partition operations and establishes
> a foundation for future local partition support while maintaining existing
> remote partition behavior.
>
> Signed-off-by: Chen Ridong <chenridong@...wei.com>
> ---
> kernel/cgroup/cpuset.c | 55 +++++++++++++++++++++++++++++++++---------
> 1 file changed, 44 insertions(+), 11 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 0787904321a9..43ce62f4959c 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1515,6 +1515,49 @@ static inline bool is_local_partition(struct cpuset *cs)
> return is_partition_valid(cs) && !is_remote_partition(cs);
> }
>
> +static void partition_state_update(struct cpuset *cs, int new_prs,
> + enum prs_errcode prs_err)
> +{
> + lockdep_assert_held(&callback_lock);
> +
> + cs->partition_root_state = new_prs;
> + WRITE_ONCE(cs->prs_err, prs_err);
> + if (!is_partition_valid(cs))
> + reset_partition_data(cs);
> +}
> +
> +/**
> + * partition_enable - Transitions a cpuset to a partition root
> + * @cs: The cpuset to enable partition for
> + * @parent: Parent cpuset of @cs, NULL for remote parent
> + * @new_prs: New partition root state to set
> + * @new_excpus: New exclusive CPUs mask for the partition
> + *
> + * Transitions a cpuset to a partition root, only for v2.
> + */
> +static void partition_enable(struct cpuset *cs, struct cpuset *parent,
> + int new_prs, struct cpumask *new_excpus)
> +{
> + bool isolcpus_updated;
> +
> + lockdep_assert_held(&cpuset_mutex);
> + WARN_ON_ONCE(new_prs <= 0);
> + WARN_ON_ONCE(!cpuset_v2());
> +
> + if (cs->partition_root_state == new_prs)
> + return;
> +
> + spin_lock_irq(&callback_lock);
> + /* enable partition should only add exclusive cpus */
> + isolcpus_updated = partition_xcpus_add(new_prs, parent, new_excpus);
> + list_add(&cs->remote_sibling, &remote_children);
> + cpumask_copy(cs->effective_xcpus, new_excpus);
> + partition_state_update(cs, new_prs, PERR_NONE);
> + spin_unlock_irq(&callback_lock);
> + update_unbound_workqueue_cpumask(isolcpus_updated);
> + cpuset_force_rebuild();
> +}
> +
partition_enable() is supposed to be a common helper used for the
creation of both local and remote partitions. The one in this patch does
work for remote partition but not for local partition. I would prefer to
make it good for both cases when you introduce it instead adding code in
patch 6 to make it work for local partition later in the series. It will
make it easier to review instead of jumping back and forth to make sure
that it will do the right thing.
Cheers,
Longman
Powered by blists - more mailing lists