> > I fixed line up problem in v3 patch, please check v3 patch in my another 
> > mail.
> > The major problem here is ADDR-SPACE and PAGE-COUNT width is greater than 
> > long size on 32 bit kernel.
>  
> OK, thanks.  But as far as the output display goes, I don't particularly
> like the naming of the "PAGE-CNT" column header.  I'd prefer that it be
> changed to reflect the "address_space.nrpages" member that it comes from.
> I also think that the page count should be right-justified.  So with this
> additional patch:
> 
> --- filesys.c_oliver  2015-06-25 15:30:44.549942532 -0400
> +++ filesys.c 2015-06-25 15:28:17.157947564 -0400
> @@ -2370,13 +2370,12 @@
>       fill_task_struct(task);
>  
>       if (flags & PRINT_PAGES) {
> -             sprintf(files_header, " FD%s%s%s%s%s%s%sTYPE%sPATH\n",
> +             sprintf(files_header, " FD%s%s%s%s%sNRPAGES%sTYPE%sPATH\n",
>                       space(MINSPACE),
>                       mkstring(buf1, VADDR_PRLEN, CENTER|LJUST, "INODE"),
>                       space(MINSPACE),
>                       mkstring(buf2, VADDR_PRLEN, CENTER|LJUST, "MAPPING"),
>                       space(MINSPACE),
> -                     mkstring(buf3, LONG_PRLEN, CENTER|LJUST, "PAGE-CNT"),
>                       space(MINSPACE),
>                       space(MINSPACE));
>       } else {
> @@ -2935,8 +2934,8 @@
>                               CENTER|RJUST|LONG_HEX,
>                               MKSTR(i_mapping)),
>                               space(MINSPACE),
> -                             mkstring(buf3, LONG_PRLEN,
> -                             CENTER|RJUST|LONG_DEC,
> +                             mkstring(buf3, strlen("NRPAGES"),
> +                             RJUST|LONG_DEC,
>                               MKSTR(count)),
>                                  space(MINSPACE),
>                                  type,

Merge above patch in my v4 patch.
Thanks a lot!

> 
> 
> >> And secondly, taking the address_space e6c6a754 from the task above,
> >> again, shouldn't the page count above be reflected in the number of 
> >> shown by the address_space tree dump, where the page dump seems to
> >> be missing about 20000 pages?:
> >> 
> >> crash> files -m e6c6a754 | wc -l
> >> 128825
> >> crash>
> > 
> > I can't reproduce this issue on my 32bit kernel.
> > 
> > Could you verify whether you can get the same number of pages by tree 
> > command?
> > 
> > First step, get the page_tree address,
> > 
> > address_space.page_tree e6c6a754
> > 
> > Second step, using tree command dump pages,
> > 
> > tree -t radix -N <page_tree address>
> 
> As it turns out, that dumpfile was created with the "snap.so" extension 
> module,
> which creates a vmcore while running on a live system.  And since the kernel
> is running while the dumpfile is being created, you may see any number of
> odd outputs.  Running the "tree" command above fails with a duplicate list 
> entry
> failure, so the "files -a" command should also fail prematurely.  So it's not 
> a
> problem.

Thanks for confirmation. Yes, live memory dump can be this kind of problem.


> >> 
> >> For that matter, displaying the address_space address is redundant 
> >> since (1) it has to be entered as the command argument, and (2) it gets 
> >> shown in every page line "MAPPING". On the other hand, perhaps the inode 
> >> that contains the address_space structure would be helpful, say, like
> >> this:
> > 
> > In my v3 patch, I files -a accept inode as parameter, instead of address 
> > space address.
> 
> OK, although I suggested "-a" because it was taking an "address_space" address
> as an argument.  So perhaps it should be "-i <inode-address>"?

I prefer -p option, as it indicates the memory mapping view for the output.
> > The benefit of this change is, files -a is more separate with files -m. > > 
> > We decoupled two commands here.
> 
> The first line of the page dump is kind of unusual:
> 
>   crash> files -a ffff8801085680b0
>   Address space ffff8801085681f0, 35002 pages
>   
>         PAGE        PHYSICAL      MAPPING       INDEX CNT FLAGS
>   ffffea00072ca3c0 1cb28f000 ffff8801085681f0        0  1 5ff0000002006c 
> referenced,uptodate,lru,active,mappedtodisk
>   ffffea0000c1f340  307cd000 ffff8801085681f0      200  1 3ff0000002006c 
> referenced,uptodate,lru,active,mappedtodisk
>   ffffea00030dea40  c37a9000 ffff8801085681f0      201  1 3ff0000002006c 
> referenced,uptodate,lru,active,mappedtodisk
>   ...
> 
> Can you make it show INODE, ADDRESS_SPACE, and NRPAGES values (in that order),
> similar to how I suggested before?
>  
> 
> > But how about using -p instead of -a for page dump?
> 
> Yes, sure -- either -i or -p makes sense.
I will use -p in v4 patch as I mentioned above. > OK, you've convinced me...  
;-)
> 
> So, aside from the patch I added above, the addition of the INODE, 
> ADDRESS_SPACE
> and NRPAGES display to the top of the "files -i|p" command, and the help page 
> additions,
> this is looking pretty good.

Sure. Please see my v4 patch.

                                          
--
Crash-utility mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/crash-utility

Reply via email to