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: <3cf686fe-7068-440f-84c6-414db71b9c36@linux.ibm.com>
Date: Fri, 23 Jan 2026 12:44:34 +0530
From: Shrikanth Hegde <sshegde@...ux.ibm.com>
To: K Prateek Nayak <kprateek.nayak@....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()



On 1/23/26 11:57 AM, K Prateek Nayak wrote:
> 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.
> 

you got it right. keep sd_llc only.

>>
>>
>> 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.
>