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

[Suzuki K. Poulose]

On 02/17/2012 07:25 PM, Dave Anderson wrote:
>
>
> ----- Original Message -----
>> 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!  ;-)
>> Ah, well, I was talking about the x86_64 dump.
>> I could explain the PPC side of affairs :-). We have page the
>> following
>> flags set for the page(with the  actual PPC44x page flags support) :
>>
>> VIRTUAL   PHYSICAL
>> d1002000  20ec00000
>>
>> PAGE DIRECTORY: c0578000
>>     PGD: c0579a20 =>  c784b000
>>     PMD: c784b000 =>  c784b010
>>     PTE: c784b010 =>  20ec0051b
>>    PAGE: 20ec00000
>>
>>      PTE     PHYSICAL   FLAGS
>> 20ec0051b  20ec00000  (PRESENT|RW|GUARDED|NO_CACHE|DIRTY|ACCESSED)
>>
>>
>> So the page is 'present', but there is no page descriptor associated
>> with it.
>> Hence dump_mem_map() would still be called and hence the problem.
>>
>> Why doesn't it get called in x86_64 case even when the flags indicate
>> page 'PRESENT' ?
>
> I don't know -- that's what I was asking!

The only suspect is :

	if (page_exists) {
>>>             if ((pc->flags & DEVMEM) && (paddr >= VTOP(vt->high_memory))) <<
                         return;


  But I'd like to prevent it
> in all cases with the patch to do_vtop().
Yep, thats the best way to fix it as of now.

Thanks
Suzuki