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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
