Re: [patch 02/13] KVM: MMU: switch to get_user_pages_fast

[Marcelo Tosatti <mtosatti@redhat.com>]

On Sun, Sep 07, 2008 at 11:45:41AM +0300, Avi Kivity wrote:
> Marcelo Tosatti wrote:
>> Avoid mmap_sem / pt lock acquision if the pagetables are present. The
>> improvement for hugepage backed guests is more significant, since pte
>> walk + page grab for such mappings is serialized by mm->page_table_lock.
>>
>> CC: Andrea Arcangeli <andrea@qumranet.com>
>>
>>   
>
> I'd like to apply this.  Andrea, can you verify that mmu notifiers  
> interaction is okay?
>
>>   static int kvm_vm_fault(struct vm_area_struct *vma, struct vm_fault 
>> *vmf)
>>  {
>> +	struct page *page[1];
>> +	unsigned long addr;
>> +	int npages;
>> +	gfn_t gfn = vmf->pgoff;
>>  	struct kvm *kvm = vma->vm_file->private_data;
>> -	struct page *page;
>>  -	if (!kvm_is_visible_gfn(kvm, vmf->pgoff))
>> +	addr = gfn_to_hva(kvm, gfn);
>> +	if (kvm_is_error_hva(addr))
>>  		return VM_FAULT_SIGBUS;
>> -	page = gfn_to_page(kvm, vmf->pgoff);
>> -	if (is_error_page(page)) {
>> -		kvm_release_page_clean(page);
>> +
>> +	npages = get_user_pages(current, current->mm, addr, 1, 1, 0, page,
>> +				NULL);
>> +	if (unlikely(npages != 1))
>>  		return VM_FAULT_SIGBUS;
>> -	}
>> -	vmf->page = page;
>> +
>> +	vmf->page = page[0];
>>  	return 0;
>>  }
>>    
>
> Why this change?

Because get_user_pages_fast grabs mmap_sem if necessary, but ->vm_fault
already holds it.

Deadlock:

CPU0                        CPU1    

down_read(mmap_sem)
kvm_vm_fault
                            down_write(mmap_sem)
gfn_to_page
get_user_pages_fast
down_read(mmap_sem)