Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 9 Mar 2021 08:29:51 -0700
From:   Alex Williamson <alex.williamson@redhat.com>
To:     Jason Gunthorpe <jgg@nvidia.com>
Cc:     "Zengtao (B)" <prime.zeng@hisilicon.com>,
        Peter Xu <peterx@redhat.com>,
        Cornelia Huck <cohuck@redhat.com>,
        Kevin Tian <kevin.tian@intel.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Giovanni Cabiddu <giovanni.cabiddu@intel.com>,
        Michel Lespinasse <walken@google.com>,
        Jann Horn <jannh@google.com>,
        Max Gurtovoy <mgurtovoy@nvidia.com>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Linuxarm <linuxarm@huawei.com>
Subject: Re: [PATCH] vfio/pci: make the vfio_pci_mmap_fault reentrant
Message-ID: <20210309082951.75f0eb01@x1.home.shazbot.org>
In-Reply-To: <20210309124609.GG2356281@nvidia.com>
References: <1615201890-887-1-git-send-email-prime.zeng@hisilicon.com>
        <20210308132106.49da42e2@omen.home.shazbot.org>
        <20210308225626.GN397383@xz-x1>
        <6b98461600f74f2385b9096203fa3611@hisilicon.com>
        <20210309124609.GG2356281@nvidia.com>
Organization: Red Hat
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, 9 Mar 2021 08:46:09 -0400
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Tue, Mar 09, 2021 at 03:49:09AM +0000, Zengtao (B) wrote:
> > Hi guys:
> > 
> > Thanks for the helpful comments, after rethinking the issue, I have proposed
> >  the following change: 
> > 1. follow_pte instead of follow_pfn.  
> 
> Still no on follow_pfn, you don't need it once you use vmf_insert_pfn

vmf_insert_pfn() only solves the BUG_ON, follow_pte() is being used
here to determine whether the translation is already present to avoid
both duplicate work in inserting the translation and allocating a
duplicate vma tracking structure.

> > 2. vmf_insert_pfn loops instead of io_remap_pfn_range
> > 3. proper undos when some call fails.
> > 4. keep the bigger lock range to avoid unessary pte install.   
> 
> Why do we need locks at all here?

For the vma tracking and testing whether the fault is already
populated.  Once we get rid of the vma list, maybe it makes sense to
only insert the faulting page rather than the entire vma, at which
point I think we'd have no reason to serialize.  Thanks,

Alex