Hi,
On 8/2/19 8:44 AM, Robert Richter wrote:
On 31.07.19 22:46:34, Jeremy Linton wrote:
@@ -358,6 +356,10 @@ static int __init parse_acpi_topology(void)
if (topology_id < 0)
return topology_id;
+ is_threaded = acpi_pptt_cpu_is_thread(cpu);
+ if (is_threaded < 0)
+ is_threaded = read_cpuid_mpidr() & MPIDR_MT_BITMASK;
+
I think the return code handling is error-prone, as in the kernel such
functions are typically used like:
if (something_is_thread) { ... }
I don't really understand why this keeps getting repeated. The negative
error code return is used by huge swaths of the kernel API. A couple
lines up the exact same paradigm is used in get_cpu_for_node() and a few
other places.
I see this is due to acpi and arch code separation so we cannot simply
move the fallback to pptt code.
Right, the PPTT->arch data structure translation is arch specific.
During the initial PPTT drop a lot of discussion when into how arm64 was
doing that translation, as well as the corresponding translation to the
core scheduler/etc.
So maybe we have a static function cpu_is_thread() in this file that
handles all the logic and directly use check_acpi_cpu_flag() from
there. However, code may change here in case of a rework as I
suggested in patch #1. In both cases the acpi api is more straight
then.
I'm ok with that, it effectively only moves those three lines to a
standalone single call-site function. To be clear, that isn't a generic
routine for anyone to call. Functions that need to know if the core is a
threaded should be checking the topology thread_id directly rather than
re-coding the acpi/dt/mpidr logic which populates it.
-Robert
if (is_threaded) {
cpu_topology[cpu].thread_id = topology_id;
topology_id = find_acpi_cpu_topology(cpu, 1);
