Re: Re: Balloon driver crash

[Jeremy Fitzhardinge <jeremy@goop.org>]

On 06/03/2010 03:38 PM, Dave McCracken wrote:
> On Thursday, June 03, 2010, Ian Campbell wrote:
>   
>>> This seems to be about line 343 of drivers/xen/balloon.c in the
>>> subroutine  decrease_reservation which is
>>>
>>>                  for (j = 0; j < balloon_npages; j++, lpfn++, mfn++) {
>>>                          if ((discontig_frame_list[j] =
>>> pfn_to_mfn(lpfn)) 
>>>
>>> != mfn)
>>>
>>>                                  discontig_free = 1;
>>>
>>>                          set_phys_to_machine(lpfn, INVALID_P2M_ENTRY);
>>>
>>> /* here */                  if (!PageHighMem(page)) {
>>>
>>>                                  ret = HYPERVISOR_update_va_mapping(
>>>                                          (unsigned long)__va(lpfn << 
>>>
>>> PAGE_SHIFT),
>>>
>>>                                          __pte_ma(0), 0);
>>>                                  BUG_ON(ret);
>>>                          }
>>>                  }
>>>
>>>
>>> >From what I can tell page is meaningless in this context as it is just 
>>>
>>> a temporary variable used in the previous loop, so I would 
>>> guess that PageHighMem should be checking something else, or page should 
>>> be set somewhere eg. at a guess page=pfn_to_page(lpfn);
>>>       
>> That would be my guess also. CCing Dave McCracken who looks to have
>> introduced this code in 0e898d5e "Add hugepage support to balloon
>> driver"
>>     
> Yes, that is definitely an error.  The offending line should be
>
> 	if (!PageHighMem(pfn_to_page(lpfn))) {
>   

Actually it looks like this was fixed on the xen/hugepage-balloon a long
time ago, but I guess that branch wasn't merged across to xen/next
because it was based on xen/master and so awkward to merge into the
.32-based and later branches.

I need to check how the current hugepage support has made it into the
.32 based tree, assuming it has.  It will take a bit of git archeology.

    J