All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: No need to retry for hva_to_pfn_remapped()
@ 2020-04-16 15:59 Peter Xu
  2020-05-04 16:59 ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Xu @ 2020-04-16 15:59 UTC (permalink / raw)
  To: kvm; +Cc: Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, peterx

hva_to_pfn_remapped() calls fixup_user_fault(), which has already
handled the retry gracefully.  Even if "unlocked" is set to true, it
means that we've got a VM_FAULT_RETRY inside fixup_user_fault(),
however the page fault has already retried and we should have the pfn
set correctly.  No need to do that again.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 virt/kvm/kvm_main.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 2f1f2f56e93d..6aaed69641a5 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1824,8 +1824,6 @@ static int hva_to_pfn_remapped(struct vm_area_struct *vma,
 		r = fixup_user_fault(current, current->mm, addr,
 				     (write_fault ? FAULT_FLAG_WRITE : 0),
 				     &unlocked);
-		if (unlocked)
-			return -EAGAIN;
 		if (r)
 			return r;
 
@@ -1896,15 +1894,12 @@ static kvm_pfn_t hva_to_pfn(unsigned long addr, bool atomic, bool *async,
 		goto exit;
 	}
 
-retry:
 	vma = find_vma_intersection(current->mm, addr, addr + 1);
 
 	if (vma == NULL)
 		pfn = KVM_PFN_ERR_FAULT;
 	else if (vma->vm_flags & (VM_IO | VM_PFNMAP)) {
 		r = hva_to_pfn_remapped(vma, addr, write_fault, writable, &pfn);
-		if (r == -EAGAIN)
-			goto retry;
 		if (r < 0)
 			pfn = KVM_PFN_ERR_FAULT;
 	} else {
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: No need to retry for hva_to_pfn_remapped()
  2020-04-16 15:59 [PATCH] KVM: No need to retry for hva_to_pfn_remapped() Peter Xu
@ 2020-05-04 16:59 ` Paolo Bonzini
  2020-05-08  2:25   ` Peter Xu
  0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2020-05-04 16:59 UTC (permalink / raw)
  To: Peter Xu, kvm; +Cc: Sean Christopherson, Vitaly Kuznetsov

On 16/04/20 17:59, Peter Xu wrote:
> hva_to_pfn_remapped() calls fixup_user_fault(), which has already
> handled the retry gracefully.  Even if "unlocked" is set to true, it
> means that we've got a VM_FAULT_RETRY inside fixup_user_fault(),
> however the page fault has already retried and we should have the pfn
> set correctly.  No need to do that again.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  virt/kvm/kvm_main.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 2f1f2f56e93d..6aaed69641a5 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1824,8 +1824,6 @@ static int hva_to_pfn_remapped(struct vm_area_struct *vma,
>  		r = fixup_user_fault(current, current->mm, addr,
>  				     (write_fault ? FAULT_FLAG_WRITE : 0),
>  				     &unlocked);
> -		if (unlocked)
> -			return -EAGAIN;
>  		if (r)
>  			return r;
>  
> @@ -1896,15 +1894,12 @@ static kvm_pfn_t hva_to_pfn(unsigned long addr, bool atomic, bool *async,
>  		goto exit;
>  	}
>  
> -retry:
>  	vma = find_vma_intersection(current->mm, addr, addr + 1);
>  
>  	if (vma == NULL)
>  		pfn = KVM_PFN_ERR_FAULT;
>  	else if (vma->vm_flags & (VM_IO | VM_PFNMAP)) {
>  		r = hva_to_pfn_remapped(vma, addr, write_fault, writable, &pfn);
> -		if (r == -EAGAIN)
> -			goto retry;
>  		if (r < 0)
>  			pfn = KVM_PFN_ERR_FAULT;
>  	} else {
> 

Queued, thanks.

Paolo


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: No need to retry for hva_to_pfn_remapped()
  2020-05-04 16:59 ` Paolo Bonzini
@ 2020-05-08  2:25   ` Peter Xu
  2020-05-08  7:07     ` Paolo Bonzini
  2020-05-29  9:45     ` Paolo Bonzini
  0 siblings, 2 replies; 5+ messages in thread
From: Peter Xu @ 2020-05-08  2:25 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Sean Christopherson, Vitaly Kuznetsov

On Mon, May 04, 2020 at 06:59:30PM +0200, Paolo Bonzini wrote:
> On 16/04/20 17:59, Peter Xu wrote:
> > hva_to_pfn_remapped() calls fixup_user_fault(), which has already
> > handled the retry gracefully.  Even if "unlocked" is set to true, it
> > means that we've got a VM_FAULT_RETRY inside fixup_user_fault(),
> > however the page fault has already retried and we should have the pfn
> > set correctly.  No need to do that again.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  virt/kvm/kvm_main.c | 5 -----
> >  1 file changed, 5 deletions(-)
> > 
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 2f1f2f56e93d..6aaed69641a5 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -1824,8 +1824,6 @@ static int hva_to_pfn_remapped(struct vm_area_struct *vma,
> >  		r = fixup_user_fault(current, current->mm, addr,
> >  				     (write_fault ? FAULT_FLAG_WRITE : 0),
> >  				     &unlocked);
> > -		if (unlocked)
> > -			return -EAGAIN;
> >  		if (r)
> >  			return r;
> >  
> > @@ -1896,15 +1894,12 @@ static kvm_pfn_t hva_to_pfn(unsigned long addr, bool atomic, bool *async,
> >  		goto exit;
> >  	}
> >  
> > -retry:
> >  	vma = find_vma_intersection(current->mm, addr, addr + 1);
> >  
> >  	if (vma == NULL)
> >  		pfn = KVM_PFN_ERR_FAULT;
> >  	else if (vma->vm_flags & (VM_IO | VM_PFNMAP)) {
> >  		r = hva_to_pfn_remapped(vma, addr, write_fault, writable, &pfn);
> > -		if (r == -EAGAIN)
> > -			goto retry;
> >  		if (r < 0)
> >  			pfn = KVM_PFN_ERR_FAULT;
> >  	} else {
> > 
> 
> Queued, thanks.

Paolo, Is it still possible to unqueue this patch?  I overlooked the fact that
if unlocked==true then the vma pointer could be invalidated, so the 2nd
follow_pfn() is potentially racy.  Sorry for the trouble!

-- 
Peter Xu


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: No need to retry for hva_to_pfn_remapped()
  2020-05-08  2:25   ` Peter Xu
@ 2020-05-08  7:07     ` Paolo Bonzini
  2020-05-29  9:45     ` Paolo Bonzini
  1 sibling, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2020-05-08  7:07 UTC (permalink / raw)
  To: Peter Xu; +Cc: kvm, Sean Christopherson, Vitaly Kuznetsov

On 08/05/20 04:25, Peter Xu wrote:
>> Queued, thanks.
> 
> Paolo, Is it still possible to unqueue this patch?  I overlooked the fact that
> if unlocked==true then the vma pointer could be invalidated, so the 2nd
> follow_pfn() is potentially racy.  Sorry for the trouble!
> 

Sure, thanks.

Paolo


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: No need to retry for hva_to_pfn_remapped()
  2020-05-08  2:25   ` Peter Xu
  2020-05-08  7:07     ` Paolo Bonzini
@ 2020-05-29  9:45     ` Paolo Bonzini
  1 sibling, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2020-05-29  9:45 UTC (permalink / raw)
  To: Peter Xu; +Cc: kvm, Sean Christopherson, Vitaly Kuznetsov

On 08/05/20 04:25, Peter Xu wrote:
> Even if "unlocked" is set to true, it
> means that we've got a VM_FAULT_RETRY inside fixup_user_fault(),

I have just reverted it.  Thanks!

Paolo


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-05-29  9:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-16 15:59 [PATCH] KVM: No need to retry for hva_to_pfn_remapped() Peter Xu
2020-05-04 16:59 ` Paolo Bonzini
2020-05-08  2:25   ` Peter Xu
2020-05-08  7:07     ` Paolo Bonzini
2020-05-29  9:45     ` Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.