Date: Wed, 16 Nov 2016 23:12:06 -0700
From: Alex Williamson <alex.williamson@redhat.com>
To: Alexey Kardashevskiy <aik@ozlabs.ru>
Cc: Kirti Wankhede <kwankhede@nvidia.com>, pbonzini@redhat.com,
        kraxel@redhat.com, cjia@nvidia.com, qemu-devel@nongnu.org,
        kvm@vger.kernel.org, kevin.tian@intel.com, jike.song@intel.com,
        bjsdjshi@linux.vnet.ibm.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v14 09/22] vfio iommu type1: Add task structure to
 vfio_dma
Message-ID: <20161116231206.411c68ef@t450s.home>
In-Reply-To: <6d256aeb-cd3b-0827-667f-054cdacc45ca@ozlabs.ru>
References: <1479329194-10247-1-git-send-email-kwankhede@nvidia.com>
        <1479329194-10247-10-git-send-email-kwankhede@nvidia.com>
        <6d256aeb-cd3b-0827-667f-054cdacc45ca@ozlabs.ru>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1766
Lines: 35

On Thu, 17 Nov 2016 16:41:14 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> On 17/11/16 07:46, Kirti Wankhede wrote:
> > Add task structure to vfio_dma structure. Task structure is used for:
> > - During DMA_UNMAP, same task who mapped it or other task who shares same
> > address space is allowed to unmap, otherwise unmap fails.
> > QEMU maps few iova ranges initially, then fork threads and from the child
> > thread calls DMA_UNMAP on previously mapped iova. Since child shares same
> > address space, DMA_UNMAP is successful.
> > - Avoid accessing struct mm while process is exiting by acquiring
> > reference of task's mm during page accounting.
> > - It is also used to get task mlock capability and rlimit for mlock.
> > 
> > Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> > Signed-off-by: Neo Jia <cjia@nvidia.com>
> > Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>  
> 
> 
> I keep whinging that @mm should be referenced, not @current but you keep
> referencing @current even if you only need @mm and you are not telling why
> - and I am wondering what I am missing here? Something else will be used
> from @task later, besides just @mm?

Yes, we reference @current from vfio_dma_do_map() and this is stored
on the struct vfio_dma.  A reference to current is held because the
external page pinning in vfio_pin_page_external() needs to test the
capabilities of the task for CAP_IPC_LOCK to know whether locked memory
limits are in effect for the task even when it's not @current (ie. an
asynchronous call from the vendor driver regardless of what task is
currently running).  There are also various get_task_mm() taken
temporarily when we're working with the mm of that task.  Do you spot
any issues with this behavior? Thanks,

Alex