12:10 a.m.
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