Re: [PATCH v3 8/8] sched/fair: Simplify SIS_UTIL handling in select_idle_cpu()

[K Prateek Nayak <kprateek.nayak@amd.com>]

Hello Shrikanth,

On 1/23/2026 11:36 AM, Shrikanth Hegde wrote:
>> +        /* because !--nr is the condition to stop scan */
>> +        nr = READ_ONCE(sd->shared->nr_idle_scan) + 1;
>> +        /* overloaded LLC is unlikely to have idle cpu/core */
>> +        if (nr == 1)
>> +            return -1;
>>       }
> 
> 
> I stared at sd->shared->nr_idle_scan for a while to see why it is safe
> even when lets say there is no LLC domain.
> 
> It is because it is sd_llc here. Not any other domains. and
> there is sd_llc check before calling select_idle_cpu.

Ack! We come here with a valid "sd_llc" from select_idle_sibling()
and "sd" and "sd->shared" are freed at the same time via call_rcu() when
the last reference is dropped so having a reference to "sd" guarantees
"sd->shared" is not freed and the topology bits will ensure
"sd_llc->shared" is always present (or it screams and we crash here).

> 
> So maybe add a comment here, saying null check for sd_llc is already there
> and that's why it is safe to call it directly.
> 
>>   +    if (!cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr))
>> +        return -1;
>> +
>>       if (static_branch_unlikely(&sched_cluster_active)) {
>>           struct sched_group *sg = sd->groups;
>>   
> 
> While reading this series, this reminded me we had discussed about unifying
> sd_llc->shared and sd_llc_shared thing into one (in v1 or v2).
> is that dropped or you plan to fix it after this series?

Must have slipped out of my mind! I believe the only other user of
"sd_llc_shared" directly would then be nohz_balancer_kick() and
{test,set}_idle_cores().

Out of those, I would only consider set_idle_core() from wakeup to
be a fast-path but we'll already have a "sd_llc" reference there
so we should be able to flip the idle_cores indicator without
needing an extra dereference.

We can only keep per-CPU "sd_llc" and remove "sd_llc_shared". I
hope that is what you were suggesting. Otherwise please let me
know if I misinterpreted the question.

> 
> 
> Other than minor comments and nits series looks good to me.
> So, for the series.
> 
> Reviewed-by: Shrikanth Hegde <sshegde@linux.ibm.com>

Thank you for reviewing the series.

-- 
Thanks and Regards,
Prateek