Re: [PATCH] powerpc/pseries: Fix numa FORM2 parsing fallback code
From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2021-11-08 05:21:35
Nicholas Piggin [off-list ref] writes:
quoted hunk ↗ jump to hunk
In case the FORM2 distance table from firmware is not the expected size, there is fallback code that just populates the lookup table as local vs remote. However it then continues on to use the distance table. Fix. Cc: Aneesh Kumar K.V <redacted> Fixes: 1c6b5a7e7405 ("powerpc/pseries: Add support for FORM2 associativity") Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/mm/numa.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-)diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c index 6f14c8fb6359..0789cde7f658 100644 --- a/arch/powerpc/mm/numa.c +++ b/arch/powerpc/mm/numa.c@@ -380,6 +380,7 @@ static void initialize_form2_numa_distance_lookup_table(void) const __be32 *numa_lookup_index; int numa_dist_table_length; int max_numa_index, distance_index; + bool good = true;
numa_dist_table is a pointer, so couldn't we just set it to NULL if the info it's pointing at is invalid?
quoted hunk ↗ jump to hunk
if (firmware_has_feature(FW_FEATURE_OPAL)) root = of_find_node_by_path("/ibm,opal");@@ -407,30 +408,26 @@ static void initialize_form2_numa_distance_lookup_table(void) if (numa_dist_table_length != max_numa_index * max_numa_index) { WARN(1, "Wrong NUMA distance information\n"); - /* consider everybody else just remote. */ - for (i = 0; i < max_numa_index; i++) { - for (j = 0; j < max_numa_index; j++) { - int nodeA = numa_id_index_table[i]; - int nodeB = numa_id_index_table[j]; - - if (nodeA == nodeB) - numa_distance_table[nodeA][nodeB] = LOCAL_DISTANCE; - else - numa_distance_table[nodeA][nodeB] = REMOTE_DISTANCE; - } - } + good = false;
ie. numa_dist_table = NULL;
quoted hunk ↗ jump to hunk
} - distance_index = 0; for (i = 0; i < max_numa_index; i++) { for (j = 0; j < max_numa_index; j++) { int nodeA = numa_id_index_table[i]; int nodeB = numa_id_index_table[j]; - - numa_distance_table[nodeA][nodeB] = numa_dist_table[distance_index++]; - pr_debug("dist[%d][%d]=%d ", nodeA, nodeB, numa_distance_table[nodeA][nodeB]); + int dist; + + if (good)
if (numa_dist_table)
quoted hunk ↗ jump to hunk
+ dist = numa_dist_table[distance_index++]; + else if (nodeA == nodeB) + dist = LOCAL_DISTANCE; + else + dist = REMOTE_DISTANCE; + numa_distance_table[nodeA][nodeB] = dist; + pr_debug("dist[%d][%d]=%d ", nodeA, nodeB, dist); } } + of_node_put(root); }
But maybe before we do that we can rename it, because it is really easy to confuse numa_dist_table and numa_distance_table if you don't look closely. cheers