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

----- Original Message ----- > 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/9937878cce2fc049283d833685c... >>> >>> 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..."). Thanks Masa, this additional patch tests OK. Note that I removed a couple unused variables, and adjusted the display justification slightly. Queued for crash-7.2.8: https://github.com/crash-utility/crash/commit/1d2bc0c65792d15f94ebfd97c22...

Dave > > From: Masayoshi Mizuma <m.mizuma(a)jp.fujitsu.com&gt; > 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 >