Re: [Crash-utility] [PATCH v3 0/3] vmalloc translation support for PPC

On 02/16/2012 09:52 PM, Dave Anderson wrote: > > > ----- Original Message ----- > ... >>> So just do the same thing -- no verbose expanation is required. >> >> There are two ways to fix this : >> >> 1) Fix dump_mem_map*() to print the header only when there is >> information to dump. >> >> --- a/memory.c >> +++ b/memory.c >> @@ -4637,13 +4637,6 @@ dump_mem_map_SPARSEMEM(struct meminfo *mi) >> continue; >> } >> >> - if (print_hdr) { >> - if (!(pc->curcmd_flags& HEADER_PRINTED)) >> - fprintf(fp, "%s", hdr); >> - print_hdr = FALSE; >> - pc->curcmd_flags |= HEADER_PRINTED; >> - } >> - >> pp = section_mem_map_addr(section); >> pp = sparse_decode_mem_map(pp, section_nr); >> phys = (physaddr_t) section_nr * >> PAGES_PER_SECTION() >> * PAGESIZE(); >> @@ -4854,6 +4847,13 @@ dump_mem_map_SPARSEMEM(struct meminfo *mi) >> } >> >> if (bufferindex> buffersize) { >> + if (print_hdr) { >> + if (!(pc->curcmd_flags& >> HEADER_PRINTED)) >> + fprintf(fp, "%s", >> hdr); >> + print_hdr = FALSE; >> + pc->curcmd_flags |= >> HEADER_PRINTED; >> + } >> + >> fprintf(fp, "%s", outputbuffer); >> bufferindex = 0; >> } >> @@ -4867,6 +4867,13 @@ dump_mem_map_SPARSEMEM(struct meminfo *mi) >> } >> >> if (bufferindex> 0) { >> + if (print_hdr) { >> + if (!(pc->curcmd_flags& HEADER_PRINTED)) >> + fprintf(fp, "%s", hdr); >> + print_hdr = FALSE; >> + pc->curcmd_flags |= HEADER_PRINTED; >> + } >> + >> fprintf(fp, "%s", outputbuffer); >> } >> >> Similarly for the dump_mem_map(). >> >> 2) Fix ppc_pgd_vtop() to return FALSE if the paddr> >> machdep->memsize >> >> --- a/ppc.c >> +++ b/ppc.c >> @@ -438,6 +438,10 @@ ppc_pgd_vtop(ulong *pgd, ulong vaddr, >> physaddr_t >> *paddr, int verbose) >> >> *paddr = PAGEBASE(pte) + PAGEOFFSET(vaddr); >> >> + if (*paddr> machdep->memsize) >> + /* We don't have pages above System RAM */ >> + return FALSE; >> + >> return TRUE; >> >> no_page: >> >> I prefer the (1). What do you think ? > > Hi Suzuki, > > Hmmm -- with respect to (1), I suppose that would work, although > given that both x86 and x86_64 pass through dump_mem_map_SPARSEMEM() > without printing the header in a non-existent-page case, I don't > understand why ppc is different? Yep, I digged into that a little, but not deep enough to debug it with a dump. Nothing was evident from the code :(.

> > And I'm thinking that a more general solution might be to change > do_vtop() here, and not even bother calling the relevant > dump_mem_map() > function if there's no page struct associated with it: > > --- memory.c 10 Feb 2012 16:41:38 -0000 1.273 > +++ memory.c 16 Feb 2012 14:18:03 -0000 > @@ -2796,7 +2796,7 @@ > do_vtop(ulong vaddr, struct task_context *tc, ulong vtop_flags) > { > physaddr_t paddr; > - ulong vma; > + ulong vma, pageptr; > int page_exists; > struct meminfo meminfo; > char buf1[BUFSIZE]; > @@ -2930,7 +2930,7 @@ > > fprintf(fp, "\n"); > > - if (page_exists) { > + if (page_exists&& phys_to_page(paddr,&pageptr)) { > if ((pc->flags& DEVMEM)&& (paddr>= > VTOP(vt->high_memory))) > return; > BZERO(&meminfo, sizeof(struct meminfo)); > > And w/respect to (2), wouldn't that just cause the generic kvtop() > to fail? And if so, it kind of re-defines the meaning of kvtop(), > even though its current callers pretty much expect to receive > a legitimate physical memory address. But if a virtual address > resolves to a PTE with any legitimate address in it, then kvtop() > should return whatever's there. Yep, I agree. > > But I'm still wondering what makes ppc behave differently in > dump_mem_map_SPARSEMEM()? > Btw, we don't have SPARSMEM on ppc44x, and end up in dump_mem_map(). I was patching both the functions to cover all the platforms.

Also, I found out that we need to abstract away the definition of Page flags as well, since it differes for different platforms (except for the _PAGE_PRESENT). I will include the changes in the next version.