On 06/09/2010 10:53 AM, Lai Jiangshan wrote:
When page fault, we always call get_user_pages(write=1).
Actually, we don't need to do this when it is not write fault.
get_user_pages(write=1) will cause shared page(ksm) copied.
If this page is not modified in future, this copying and the copied page
are just wasted. Ksm may scan and merge them and may cause thrash.
This patch is not for inclusion, because I know nothing about mmio
and this patch includes a "workaround" which ensures mmio pfns
are always writable in tdp_page_fault().
The guest can't even boot up without this workaround.
mmio pfns are used for device assignment. These are host pfns that
don't have a struct page, instead they belong to a device BAR.
I don't understand why you see a failure since they aren't even present
on guests without assigned devices.
@@ -2357,7 +2359,12 @@ static int tdp_page_fault(struct kvm_vcpu *vcpu, gva_t gpa,
mmu_seq = vcpu->kvm->mmu_notifier_seq;
smp_rmb();
- pfn = kvm_get_pfn_for_gfn(vcpu->kvm, gfn, 1);
+ pfn = kvm_get_pfn_for_gfn(vcpu->kvm, gfn, error_code& PFERR_WRITE_MASK);
This is a pessimization, since now we may need two faults per page, one
to page it in read-only and another to establish write access.
May not be so bad for tdp, but will surely reduce performance on shadow.
The way I think it should be improved, is to extend
get_user_pages_fast() to also return the pte. So now, if we get a page
for read, but it happens to have a writeable/dirty pte, we can still
allow write access in the spte.
+ if (!(error_code& PFERR_WRITE_MASK)&& kvm_is_mmio_pfn(pfn)) {
+ kvm_release_pfn_clean(pfn);
+ /* I don't know why we have to ensure mmio pfns are always writable. */
+ pfn = kvm_get_pfn_for_gfn(vcpu->kvm, gfn, 1);
+ }
Wierd. For what gfn/pfns does this trigger?
In general this is a great optimization.
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
