diff mbox series

[5/9] cpu/SMT: Create topology_smt_thread_allowed()

Message ID 20230524155630.794584-5-mpe@ellerman.id.au (mailing list archive)
State Superseded
Headers show
Series [1/9] cpu/SMT: Move SMT prototypes into cpu_smt.h | expand

Commit Message

Michael Ellerman May 24, 2023, 3:56 p.m. UTC 
A subsequent patch will enable partial SMT states, ie. when not all SMT
threads are brought online.

To support that, add an arch helper which checks whether a given CPU is
allowed to be brought online depending on how many SMT threads are
currently enabled.

Call the helper from cpu_smt_enable(), and cpu_smt_allowed() when SMT is
enabled, to check if the particular thread should be onlined. Notably,
also call it from cpu_smt_disable() if CPU_SMT_ENABLED, to allow
offlining some threads to move from a higher to lower number of threads
online.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/x86/include/asm/topology.h |  2 ++
 arch/x86/kernel/smpboot.c       | 15 +++++++++++++++
 kernel/cpu.c                    | 10 +++++++++-
 3 files changed, 26 insertions(+), 1 deletion(-)

Comments

Thomas Gleixner June 10, 2023, 10:35 p.m. UTC | #1 
On Thu, May 25 2023 at 01:56, Michael Ellerman wrote:
> A subsequent patch will enable partial SMT states, ie. when not all SMT
> threads are brought online.

Nitpick. I stumbled over this 'subsequent patch' theme a couple of times
now because it's very similar to the 'This patch does' phrase.

Just explain what you want to achieve at the end.

>  #else
>  #define topology_max_packages()			(1)
>  static inline int
> @@ -159,6 +160,7 @@ static inline int topology_max_smt_threads(void) { return 1; }
>  static inline bool topology_is_primary_thread(unsigned int cpu) { return true; }
>  static inline bool topology_smt_supported(void) { return false; }
>  static inline bool topology_smt_threads_supported(unsigned int threads) { return false; }
> +static inline bool topology_smt_thread_allowed(unsigned int cpu) { return false; }

Not all these functions need a !SMP stub. Think about the context in
which they are called. There is probably precedence for pointless ones,
but that does not make an argument to add more.

> +/**
> + * topology_smt_thread_allowed - When enabling SMT check whether this particular
> + *				 CPU thread is allowed to be brought online.
> + * @cpu:	CPU to check
> + */
> +bool topology_smt_thread_allowed(unsigned int cpu)
> +{
> +	/*
> +	 * No extra logic s required here to support different thread values
> +	 * because threads will always == 1 or smp_num_siblings because of
> +	 * topology_smt_threads_supported().
> +	 */
> +	return true;
> +}
> +

As x86 only supoorts the on/off model there is no need for this function
if you pick up the CONFIG_SMT_NUM_THREADS_DYNAMIC idea.

You still need something like that for your PPC use case, but that
reduces the overall impact, right?

Thanks,

        tglx
diff mbox series

Patch

diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index 197ec2589f5d..c6245ea6e589 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -145,6 +145,7 @@  int topology_phys_to_logical_die(unsigned int die, unsigned int cpu);
 bool topology_is_primary_thread(unsigned int cpu);
 bool topology_smt_supported(void);
 bool topology_smt_threads_supported(unsigned int threads);
+bool topology_smt_thread_allowed(unsigned int cpu);
 #else
 #define topology_max_packages()			(1)
 static inline int
@@ -159,6 +160,7 @@  static inline int topology_max_smt_threads(void) { return 1; }
 static inline bool topology_is_primary_thread(unsigned int cpu) { return true; }
 static inline bool topology_smt_supported(void) { return false; }
 static inline bool topology_smt_threads_supported(unsigned int threads) { return false; }
+static inline bool topology_smt_thread_allowed(unsigned int cpu) { return false; }
 #endif
 
 static inline void arch_fix_phys_package_id(int num, u32 slot)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index c7ba62beae3e..244b4df40600 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -298,6 +298,21 @@  bool topology_smt_threads_supported(unsigned int threads)
 	return threads == 1 || threads == smp_num_siblings;
 }
 
+/**
+ * topology_smt_thread_allowed - When enabling SMT check whether this particular
+ *				 CPU thread is allowed to be brought online.
+ * @cpu:	CPU to check
+ */
+bool topology_smt_thread_allowed(unsigned int cpu)
+{
+	/*
+	 * No extra logic s required here to support different thread values
+	 * because threads will always == 1 or smp_num_siblings because of
+	 * topology_smt_threads_supported().
+	 */
+	return true;
+}
+
 /**
  * topology_phys_to_logical_pkg - Map a physical package id to a logical
  *
diff --git a/kernel/cpu.c b/kernel/cpu.c
index a08dd8f93675..72b9a03a4fef 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -458,7 +458,7 @@  early_param("nosmt", smt_cmdline_disable);
 
 static inline bool cpu_smt_allowed(unsigned int cpu)
 {
-	if (cpu_smt_control == CPU_SMT_ENABLED)
+	if (cpu_smt_control == CPU_SMT_ENABLED && topology_smt_thread_allowed(cpu))
 		return true;
 
 	if (topology_is_primary_thread(cpu))
@@ -2273,6 +2273,12 @@  int cpuhp_smt_disable(enum cpuhp_smt_control ctrlval)
 	for_each_online_cpu(cpu) {
 		if (topology_is_primary_thread(cpu))
 			continue;
+		/*
+		 * Disable can be called with CPU_SMT_ENABLED when changing
+		 * from a higher to lower number of SMT threads per core.
+		 */
+		if (ctrlval == CPU_SMT_ENABLED && topology_smt_thread_allowed(cpu))
+			continue;
 		ret = cpu_down_maps_locked(cpu, CPUHP_OFFLINE);
 		if (ret)
 			break;
@@ -2307,6 +2313,8 @@  int cpuhp_smt_enable(void)
 		/* Skip online CPUs and CPUs on offline nodes */
 		if (cpu_online(cpu) || !node_online(cpu_to_node(cpu)))
 			continue;
+		if (!topology_smt_thread_allowed(cpu))
+			continue;
 		ret = _cpu_up(cpu, 0, CPUHP_ONLINE);
 		if (ret)
 			break;