Re: [PATCH 1/3] drm/i915/userptr: Flush cancellations before mmu-notifier invalidate returns

From: Chris Wilson <chris@chris-wilson.co.uk>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/3] drm/i915/userptr: Flush cancellations before mmu-notifier invalidate returns
Date: Tue, 5 Apr 2016 13:34:02 +0100	[thread overview]
Message-ID: <20160405123402.GK24026@nuc-i3427.alporthouse.com> (raw)
In-Reply-To: <5703A9F1.80805@linux.intel.com>

On Tue, Apr 05, 2016 at 01:05:05PM +0100, Tvrtko Ursulin wrote:
> 
> On 03/04/16 18:14, Chris Wilson wrote:
> >In order to ensure that all invalidations are completed before the
> >operation returns to userspace (i.e. before the mmap() syscall returns)
> >we need to flush the outstanding operations. To this end, we submit all
> >the per-object invalidation work to a private workqueue and then flush
> >the workqueue at the end of the function. We are allowed to block inside
> >invalidate_range_start, and struct_mutex is the inner lock so the worker
> >is not hiding any lock inversions. The advantage of using the workqueue
> >over direct calling of cancel_userptr() is that it allows us to
> >parallelise the potential waits and unpinning of large objects.
> 
> There was no direct calling, but running it from a shared wq which
> is now private, per task->mm.

I meant keep using a wq as opposed to changing the invalidate_range_begin to
call cancel_userptr() directly.

> But it sounds so obvious now that the mmu notifier needs to wait for
> cancel_userptr workers to complete that I wonder how we missed that
> until now.
> 
> I would also spell out the two new bits explicitly in the commit:
> waiting for workers and waiting on rendering.

Ok, deleted a couple of attempts already. Will get a cup of tea and try
and write something eloquent.

> With some more description for the latter - why it is needed?
> i915_vma_unbind would be doing a flavour of waiting already.

It allows us to drop the lock whilst waiting for outstanding rendering,
to try and prevent contention between munmap in one process and execbuf
in another. Later on we can reduce the lock here further.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx