[Intel-gfx] [PATCH] RFC drm/i915: Remove (struct_mutex) locking for wait-ioctl

[Chris Wilson]

On Wed, Dec 23, 2015 at 11:19:23AM +0000, Chris Wilson wrote:
> With a bit of care (and leniency) we can iterate over the object and
> wait for previous rendering to complete with judicial use of atomic
> reference counting. The ABI requires us to ensure that an active object
> is eventually flushed (like the busy-ioctl) which is guaranteed by our
> management of requests (i.e. everything that is submitted to hardware is
> flushed in the same request). All we have to do is ensure that we can
> detect when the requests are complete for reporting when the object is
> idle (without triggering ETIME) - this is handled by
> __i915_wait_request.
> 
> The biggest danger in the code is walking the object without holding any
> locks. We iterate over the set of last requests and carefully grab a
> reference upon it. (If it is changing beneath us, that is the usual
> userspace race and even with locking you get the same indeterminate
> results.) If the request is unreferenced beneath us, it will be disposed
> of into the request cache - so we have to carefully order the retrieval
> of the request pointer with its removal.
> 
> The impact of this is actually quite small - the return to userspace
> following the wait was already lockless. What we achieve here is
> completing an already finished wait without hitting the struct_mutex,
> our hold is quite short and so we are typically just a victim of
> contention rather than a cause.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

Some food for thought. I would especially like someone to poke holes in
the racy pointer lookup and check the store mb() versus the
rcu-reference.

The same technique would also apply to busy-ioctl, and that's about the
limits of its applicablity. Though isolating those from contention and
having to do a flush-active should be beneficial system-wide.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre