[PATCH v4 0/4] arch_topology: Bunch of error patch related fixes

[PATCH v4 0/4] arch_topology: Bunch of error patch related fixes

[Sudeep Holla]

Hi Greg,

Hopefully no more issues get reported by sashiko. It all started with
one issue[1] one patch and ended with 4 different issues so far. All of
them are related to error path handling.

v3->v4:
 - Validate all possible CPUs before normalizing capacities
 - v3: https://lore.kernel.org/all/20261109173125.1734847-1-sudeep.holla@kernel.org

v2->v3:
- Added fixes to couple of more issues pointed by Sashiko
- v2: https://lore.kernel.org/all/20261106164408.158337-1-sudeep.holla@kernel.org

v1->v2:
- Reset max_smt_thread_num to 1 on failure similar to parse_socket()
- Grouped below 2 in one series and posted them together
- v1: https://lore.kernel.org/all/20261104153413.2786066-1-sudeep.holla@kernel.org
- v1: https://lore.kernel.org/all/20261103093421.1283949-1-sudeep.holla@kernel.org

Regards,
Sudeep

[1] https://sashiko.dev/#/bug/linux-73adead8-21f3-470d-938c-b1f07a269a72

Sudeep Holla (4):
  arch_topology: Reset SMT state when socket parsing fails
  arch_topology: Free ACPI SMT entries on lookup failure
  arch_topology: Reset SMT state when topology is unavailable
  arch_topology: Avoid zero CPU capacity on parse failure

 drivers/base/arch_topology.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

-- 
2.43.0

[PATCH v4 1/4] arch_topology: Reset SMT state when socket parsing fails

[Sudeep Holla]

If a later socket's cluster fails device tree parsing, parse_socket()
returns immediately. Cores parsed earlier can have raised
max_smt_thread_num, but this return skips the reset to 1 and the
cpu_smt_set_num_threads() notification. The framework is left with an
inconsistent SMT state after topology parsing fails.

Break out of the socket loop on parse_cluster() failure so the existing
error handling resets the thread count and notifies the SMT framework
before returning the error.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-73adead8-21f3-470d-938c-b1f07a269a72
Fixes: 5deb9c789ae4 ("arch_topology: Support SMT control for OF based system")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/base/arch_topology.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index e51fc9f5e976..8ed29885dccf 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -631,7 +631,7 @@ static int __init parse_socket(struct device_node *socket)
 		has_socket = true;
 		ret = parse_cluster(c, package_id, -1, 0);
 		if (ret != 0)
-			return ret;
+			break;
 
 		package_id++;
 	} while (1);
-- 
2.43.0

[PATCH v4 2/4] arch_topology: Free ACPI SMT entries on lookup failure

[Sudeep Holla]

A later CPU can fail find_acpi_cpu_topology() after earlier threaded
CPUs have populated hetero_cpu. Returning immediately leaks the stored
cpu_smt_info objects and the XArray nodes.

Send the error through the existing cleanup and retain its return code.
Only update the SMT thread count after a successful parse.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-f41acdf3-9470-4de9-b497-d5de16c8e7e8
Fixes: e6b18ebfaf63 ("arm64: topology: Support SMT control on ACPI based system")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/base/arch_topology.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 8ed29885dccf..eea4c4257d08 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -858,7 +858,7 @@ __weak int __init parse_acpi_topology(void)
 	struct cpu_smt_info *entry;
 	struct xarray hetero_cpu;
 	unsigned long hetero_id;
-	int cpu, topology_id;
+	int cpu, topology_id, ret = 0;
 
 	if (acpi_disabled)
 		return 0;
@@ -867,8 +867,10 @@ __weak int __init parse_acpi_topology(void)
 
 	for_each_possible_cpu(cpu) {
 		topology_id = find_acpi_cpu_topology(cpu, 0);
-		if (topology_id < 0)
-			return topology_id;
+		if (topology_id < 0) {
+			ret = topology_id;
+			goto out;
+		}
 
 		if (acpi_cpu_is_threaded(cpu)) {
 			cpu_topology[cpu].thread_id = topology_id;
@@ -912,6 +914,7 @@ __weak int __init parse_acpi_topology(void)
 		cpu_topology[cpu].package_id = topology_id;
 	}
 
+out:
 	/*
 	 * This is a short loop since the number of XArray elements is the
 	 * number of heterogeneous CPU clusters. On a homogeneous system
@@ -923,9 +926,13 @@ __weak int __init parse_acpi_topology(void)
 		kfree(entry);
 	}
 
+	/* Reset max_smt_thread_num to 1 on failure like in parse_socket() */
+	if (ret)
+		max_smt_thread_num = 1;
+
 	cpu_smt_set_num_threads(max_smt_thread_num, max_smt_thread_num);
 	xa_destroy(&hetero_cpu);
-	return 0;
+	return ret;
 }
 
 void __init init_cpu_topology(void)
-- 
2.43.0

[PATCH v4 3/4] arch_topology: Reset SMT state when topology is unavailable

[Sudeep Holla]

If DT has no CPU information or cpu-map, parsing succeeds without
notifying the SMT framework. A later topology validation failure can
discard parsed data while leaving the SMT thread count set by
parse_socket().

Reset the count to one on these paths so the SMT framework matches
the topology that remains available.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-486a8656-d962-4966-8d90-bbb2ce276147
Fixes: 5deb9c789ae4 ("arch_topology: Support SMT control for OF based system")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/base/arch_topology.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index eea4c4257d08..c439fc91270d 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -662,6 +662,7 @@ static int __init parse_dt_topology(void)
 
 	if (!cn) {
 		pr_err("No CPU information found in DT\n");
+		cpu_smt_set_num_threads(1, 1);
 		return 0;
 	}
 
@@ -672,8 +673,10 @@ static int __init parse_dt_topology(void)
 	struct device_node *map __free(device_node) =
 		of_get_child_by_name(cn, "cpu-map");
 
-	if (!map)
+	if (!map) {
+		cpu_smt_set_num_threads(1, 1);
 		return ret;
+	}
 
 	ret = parse_socket(map);
 	if (ret != 0)
@@ -951,6 +954,7 @@ void __init init_cpu_topology(void)
 		 * arch-specific early cache level detection a chance to run.
 		 */
 		reset_cpu_topology();
+		cpu_smt_set_num_threads(1, 1);
 	}
 
 	for_each_possible_cpu(cpu) {
-- 
2.43.0

[PATCH v4 4/4] arch_topology: Avoid zero CPU capacity on parse failure

[Sudeep Holla]

When a possible CPU is missing from cpu-map, parse_dt_topology()
normalizes the partially populated raw_capacity array before checking
that all possible CPUs are represented. This leaves the missing CPU with
a capacity of 0.

Validate all possible CPUs before normalizing capacities so parse
failures do not write partial values to cpu_scale. Free raw_capacity on
the failure path to discard partial data and avoid a leak.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-1a2a8094-971d-40cb-b0cf-2e6d9f26c549
Fixes: 60c1b220d8bc ("cpu-topology: Move cpu topology code to common code.")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
 drivers/base/arch_topology.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index c439fc91270d..e868b7de88fa 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -682,8 +682,6 @@ static int __init parse_dt_topology(void)
 	if (ret != 0)
 		return ret;
 
-	topology_normalize_cpu_scale();
-
 	/*
 	 * Check that all cores are in the topology; the SMP code will
 	 * only mark cores described in the DT as possible.
@@ -693,6 +691,8 @@ static int __init parse_dt_topology(void)
 			return -EINVAL;
 		}
 
+	topology_normalize_cpu_scale();
+
 	return ret;
 }
 #endif
@@ -954,6 +954,7 @@ void __init init_cpu_topology(void)
 		 * arch-specific early cache level detection a chance to run.
 		 */
 		reset_cpu_topology();
+		free_raw_capacity();
 		cpu_smt_set_num_threads(1, 1);
 	}
 
-- 
2.43.0