Re: [PATCH] Handle vma regions with no backing page

[Andrea Arcangeli <andrea@qumranet.com>]

On Tue, Jun 03, 2008 at 02:17:55PM +0300, Ben-Ami Yassour wrote:
> Anthony Liguori <aliguori@us.ibm.com> wrote on 04/29/2008 05:32:09 PM:
>
>> Subject
>>
>> [PATCH] Handle vma regions with no backing page
>>
>> This patch allows VMA's that contain no backing page to be used for guest
>> memory.  This is a drop-in replacement for Ben-Ami's first page in his 
>> direct
>> mmio series.  Here, we continue to allow mmio pages to be represented in 
>> the
>> rmap.
>
>>  struct page *gfn_to_page(struct kvm *kvm, gfn_t gfn)
>>  {
>> -   return pfn_to_page(gfn_to_pfn(kvm, gfn));
>> +   pfn_t pfn;
>> +
>> +   pfn = gfn_to_pfn(kvm, gfn);
>> +   if (pfn_valid(pfn))
>> +      return pfn_to_page(pfn);
>> +
>> +   return NULL;
>>  }
>
> We noticed that pfn_valid does not always works as expected by this patch 
> to indicate that a pfn has a backing page.
> We have seen a case where CONFIG_NUMA was not set and then where pfn_valid 
> returned 1 for an mmio pfn.
> We then changed the config file with CONFIG_NUMA set and it worked fine as 
> expected (since a different implementation of pfn_valid was used).
>
> How should we overcome this issue?

There's a page_is_ram() too, but that's the e820 map check and it
means it's RAM not that there's a page backing store. Certainly if
it's not ram we should go ahead with just the pfn but it'd be a
workaround.

I really think it'd be better off to fix pfn_valid to work for NUMA. I
can't see how pfn_valid can be ok to return true when there's no
backing page... Probably pfn_valid was used for debugging todate, but
if you check vm_normal_page you'll see that it is not used just for
debugging and it seems VM_MIXEDMAP will break as much as KVM.

I can't see how VM_MIXEDMAP can be sane doing pfn_to_page(pfn) and
pretending this is a normal page, when there's no 'struct page'
backing the pfn.