Re: [PATCH -next] cpuset: Remove unnecessary checks in rebuild_sched_domains_locked

[Waiman Long]

On 11/25/25 10:17 PM, Chen Ridong wrote:
> On 2025/11/26 10:33, Waiman Long wrote:
> > On 11/25/25 8:01 PM, Chen Ridong wrote:
> > > On 2025/11/26 2:16, Waiman Long wrote:
> > > > > active CPUs, preventing partition_sched_domains from being invoked with
> > > > > offline CPUs.
> > > > >
> > > > > Signed-off-by: Chen Ridong <chenridong@xxxxxxxxxx>
> > > > > ---
> > > > >     kernel/cgroup/cpuset.c | 29 ++++++-----------------------
> > > > >     1 file changed, 6 insertions(+), 23 deletions(-)
> > > > >
> > > > > diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> > > > > index daf813386260..1ac58e3f26b4 100644
> > > > > --- a/kernel/cgroup/cpuset.c
> > > > > +++ b/kernel/cgroup/cpuset.c
> > > > > @@ -1084,11 +1084,10 @@ void dl_rebuild_rd_accounting(void)
> > > > >      */
> > > > >     void rebuild_sched_domains_locked(void)
> > > > >     {
> > > > > -    struct cgroup_subsys_state *pos_css;
> > > > >         struct sched_domain_attr *attr;
> > > > >         cpumask_var_t *doms;
> > > > > -    struct cpuset *cs;
> > > > >         int ndoms;
> > > > > +    int i;
> > > > >           lockdep_assert_cpus_held();
> > > > >         lockdep_assert_held(&cpuset_mutex);
> > > > In fact, the following code and the comments above in rebuild_sched_domains_locked() are also no
> > > > longer relevant. So you may remove them as well.
> > > >
> > > >           if (!top_cpuset.nr_subparts_cpus &&
> > > >               !cpumask_equal(top_cpuset.effective_cpus, cpu_active_mask))
> > > >                   return;
> > > Thank you for reminding me.
> > >
> > > I initially retained this code because I believed it was still required for cgroup v1, as I recalled
> > > that synchronous operation is exclusive to cgroup v2.
> > >
> > > However, upon re-examining the code, I confirm it can be safely removed. For cgroup v1,
> > > rebuild_sched_domains_locked is called synchronously, and only the migration task (handled by
> > > cpuset_migrate_tasks_workfn) operates asynchronously. Consequently, cpuset_hotplug_workfn is
> > > guaranteed to complete before the hotplug workflow finishes.
> > Yes, v1 still have a task migration part that is done asynchronously because of the lock ordering
> > issue. Even if this code has to be left because of v1, you should still update the comment to
> > reflect that. Please try to keep the comment updated to help others to have a better understanding
> > of what the code is doing.
> >
> > Thanks,
> > Longman
> Hi Longman,
>
> Just to confirm (in case I misunderstood): I believe it is safe to remove the check on
> top_cpuset.effective_cpus (for both cgroup v1 and v2). I will proceed to remove both the
> corresponding code and its associated comment(not update the comment).
>
>            if (!top_cpuset.nr_subparts_cpus &&
>                !cpumask_equal(top_cpuset.effective_cpus, cpu_active_mask))
>                    return;
>
> Additionally, I should add a comment to clarify the rationale for introducing the
> WARN_ON_ONCE(!cpumask_subset(doms[i], cpu_active_mask)) warning.
>
> Does this approach look good to you? Please let me know if I’ve missed anything or if further
> adjustments are needed.
Yes, that is good for me. I was just talking about a hypothetical 
situation, not that you have to update the comment.

Cheers,
Longman