Re: [Crash-utility] patch to add vmss memory regions support

Dyno, I haven't really begun to review the patch yet, but when I was checking read_vmware_vmss() for the incorrect PAGE_SHIFT usage, I notice that this change is also incorrect: - if (fread(bufptr, 1 , cnt, vmss.dfp) != cnt) - return READ_ERROR; + cnt = fread(bufptr, 1, cnt, vmss.dfp); return cnt; } The readmem() function keys on READ_ERROR, SEEK_ERROR and PAGE_EXCLUDED, and if none of those are returned, it presumes the read was successful. See the READMEM() macro/call in the readmem() function. Dave ----- Original Message ----- > > ----- Original Message ----- >> Dave, >> patch attached. thanks. >> rgds, >> Dyno > > OK thanks, but now can you re-work the patch so that crash builds > cleanly with "make warn"? Here on an x86_64 with gcc-4.7.2: > > $ make warn > TARGET: X86_64 > CRASH: 7.1.1rc12 > GDB: 7.6 > > cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6 build_data.c -Wall -O2 > -Wstrict-prototypes -Wmissing-prototypes -fstack-protector > -Wformat-security > cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6 vmware_vmss.c -Wall -O2 > -Wstrict-prototypes -Wmissing-prototypes -fstack-protector > -Wformat-security > vmware_vmss.c: In function ‘vmware_vmss_init’: > vmware_vmss.c:218:7: warning: dereferencing type-punned pointer will break > strict-aliasing rules [-Wstrict-aliasing] > vmware_vmss.c:222:7: warning: dereferencing type-punned pointer will break > strict-aliasing rules [-Wstrict-aliasing] > vmware_vmss.c:225:7: warning: dereferencing type-punned pointer will break > strict-aliasing rules [-Wstrict-aliasing] > vmware_vmss.c:228:7: warning: dereferencing type-punned pointer will break > strict-aliasing rules [-Wstrict-aliasing] > vmware_vmss.c:231:7: warning: dereferencing type-punned pointer will break > strict-aliasing rules [-Wstrict-aliasing] > ... > > And then for a quick sanity check, I noted that the patch won't even > compile at all on an ARM64 machine: > > $ make warn > TARGET: ARM64 > CRASH: 7.1.0 > GDB: 7.6 > > cc -c -g -DARM64 -DGDB_7_6 build_data.c -Wall -O2 -Wstrict-prototypes > -Wmissing-prototypes -fstack-protector -Wformat-security > cc -c -g -DARM64 -DGDB_7_6 netdump.c -Wall -O2 -Wstrict-prototypes > -Wmissing-prototypes -fstack-protector -Wformat-security > cc -c -g -DARM64 -DGDB_7_6 vmware_vmss.c -Wall -O2 -Wstrict-prototypes > -Wmissing-prototypes -fstack-protector -Wformat-security > vmware_vmss.c: In function ‘vmware_vmss_init’: > vmware_vmss.c:218:7: warning: dereferencing type-punned pointer will break > strict-aliasing rules [-Wstrict-aliasing] > vmss.regionscount = *(uint32_t*)val; > ^ > vmware_vmss.c:222:7: warning: dereferencing type-punned pointer will break > strict-aliasing rules [-Wstrict-aliasing] > vmss.regions[idx[0]].startpagenum = *(uint32_t*)val; > ^ > vmware_vmss.c:225:7: warning: dereferencing type-punned pointer will break > strict-aliasing rules [-Wstrict-aliasing] > vmss.regions[idx[0]].startppn = *(uint32_t*)val; > ^ > vmware_vmss.c:228:7: warning: dereferencing type-punned pointer will break > strict-aliasing rules [-Wstrict-aliasing] > vmss.regions[idx[0]].size = *(uint32_t*)val; > ^ > vmware_vmss.c:231:7: warning: dereferencing type-punned pointer will break > strict-aliasing rules [-Wstrict-aliasing] > vmss.alignmask = *(uint32_t*)val; > ^ > vmware_vmss.c: In function ‘vmware_vmss_page_size’: > vmware_vmss.c:277:9: error: ‘PAGE_SIZE’ undeclared (first use in this > function) > return PAGE_SIZE; > ^ > vmware_vmss.c:277:9: note: each undeclared identifier is reported only once > for each function it appears in > vmware_vmss.c: In function ‘read_vmware_vmss’: > vmware_vmss.c:287:37: error: ‘PAGE_SHIFT’ undeclared (first use in this > function) > uint32_t ppn = (uint32_t) (pos >> PAGE_SHIFT); > ^ > vmware_vmss.c: In function ‘vmware_vmss_page_size’: > vmware_vmss.c:278:1: warning: control reaches end of non-void function > [-Wreturn-type] > } > ^ > make[4]: *** [vmware_vmss.o] Error 1 > make[3]: *** [gdb] Error 2 > make[2]: *** [rebuild] Error 2 > make[1]: *** [gdb_merge] Error 2 > make: *** [warn] Error 2 > $ > > And that's because PAGE_SIZE is only #define'd in defs.h for x86_64, and > and PAGE_SHIFT is only #define'd in defs.h for x86_64 and ppc32. > > There are machdep->pagesize and machdep->pageshift that get assigned for > each architecture. So you could use machdep->pageshift in your patch > to read_vmware_vmss(). > > > But then there's this: > > uint vmware_vmss_page_size(void) > { > - return 4096; > + return PAGE_SIZE; > } > > which is pretty much the wrong way to do it both before and after. > I'm not sure how you want to handle it. > > Dave > > > > > > > >> On 3/24/15 10:00 AM, Dyno Hongjun Fu wrote: >>> ---------- Forwarded message ---------- >>> From: Dave Anderson <anderson(a)redhat.com&gt; >>> Date: Tue, Mar 24, 2015 at 7:17 AM >>> Subject: Re: [Crash-utility] patch to add vmss memory regions support >>> To: "Discussion list for crash utility usage, maintenance and >>> development" <crash-utility(a)redhat.com&gt; >>> >>> >>> >>> Dyno, >>> >>> Can you re-post this path so that it applies cleanly to the >>> current git repository? This is what happens: >>> >>> $ git clone >>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_crash-2Du... >>> Cloning into 'crash'... >>> remote: Counting objects: 811, done. >>> remote: Total 811 (delta 0), reused 0 (delta 0), pack-reused 811 >>> Receiving objects: 100% (811/811), 1.87 MiB | 1.04 MiB/s, done. >>> Resolving deltas: 100% (532/532), done. >>> $ cd crash >>> $ patch -p1 < ../vmware_memory_region_support.patch >>> patching file vmware_vmss.c >>> Reversed (or previously applied) patch detected! Assume -R? [n] n >>> Apply anyway? [n] y >>> Hunk #1 succeeded at 42 with fuzz 2 (offset 2 lines). >>> Hunk #2 succeeded at 141 (offset 2 lines). >>> Hunk #3 succeeded at 164 (offset 2 lines). >>> Hunk #4 succeeded at 192 (offset 2 lines). >>> Hunk #5 succeeded at 217 (offset 2 lines). >>> Hunk #6 FAILED at 256. >>> Hunk #7 succeeded at 276 (offset 2 lines). >>> 1 out of 7 hunks FAILED -- saving rejects to file vmware_vmss.c.rej >>> patching file vmware_vmss.h >>> $ >>> >>> Thanks, >>> Dave >>> >>> >>> ----- Original Message ----- >>>> Dave, >>>> >>>> attached patch is to add vmss memroy regions support. >>>> >>>> There might be holes in the memory address saved for PCI etc. >>>> In such case memory dump is divided into regions. Currently >>>> up to 3 regions are supported. >>>> >>>> Memory dump larger than 3GB will have the first hole. >>>> My dropbox space used up, I cannot attach a 4GB memory dump. >>>> and here is how it looks like in the vmss meta data dump. >>>> >>>> 3GB: >>>> === >>>> - Group: memory pos=0x1f6f6 size=0xc000090c >>>> ------------------------------------ >>>> align_mask[0, 0] => 0x00ffff >>>> regionsCount => 0x000000 >>>> Memory[0, 0] => BLOCK, pos=0x20000, size=0xc0000000 >>>> >>>> 4GB: >>>> === >>>> - Group: memory pos=0x1f6f6 size=0x10000090c >>>> ----------------------------------- >>>> align_mask[0, 0] => 0x00ffff >>>> regionsCount => 0x000002 >>>> regionPageNum[0] => 0x000000 >>>> regionPPN[0] => 0x000000 >>>> regionSize[0] => 0x0c0000 >>>> regionPageNum[1] => 0x0c0000 >>>> regionPPN[1] => 0x100000 >>>> regionSize[1] => 0x040000 >>>> Memory[0, 0] => BLOCK, pos=0x20000, size=0x100000000 >>>> >>>> rgds, >>>> Dyno >>>> >>>> -- >>>> Crash-utility mailing list >>>> Crash-utility(a)redhat.com >>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.redhat.com_mailm... >>> -- >>> Crash-utility mailing list >>> Crash-utility(a)redhat.com >>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.redhat.com_mailm... >>> >>> >> -- >> Crash-utility mailing list >> Crash-utility(a)redhat.com >> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.redhat.com_mailm... > -- > Crash-utility mailing list > Crash-utility(a)redhat.com > https://urldefense.proofpoint.com/v2/url?u=https-3A__www.redhat.com_mailm... -- Crash-utility mailing list Crash-utility(a)redhat.com https://urldefense.proofpoint.com/v2/url?u=https-3A__www.redhat.com_mailm...