[Intel-gfx] [PATCH 21/22] drm/i915: Enable lockless lookup of request tracking via RCU

[Chris Wilson]

On Fri, Jul 29, 2016 at 09:49:54AM +0100, Chris Wilson wrote:
> On Fri, Jul 29, 2016 at 10:41:14AM +0200, Daniel Vetter wrote:
> > I guess it doesn't hurt to make this really, really clear. Perfect! Well
> > almost, one nit:
> > 
> > > 
> > >                 /* What stops the following rcu_dereference() from occuring
> > >                  * before the above i915_gem_request_get_rcu()? If we were
> > >                  * to read the value before pausing to get the reference to
> > >                  * the request, we may not notice a change in the active
> > >                  * tracker.
> > >                  *
> > >                  * The rcu_dereference() is a mere read barrier, which means
> > 
> > s/read barrier/barrier of depending reads/, rcu_dereference is not even a
> > full rmb!
> > 
> > >                  * that operations after it will appear after, neither the
> > 
> > hence also: s/operations/any operations through the read pointer/
> 
> Ah right, that needs to be dependent reads. Changes look good.


       do {
                struct drm_i915_gem_request *request;

                request = rcu_dereference(active->request);
                if (!request || i915_gem_request_completed(request))
                        return NULL;

                request = i915_gem_request_get_rcu(request);

                /* What stops the following rcu_access_pointer() from occurring
                 * before the above i915_gem_request_get_rcu()? If we were
                 * to read the value before pausing to get the reference to
                 * the request, we may not notice a change in the active
                 * tracker.
                 *
                 * The rcu_access_pointer() is a mere compiler barrier, which
                 * means both the CPU and compiler are free to perform the
                 * memory read without constraint. The compiler only has to
                 * ensure that any operations after the rcu_access_pointer()
                 * occur afterwards in program order. This means the read may
                 * be performed earlier by an out-of-order CPU, or adventurous
                 * compiler.
                 *
                 * The atomic operation at the heart of
                 * i915_gem_request_get_rcu(), see fence_get_rcu(), is
                 * atomic_inc_not_zero() which is only a full memory barrier
                 * when successful. That is, if i915_gem_request_get_rcu()
                 * returns the request (and so with the reference counted
                 * incremented) then the following read for rcu_access_pointer()
                 * must occur after the atomic operation and so confirm
                 * that this request is the one currently being tracked.
                 */
                if (!request || request == rcu_access_pointer(active->request))
                        return rcu_pointer_handoff(request);

                i915_gem_request_put(request);
        } while (1);


-- 
Chris Wilson, Intel Open Source Technology Centre