Re: Xen-unstable panic: FATAL PAGE FAULT

["Jan Beulich" <JBeulich@novell.com>]

>>> On 31.08.10 at 18:35, Keir Fraser <keir.fraser@eu.citrix.com> wrote:
> On 31/08/2010 17:22, "Jan Beulich" <JBeulich@novell.com> wrote:
> 
>>> Where is even that constraint ensured in the code? I can't see it (I would
>>> have assumed that pfn_pdx_hole_setup() would be ensuring it).
>> 
>> That's somewhat implicit: srat_parse_regions() gets passed an
>> address that is at least BOOTSTRAP_DIRECTMAP_END (i.e. 4G).
>> Thus srat_parse_regions() starts off with a mask with the lower
>> 32 bits all set (only more bits can get set subsequently). Thus
>> the earliest zero bit pfn_pdx_hole_setup() can find is bit 20
>> (due to the >> PAGE_SHIFT in the invocation). Consequently
>> the smallest chunk where arithmetic is valid really is 4Gb, not
>> 256Mb as I first wrote.
> 
> Well, that's a bit too implicit for me. How about we initialise 'j' to
> MAX_ORDER in pfn_pdx_hole_setup() with a comment about supporting page_info
> pointer arithmetic within allocatable multi-page regions?
> 
> Something like the appended (but with a code comment)?

Yes, that would seem reasonable (and not affecting current behavior).

Jan

> --- a/xen/arch/x86/x86_64/mm.c    Mon Aug 30 14:59:12 2010 +0100
> +++ b/xen/arch/x86/x86_64/mm.c    Tue Aug 31 17:34:34 2010 +0100
> @@ -165,7 +165,8 @@
>  {
>      unsigned int i, j, bottom_shift, hole_shift;
>  
> -    for ( hole_shift = bottom_shift = j = 0; ; )
> +    hole_shift = bottom_shift = 0;
> +    for ( j = MAX_ORDER-1; ; )
>      {
>          i = find_next_zero_bit(&mask, BITS_PER_LONG, j);
>          j = find_next_bit(&mask, BITS_PER_LONG, i);