Re: minidump size on amd64

From: Alan Cox <alc_at_rice.edu>
Date: Tue, 09 Nov 2010 02:02:07 -0600
Andriy Gapon wrote:
> So, here is the next version of the patch:
> http://people.freebsd.org/~avg/amd64-minidump.4.diff
>
> Changes since the last version:
> 1. libkvm - try to support both the new and the previous formats/versions of
> amd64 minidump.  I am not entirely sure about style in which I handled handling
> of version 1 minidump.  Identifier names like pmapsize (for "page map size") and
> page_map could also be improved, perhaps.
> 2. kernel - implemented dumping of 1GB pages via "fake" 512 x 2MB pages per
> Alan's suggestion.
>
> The change is only compile tested so far.  Not sure if it's possible to test
> handling 1GB pages yet :-)
>
> As always, reviews, testing and suggestions are very welcome.
>   


The kernel portion of the patch looks correct.  If I were to make one 
stylistic suggestion, it would be to make the control flow of the outer 
and inner loops as similar as possible, that is,

    for (...
       if ((pdp[i] & PG_V) == 0) {
          ...
          continue;
       }
       if ((pdp[i] & PG_PS) != 0) {
          ...
          continue;
       }
       for (...
          if ((pd[j] & PG_V) == 0)
             continue;
          if ((pd[j] & PG_PS) != 0) {
             ...
             continue;
          }
          for (...
             if ((pt[x] & PG_V) == 0)
                continue;
             ...

I think this would make the code a little easier to follow.

Alan
Received on Tue Nov 09 2010 - 07:02:10 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:09 UTC