Andriy Gapon wrote:
quoted text
> 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

_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"