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@xxxxxxxxxx>
> > ---
> > 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
