Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit

From: Baolu Lu
Date: Fri Apr 29 2022 - 03:54:09 EST

On 2022/4/28 16:39, Jean-Philippe Brucker wrote:
On Tue, Apr 26, 2022 at 04:31:57PM -0700, Dave Hansen wrote:
On 4/26/22 09:48, Jean-Philippe Brucker wrote:
On Tue, Apr 26, 2022 at 08:27:00AM -0700, Dave Hansen wrote:
On 4/25/22 09:40, Jean-Philippe Brucker wrote:
The problem is that we'd have to request the device driver to stop DMA
before we can destroy the context and free the PASID. We did consider
doing this in the release() MMU notifier, but there were concerns about
blocking mmput() for too long (for example
though I think there was a more recent discussion). We also need to drain
the PRI and fault queues to get rid of all references to that PASID.
Is the concern truly about blocking mmput() itself? Or, is it about
releasing the resources associated with the mm?
The latter I think, this one was about releasing pages as fast as possible
if the process is picked by the OOM killer.

We're tying the PASID to the life of the mm itself, not the mm's address
space. That means the PASID should be tied to

The address space is what the OOM killer is after. That gets refcounted
with mmget()/mmput()/mm->mm_users. The OOM killer is satiated by the
page freeing done in __mmput()->exit_mmap().

Also, all the VMAs should be gone after exit_mmap(). So, even if
vma->vm_file was holding a reference to a device driver, that reference
should be gone by the time __mmdrop() is actually freeing the PASID.

I agree with all that. The concern was about tearing down the PASID in the
IOMMU and device from the release() MMU notifier, which would happen in
exit_mmap(). But doing the teardown at or before __mmdrop() is fine. And
since the IOMMU drivers need to hold mm->mm_count anyway between bind()
and unbind(), I think Fenghua's fix works.

But I didn't find mmgrab()/mmdrop() get called in both arm and intel
IOMMU drivers.

$ git grep mmgrab drivers/iommu/
[no output]

Do we need to add these in a separated fix patch, or I missed anything

Best regards,