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
