linux-kernel - Re: [PATCH v3 8/8] sched/fair: Simplify SIS_UTIL handling in select_idle

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <00a09fd8-6ed7-4ebf-a146-d054e98cc45b@amd.com>
Date: Fri, 23 Jan 2026 11:57:28 +0530
From: K Prateek Nayak <kprateek.nayak@....com>
To: Shrikanth Hegde <sshegde@...ux.ibm.com>
CC: Dietmar Eggemann <dietmar.eggemann@....com>, Steven Rostedt
	<rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>, Mel Gorman
	<mgorman@...e.de>, Valentin Schneider <vschneid@...hat.com>, Chen Yu
	<yu.c.chen@...el.com>, "Gautham R. Shenoy" <gautham.shenoy@....com>, "Ingo
 Molnar" <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>, Juri Lelli
	<juri.lelli@...hat.com>, Vincent Guittot <vincent.guittot@...aro.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 8/8] sched/fair: Simplify SIS_UTIL handling in
 select_idle_cpu()

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@...ux.ibm.com>

Thank you for reviewing the series.

-- 
Thanks and Regards,
Prateek