[Crash-utility] [PATCH v3 0/3] vmalloc translation support for PPC
Suzuki K. Poulose
suzuki at in.ibm.com
Fri Feb 17 06:10:22 UTC 2012
On 02/17/2012 12:43 AM, Dave Anderson wrote:
>
>
> ----- Original Message -----
>> 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 :(.
>
> Right -- I tried debugging it from the x86 and x86_64 perspective,
> and couldn't see why ppc would be different! ;-)
>
> Oh well...
>
>>>
>>> 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.
>
> OK, so do you agree that just patching do_vtop() makes more sense?
Yep. One more question. Why don't we use phys_to_page() in the dump_mem_mapXX() to
get the page struct and then do the rest of the work ?
Thanks
Suzuki
>
>>
>> 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.
>
> OK.
>
> Thanks,
> Dave
>
More information about the Crash-utility
mailing list