linux-kernel - Re: [PATCH] sched/fair: Add more core cookie check in wake up fast path

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] [day] [month] [year] [list]

Message-ID: <92c22ad2-d84c-44e4-b34f-a80f7f3748a6@amd.com>
Date: Fri, 21 Nov 2025 08:24:17 +0530
From: K Prateek Nayak <kprateek.nayak@....com>
To: Fernand Sieber <sieberf@...zon.com>, <mingo@...hat.com>,
	<peterz@...radead.org>
CC: <linux-kernel@...r.kernel.org>, <juri.lelli@...hat.com>,
	<vincent.guittot@...aro.org>, <dietmar.eggemann@....com>,
	<rostedt@...dmis.org>, <bsegall@...gle.com>, <mgorman@...e.de>,
	<vschneid@...hat.com>, <dwmw@...zon.co.uk>, <jschoenh@...zon.de>,
	<liuyuxua@...zon.com>
Subject: Re: [PATCH] sched/fair: Add more core cookie check in wake up fast
 path

Hello Fernand,

On 11/20/2025 3:49 PM, Fernand Sieber wrote:
> The fast path in select_idle_sibling() can place tasks on CPUs without
> considering core scheduling constraints, potentially causing immediate
> force idle when the sibling runs an incompatible task.
> 
> Add cookie compatibility checks before selecting a CPU in the fast path.
> This prevents placing waking tasks on CPUs where the sibling is running
> an incompatible task, reducing force idle occurrences.
> 
> Testing
> =======
> 
> Perf testing using 10 threads on 8 CPUs on a platform with each core
> sporting two hyperthreads.
> Each thread is running 1200 iterations of a cycle consisting of a random
> period of work between 0-10ms followed by a random period of sleep 0-30ms.
> 
> The goal of this configuration is to apply a light load on the system
> (33%), with randomization causing opportunities for scheduling placement
> including all cases of idle cores, partially idle cores and fully busy
> cores.
> 
> Each test configuration is run 3 times and time to work completion is
> measured and averaged. First configuration doesn't use cookies, second
> configuration groups the 10 threads in 5 pairs of cookies, third
> configuration also groups the 10 threads in 5 pairs of cookies, but with
> this patch applied.
> 
> Test Results (seconds)
> 
> Configuration          Run 1    Run 2    Run 3    Average  Overhead
> No cookie              24.653   24.454   24.586   24.564   0.000
> Cookie                 25.896   26.256   25.979   26.044   1.480
> Cookie + patch         25.346   25.007   25.042   25.132   0.568
> 
> The patch reduces cookie overhead by 61.7% (0.568/1.480 = 0.383)

Thank you for including the benchmark numbers.

> 
> Signed-off-by: Fernand Sieber <sieberf@...zon.com>
> ---
>  kernel/sched/fair.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 5b752324270b..90ceb3da2251 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7647,7 +7647,7 @@ static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int t
>  		 */
>  		if (!cpumask_test_cpu(cpu, sched_domain_span(sd)))
>  			continue;
> -		if (available_idle_cpu(cpu) || sched_idle_cpu(cpu))
> +		if (__select_idle_cpu(cpu, p) != -1)
>  			return cpu;
>  	}
> 
> @@ -7841,6 +7841,7 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
>  	lockdep_assert_irqs_disabled();
> 
>  	if ((available_idle_cpu(target) || sched_idle_cpu(target)) &&
> +	    sched_core_cookie_match(cpu_rq(target), p) &&

nit.

You can replace the whole

  (available_idle_cpu() || sched_idle_cpu()) && sched_core_cookie_match()

with "__select_idle_cpu() != 1" but since this pattern keeps repeating,
you can perhaps extract it into a helper __is_idle_cpu() that returns a
boolean and you can add a follow up cleanup to convert all the:

  idle_cpu = __select_idle_cpu(cpu, p);
  if ((unsigned int)idle_cpu < nr_cpumask_bits)
    return idle_cpu;

pattern to:

  if (__is_idle_cpu(cpu, p))
    return cpu;

after which select_idle_core() for !CONFIG_SCHED_SMT would be the only
user of __select_idle_cpu() and you can move that entire logic into the
select_idle_core() function. Thoughts?

Apart from that, these changes look good to me since they are making all
the idle cpu checks along the wakeup path consistent with those in
select_idle_cpu() so feel free to add:

Reviewed-by: K Prateek Nayak <kprateek.nayak@....com>

>  	    asym_fits_cpu(task_util, util_min, util_max, target))
>  		return target;
> 
> @@ -7849,6 +7850,7 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
>  	 */
>  	if (prev != target && cpus_share_cache(prev, target) &&
>  	    (available_idle_cpu(prev) || sched_idle_cpu(prev)) &&
> +	    sched_core_cookie_match(cpu_rq(prev), p) &&
>  	    asym_fits_cpu(task_util, util_min, util_max, prev)) {
> 
>  		if (!static_branch_unlikely(&sched_cluster_active) ||
> @@ -7881,6 +7883,7 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
>  	    recent_used_cpu != target &&
>  	    cpus_share_cache(recent_used_cpu, target) &&
>  	    (available_idle_cpu(recent_used_cpu) || sched_idle_cpu(recent_used_cpu)) &&
> +	    sched_core_cookie_match(cpu_rq(recent_used_cpu), p) &&
>  	    cpumask_test_cpu(recent_used_cpu, p->cpus_ptr) &&
>  	    asym_fits_cpu(task_util, util_min, util_max, recent_used_cpu)) {
> 
-- 
Thanks and Regards,
Prateek