[Crash-utility] [PATCH] Fix for "kmem -n" option on Linux 5.4-rc1

[Masayoshi Mizuma]

Hi Dave,

On Fri, Oct 18, 2019 at 11:12:22AM -0400, Dave Anderson wrote:
> 
> ----- Original Message -----
> > 
> > 
> > Thanks Masa -- queued for crash-7.2.8:
> > 
> >   https://github.com/crash-utility/crash/commit/9937878cce2fc049283d833685cb939caca462ca
> > 
> > Dave
> 
> Hi Masa,
> 
> I spoke too soon -- I originally tested this on an x86_64 machine, but I now see that
> it fails on the other architectures.  That's because of your newly-introduced dependence
> upon "memory_block_size_probed", which is declared in "arch/x86/mm/init_64.c".
> 
> I won't revert the commit, but can you look at fixing this for the other architectures?
> If that's not possible, please restrict the functionality to x86_64.

Thank you for your review!
Could you check the following patch? That's the incremental patch for 
commit 9937878 ("Fix for the "kmem -n" option on Linux-5.4-rc1...").

From: Masayoshi Mizuma <m.mizuma at jp.fujitsu.com>
Date: Fri, 18 Oct 2019 17:45:04 -0400
Subject: [PATCH] Additional fix for "kmem -n" option on Linux 5.4-rc1

commit 9937878 ("Fix for the "kmem -n" option on Linux-5.4-rc1...")
works on x86, however, it fails on aarch64 or so because
memory_block_size_probed symbol is in x86 only.

Show the start phyical address in case the kernel doesn't have
memory_block_size_probed and memory_block.end_section_nr.
---
 memory.c | 48 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/memory.c b/memory.c
index 0a79838..0a8747b 100644
--- a/memory.c
+++ b/memory.c
@@ -17401,6 +17401,23 @@ fill_memory_block_name(ulong memblock, char *name)
 	read_string(value, name, BUFSIZE-1);
 }

+static void
+fill_memory_block_parange(ulong saddr, ulong eaddr, char *parange)
+{
+	char buf1[BUFSIZE];
+	char buf2[BUFSIZE];
+
+	memset(parange, 0, sizeof(*parange) * BUFSIZE);
+
+	if (eaddr == ULLONG_MAX)
+		sprintf(parange, "%s",
+			mkstring(buf1, PADDR_PRLEN*2 + 3, LJUST|LONG_HEX, MKSTR(saddr)));
+	else
+		sprintf(parange, "%s - %s",
+			mkstring(buf1, PADDR_PRLEN, LJUST|LONG_HEX, MKSTR(saddr)),
+			mkstring(buf2, PADDR_PRLEN, LJUST|LONG_HEX, MKSTR(eaddr)));
+}
+
 static void
 fill_memory_block_srange(ulong start_sec, char *srange)
 {
@@ -17413,9 +17430,10 @@ static void
 print_memory_block(ulong memory_block)
 {
 	ulong start_sec, end_sec, nid;
-	ulong memblock_size, mbs, start_addr, end_addr;
+	ulong memblock_size, mbs, start_addr, end_addr = ULLONG_MAX;
 	char statebuf[BUFSIZE];
 	char srangebuf[BUFSIZE];
+	char parangebuf[BUFSIZE];
 	char name[BUFSIZE];
 	char buf1[BUFSIZE];
 	char buf2[BUFSIZE];
@@ -17437,7 +17455,7 @@ print_memory_block(ulong memory_block)
 			&mbs, sizeof(ulong), "memory_block_size_probed",
 			FAULT_ON_ERROR);
 		end_addr = start_addr + mbs - 1;
-	} else {
+	} else if (MEMBER_EXISTS("memory_block", "end_section_nr")) {
 	        readmem(memory_block + OFFSET(memory_block_end_section_nr), KVADDR,
 			&end_sec, sizeof(void *), "memory_block end_section_nr",
 			FAULT_ON_ERROR);
@@ -17446,33 +17464,28 @@ print_memory_block(ulong memory_block)

 	fill_memory_block_state(memory_block, statebuf);
 	fill_memory_block_name(memory_block, name);
+	fill_memory_block_parange(start_addr, end_addr, parangebuf);
 	fill_memory_block_srange(start_sec, srangebuf);

 	if (MEMBER_EXISTS("memory_block", "nid")) {
 		readmem(memory_block + OFFSET(memory_block_nid), KVADDR, &nid,
 			sizeof(void *), "memory_block nid", FAULT_ON_ERROR);
-		fprintf(fp, " %s %s %s - %s %s %s %s\n",
+		fprintf(fp, " %s %s %s %s %s %s\n",
 			mkstring(buf1, VADDR_PRLEN, LJUST|LONG_HEX,
 			MKSTR(memory_block)),
 			mkstring(buf2, 12, CENTER, name),
-			mkstring(buf3, PADDR_PRLEN, RJUST|LONG_HEX,
-			MKSTR(start_addr)),
-			mkstring(buf4, PADDR_PRLEN, LJUST|LONG_HEX,
-			MKSTR(end_addr)),
+			parangebuf,
 			mkstring(buf5, strlen("NODE"), CENTER|LONG_DEC,
 			MKSTR(nid)),
 			mkstring(buf6, strlen("CANCEL_OFFLINE"), LJUST,
 			statebuf),
 			mkstring(buf7, 12, LJUST, srangebuf));
 	} else
-		fprintf(fp, " %s %s %s - %s %s %s\n",
+		fprintf(fp, " %s %s %s %s %s\n",
 			mkstring(buf1, VADDR_PRLEN, LJUST|LONG_HEX,
 			MKSTR(memory_block)),
 			mkstring(buf2, 10, CENTER, name),
-			mkstring(buf3, PADDR_PRLEN, RJUST|LONG_HEX,
-			MKSTR(start_addr)),
-			mkstring(buf4, PADDR_PRLEN, LJUST|LONG_HEX,
-			MKSTR(end_addr)),
+			parangebuf,
 			mkstring(buf5, strlen("CANCEL_OFFLINE"), LJUST,
 			statebuf),
 			mkstring(buf6, 12, LJUST, srangebuf));
@@ -17537,6 +17550,7 @@ dump_memory_blocks(int initialize)
 	int klistcnt, i;
 	struct list_data list_data;
 	char mb_hdr[BUFSIZE];
+	char paddr_hdr[BUFSIZE];
 	char buf1[BUFSIZE];
 	char buf2[BUFSIZE];
 	char buf3[BUFSIZE];
@@ -17553,11 +17567,17 @@ dump_memory_blocks(int initialize)

 	init_memory_block(&list_data, &klistcnt, &klistbuf);

+	if ((symbol_exists("memory_block_size_probed")) ||
+	    (MEMBER_EXISTS("memory_block", "end_section_nr")))
+		sprintf(paddr_hdr, "%s", "PHYSICAL RANGE");
+	else
+		sprintf(paddr_hdr, "%s", "PHYSICAL START");
+
 	if (MEMBER_EXISTS("memory_block", "nid"))
 		sprintf(mb_hdr, "\n%s %s %s     %s %s %s\n",
 			mkstring(buf1, VADDR_PRLEN, CENTER|LJUST, "MEM_BLOCK"),
 			mkstring(buf2, 10, CENTER, "NAME"),
-			mkstring(buf3, PADDR_PRLEN*2 + 2, CENTER, "PHYSICAL RANGE"),
+			mkstring(buf3, PADDR_PRLEN*2 + 2, CENTER, paddr_hdr),
 			mkstring(buf4, strlen("NODE"), CENTER, "NODE"),
 			mkstring(buf5, strlen("CANCEL_OFFLINE"), LJUST, "STATE"),
 			mkstring(buf6, 12, LJUST, "START_SECTION_NO"));
@@ -17565,7 +17585,7 @@ dump_memory_blocks(int initialize)
 		sprintf(mb_hdr, "\n%s %s %s     %s %s\n",
 			mkstring(buf1, VADDR_PRLEN, CENTER|LJUST, "MEM_BLOCK"),
 			mkstring(buf2, 10, CENTER, "NAME"),
-			mkstring(buf3, PADDR_PRLEN*2, CENTER, "PHYSICAL RANGE"),
+			mkstring(buf3, PADDR_PRLEN*2, CENTER, paddr_hdr),
 			mkstring(buf4, strlen("CANCEL_OFFLINE"), LJUST, "STATE"),
 			mkstring(buf5, 12, LJUST, "START_SECTION_NO"));
 	fprintf(fp, "%s", mb_hdr);
--
2.18.1