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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility


[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux