diff mbox series

[v6,5/7] ACPI/NUMA: Return memblk modification state from numa_fill_memblks()

Message ID 20240430092200.2335887-6-rrichter@amd.com
State New
Headers show
Series SRAT/CEDT fixes and updates | expand

Commit Message

Robert Richter April 30, 2024, 9:21 a.m. UTC 
When registering a memory range a possibly overlapping memory block
will be extended instead of creating a new one. If both ranges exactly
overlap, the blocks remain unchanged and are just reused. The
information if a memblock was extended is useful for diagnostics.

Change return code of numa_fill_memblks() to also report if memblocks
have been modified.

Link: https://lore.kernel.org/all/ZiqnbD0CB9WUL1zu@aschofie-mobl2/T/#u
Cc: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Robert Richter <rrichter@amd.com>
---
 arch/x86/mm/numa.c       | 33 ++++++++++++++++++---------------
 drivers/acpi/numa/srat.c |  5 +++--
 2 files changed, 21 insertions(+), 17 deletions(-)

Comments

Robert Richter May 2, 2024, 12:45 p.m. UTC | #1 
On 30.04.24 10:05:39, Dan Williams wrote:
> Robert Richter wrote:
> > When registering a memory range a possibly overlapping memory block
> > will be extended instead of creating a new one. If both ranges exactly
> > overlap, the blocks remain unchanged and are just reused. The
> > information if a memblock was extended is useful for diagnostics.
> 
> What diagnostic flow is this useful for?
> 
> I feel like this post-hoc debug prints for problems we never expect to
> have again, or is there an enduring need for this?

As we are already targeting -rc7, I am going to drop the printout
patches to not block the first patches in this series and will post
them again after the next merge window.

The SRAT logging is very useful to get a picture of the firmware
provided mem blocks. For NUMA debugging these are very helpful, esp.
when working with unmodified generic or distribution kernels. As CFMWS
entries add additional similar information as SRAT, that information
is no longer complete on systems with a CEDT.

The patches just enable the same level of logging for CFMWS as for
SRAT which just adds a single line (info level) per CFMWS entry (in
total just a few). The table printouts are at debug level already. Of
course, there are always other ways to get this information from, but
a grep of dmesg makes things very easy to check things.

Thanks,

-Robert
diff mbox series

Patch

diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
index ce84ba86e69e..e34e96d57656 100644
--- a/arch/x86/mm/numa.c
+++ b/arch/x86/mm/numa.c
@@ -950,15 +950,16 @@  static struct numa_memblk *numa_memblk_list[NR_NODE_MEMBLKS] __initdata;
  * address range @start-@end
  *
  * RETURNS:
- * 0		  : Success
- * NUMA_NO_MEMBLK : No memblks exist in address range @start-@end
+ * NUMA_NO_MEMBLK if no memblks exist in address range @start-@end,
+ * zero on success without blocks modified and non-zero positive
+ * values on success with blocks modified.
  */
 
 int __init numa_fill_memblks(u64 start, u64 end)
 {
 	struct numa_memblk **blk = &numa_memblk_list[0];
 	struct numa_meminfo *mi = &numa_meminfo;
-	int count = 0;
+	int count = 0, modified = 0;
 	u64 prev_end;
 
 	/*
@@ -981,25 +982,27 @@  int __init numa_fill_memblks(u64 start, u64 end)
 	/* Sort the list of pointers in memblk->start order */
 	sort(&blk[0], count, sizeof(blk[0]), cmp_memblk, NULL);
 
-	/* Make sure the first/last memblks include start/end */
-	blk[0]->start = min(blk[0]->start, start);
-	blk[count - 1]->end = max(blk[count - 1]->end, end);
-
 	/*
 	 * Fill any gaps by tracking the previous memblks
 	 * end address and backfilling to it if needed.
 	 */
-	prev_end = blk[0]->end;
-	for (int i = 1; i < count; i++) {
+	prev_end = start;
+	for (int i = 0; i < count; i++) {
 		struct numa_memblk *curr = blk[i];
 
-		if (prev_end >= curr->start) {
-			if (prev_end < curr->end)
-				prev_end = curr->end;
-		} else {
+		if (prev_end < curr->start) {
 			curr->start = prev_end;
-			prev_end = curr->end;
+			modified = 1;
 		}
+
+		if (prev_end < curr->end)
+			prev_end = curr->end;
 	}
-	return 0;
+
+	if (blk[count - 1]->end < end) {
+		blk[count - 1]->end = end;
+		modified = 1;
+	}
+
+	return modified;
 }
diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c
index e3f26e71637a..76b39a6d3aef 100644
--- a/drivers/acpi/numa/srat.c
+++ b/drivers/acpi/numa/srat.c
@@ -326,7 +326,7 @@  static int __init acpi_parse_cfmws(union acpi_subtable_headers *header,
 	struct acpi_cedt_cfmws *cfmws;
 	int *fake_pxm = arg;
 	u64 start, end;
-	int node;
+	int node, modified;
 
 	cfmws = (struct acpi_cedt_cfmws *)header;
 	start = cfmws->base_hpa;
@@ -338,7 +338,8 @@  static int __init acpi_parse_cfmws(union acpi_subtable_headers *header,
 	 * found for any portion of the window to cover the entire
 	 * window.
 	 */
-	if (!numa_fill_memblks(start, end))
+	modified = numa_fill_memblks(start, end);
+	if (modified != NUMA_NO_MEMBLK)
 		return 0;
 
 	/* No SRAT description. Create a new node. */