Re: [PATCH RFC 2/4] drm/i915: IOMMU based SVM implementation v13

David Woodhouse
2016-08-15T12:30:15Z

[-- Attachment #1.1: Type: text/plain, Size: 1469 bytes --]

On Mon, 2016-08-15 at 13:23 +0100, Chris Wilson wrote:
> On Mon, Aug 15, 2016 at 01:13:25PM +0100, David Woodhouse wrote:
> > On Mon, 2016-08-15 at 13:05 +0100, Chris Wilson wrote:
> > > On Mon, Aug 15, 2016 at 02:48:05PM +0300, Mika Kuoppala wrote:
> > > >
> > > > + struct task_struct *task;
> > >
> > > We don't need the task, we need the mm.
> > >
> > > Holding the task is not sufficient.
> >
> > From the pure DMA point of view, you don't need the MM at all. I handle
> > all that from the IOMMU side so it's none of your business, darling.
>
> But you don't keep the mm alive for the duration of device activity,
> right? And you don't wait for the device to finish before releasing the
> mmu? (iiuc intel-svm.c)

We don't "keep it alive" (i.e. bump mm->mm_users), no.
We *did*, but it caused problems. See commit e57e58bd390a68 for the
gory details.

Now we only bump mm->mm_count so if the process exits, the MM can still
be torn down.

Since exit_mmap() happens before exit_files(), what happens on an
unclean shutdown is that the GPU may start to take faults on the PASID
which is in the process of exiting, before the corresponding file
descriptor gets closed.

So no, we don't wait for the device to finish before releasing the MM.
That would involve calling back into device-driver code from the
mmu_notifier callback, with "interesting" locking constraints. We don't
trust device drivers that much :)

--
dwmw2

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5760 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]