linux-kernel - Re: [PATCH v2 1/6] powerpc/smp: Cache CPU has Asymmetric SMP

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 for Android: free password hash cracker in your pocket

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

Message-ID: <87y1fz5j03.fsf@mail.lhotse>
Date:   Thu, 19 Oct 2023 15:33:16 +1100
From:   Michael Ellerman <mpe@...erman.id.au>
To:     Srikar Dronamraju <srikar@...ux.vnet.ibm.com>
Cc:     linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
        Christophe Leroy <christophe.leroy@...roup.eu>,
        Nicholas Piggin <npiggin@...il.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Rohan McLure <rmclure@...ux.ibm.com>,
        Valentin Schneider <vschneid@...hat.com>,
        Josh Poimboeuf <jpoimboe@...nel.org>,
        "ndesaulniers@...gle.com" <ndesaulniers@...gle.com>,
        Mark Rutland <mark.rutland@....com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/6] powerpc/smp: Cache CPU has Asymmetric SMP

Srikar Dronamraju <srikar@...ux.vnet.ibm.com> writes:
> Currently cpu feature flag is checked whenever powerpc_smt_flags gets
> called. This is an unnecessary overhead. CPU_FTR_ASYM_SMT is set based
> on the processor and all processors will either have this set or will
> have it unset.

The cpu_has_feature() test is implemented with a static key.

So AFAICS this is just replacing one static key with another?

I see that you use the new static key in subsequent patches. But
couldn't those just use the existing cpu feature test?

Anyway I'd be interested to see how the generated code differs
before/after this.

cheers

> Hence only check for the feature flag once and cache it to be used
> subsequently. This commit will help avoid a branch in powerpc_smt_flags
>
> Signed-off-by: Srikar Dronamraju <srikar@...ux.vnet.ibm.com>
> ---
> Changelog:
> v1->v2: Using static keys instead of a variable.
> Using pr_info_once instead of printk
>
>  arch/powerpc/kernel/smp.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 5826f5108a12..37c41297c9ce 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -988,18 +988,16 @@ static int __init init_thread_group_cache_map(int cpu, int cache_property)
>  }
>  
>  static bool shared_caches;
> +DEFINE_STATIC_KEY_FALSE(powerpc_asym_packing);
>  
>  #ifdef CONFIG_SCHED_SMT
>  /* cpumask of CPUs with asymmetric SMT dependency */
>  static int powerpc_smt_flags(void)
>  {
> -	int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
> +	if (static_branch_unlikely(&powerpc_asym_packing))
> +		return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
>  
> -	if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
> -		printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
> -		flags |= SD_ASYM_PACKING;
> -	}
> -	return flags;
> +	return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
>  }
>  #endif
>  
> @@ -1686,6 +1684,11 @@ static void __init fixup_topology(void)
>  {
>  	int i;
>  
> +	if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
> +		pr_info_once("Enabling Asymmetric SMT scheduling\n");
> +		static_branch_enable(&powerpc_asym_packing);
> +	}
> +
>  #ifdef CONFIG_SCHED_SMT
>  	if (has_big_cores) {
>  		pr_info("Big cores detected but using small core scheduling\n");
> -- 
> 2.31.1