[Intel-gfx] [PATCH] drm/i915/breadcrumbs: Drop request reference for the signaler thread

[Chris Wilson]

Quoting Chris Wilson (2018-01-31 17:30:46)
> Quoting Tvrtko Ursulin (2018-01-31 17:18:55)
> > 
> > On 22/01/2018 15:41, Chris Wilson wrote:
> > > +static struct drm_i915_gem_request *first_signal(struct intel_breadcrumbs *b)
> > > +{
> > > +     /*
> > > +      * See the big warnings for i915_gem_active_get_rcu() and similarly
> > > +      * for dma_fence_get_rcu_safe() that explain the intricacies involved
> > > +      * here with defeating CPU/compiler speculation and enforcing
> > > +      * the required memory barriers.
> > > +      */
> > > +     do {
> > > +             struct drm_i915_gem_request *request;
> > > +
> > > +             request = rcu_dereference(b->first_signal);
> > > +             if (request)
> > > +                     request = i915_gem_request_get_rcu(request);
> > > +
> > > +             barrier();
> > > +
> > > +             if (!request || request == rcu_access_pointer(b->first_signal))
> > > +                     return rcu_pointer_handoff(request);
> > > +
> > > +             i915_gem_request_put(request);
> > > +     } while (1);
> > > +}
> > > +
> > >   static int intel_breadcrumbs_signaler(void *arg)
> > >   {
> > >       struct intel_engine_cs *engine = arg;
> > > @@ -667,41 +715,21 @@ static int intel_breadcrumbs_signaler(void *arg)
> > >                * a new client.
> > >                */
> > >               rcu_read_lock();
> > > -             request = rcu_dereference(b->first_signal);
> > > -             if (request)
> > > -                     request = i915_gem_request_get_rcu(request);
> > > +             request = first_signal(b);
> > 
> > get_ prefix would be good to signify a get vs peek. Maybe even _rcu suffix.
> 
> Sold.

Whilst you are looking at this, I should just say that first_signal() is
what we should have been doing all this time; it's really a very obscure
bug fix. And fwiw, the s/rbtree/list/ patch eliminates it.
-Chris