Received-SPF: pass (google.com: domain of linux-kernel+bounces-213098-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: Michael Ellerman <mpe@ellerman.id.au>
To: "Nysal Jan K.A." <nysal@linux.ibm.com>, Thomas Gleixner
 <tglx@linutronix.de>
Cc: Tyrel Datwyler <tyreld@linux.ibm.com>, Michal Suchanek
 <msuchanek@suse.de>, "Nysal Jan K.A" <nysal@linux.ibm.com>, Nicholas
 Piggin <npiggin@gmail.com>, Christophe Leroy
 <christophe.leroy@csgroup.eu>, "Naveen N. Rao"
 <naveen.n.rao@linux.ibm.com>, Peter Zijlstra <peterz@infradead.org>,
 Laurent Dufour <ldufour@linux.ibm.com>, linuxppc-dev@lists.ozlabs.org,
 linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/2] Skip offline cores when enabling SMT on PowerPC
In-Reply-To: <20240612185046.1826891-1-nysal@linux.ibm.com>
References: <20240612185046.1826891-1-nysal@linux.ibm.com>
Date: Thu, 13 Jun 2024 21:34:10 +1000
Message-ID: <875xudoz4d.fsf@mail.lhotse>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

"Nysal Jan K.A." <nysal@linux.ibm.com> writes:
> From: "Nysal Jan K.A" <nysal@linux.ibm.com>
>
> After the addition of HOTPLUG_SMT support for PowerPC [1] there was a
> regression reported [2] when enabling SMT.

This implies it was a kernel regression. But it can't be a kernel
regression because previously there was no support at all for the sysfs
interface on powerpc.

IIUIC the regression was in the ppc64_cpu userspace tool, which switched
to using the new kernel interface without taking into account the way it
behaves.

Or are you saying the kernel behaviour changed on x86 after the powerpc
HOTPLUG_SMT was added?

> On a system with at least
> one offline core, when enabling SMT, the expectation is that no CPUs
> of offline cores are made online.
>
> On a POWER9 system with 4 cores in SMT4 mode:
> $ ppc64_cpu --info
> Core   0:    0*    1*    2*    3*
> Core   1:    4*    5*    6*    7*
> Core   2:    8*    9*   10*   11*
> Core   3:   12*   13*   14*   15*
>
> Turn only one core on:
> $ ppc64_cpu --cores-on=1
> $ ppc64_cpu --info
> Core   0:    0*    1*    2*    3*
> Core   1:    4     5     6     7
> Core   2:    8     9    10    11
> Core   3:   12    13    14    15
>
> Change the SMT level to 2:
> $ ppc64_cpu --smt=2
> $ ppc64_cpu --info
> Core   0:    0*    1*    2     3
> Core   1:    4     5     6     7
> Core   2:    8     9    10    11
> Core   3:   12    13    14    15
>
> As expected we see only two CPUs of core 0 are online
>
> Change the SMT level to 4:
> $ ppc64_cpu --smt=4
> $ ppc64_cpu --info
> Core   0:    0*    1*    2*    3*
> Core   1:    4*    5*    6*    7*
> Core   2:    8*    9*   10*   11*
> Core   3:   12*   13*   14*   15*
>
> The CPUs of offline cores are made online. If a core is offline then
> enabling SMT should not online CPUs of this core.

That's the way the ppc64_cpu tool behaves, but it's not necessarily what
other arches want.

> An arch specific
> function topology_is_core_online() is proposed to address this.
> Another approach is to check the topology_sibling_cpumask() for any
> online siblings. This avoids the need for an arch specific function
> but is less efficient and more importantly this introduces a change
> in existing behaviour on other architectures.

It's only x86 and powerpc right?

Having different behaviour on the only two arches that support the
interface does not seem like a good result.

> What is the expected behaviour on x86 when enabling SMT and certain cores
> are offline? 

AFAIK no one really touches SMT on x86 other than to turn it off for
security reasons.

cheers

> [1] https://lore.kernel.org/lkml/20230705145143.40545-1-ldufour@linux.ibm.com/
> [2] https://groups.google.com/g/powerpc-utils-devel/c/wrwVzAAnRlI/m/5KJSoqP4BAAJ
>
> Nysal Jan K.A (2):
>   cpu/SMT: Enable SMT only if a core is online
>   powerpc/topology: Check if a core is online
>
>  arch/powerpc/include/asm/topology.h | 13 +++++++++++++
>  kernel/cpu.c                        | 12 +++++++++++-
>  2 files changed, 24 insertions(+), 1 deletion(-)
>
>
> base-commit: c760b3725e52403dc1b28644fb09c47a83cacea6
> -- 
> 2.35.3