[Intel-gfx] [PATCH 18/21] drm/i915: Embed signaling node into the GEM request

[Chris Wilson]

On Tue, Jun 07, 2016 at 01:31:25PM +0100, Tvrtko Ursulin wrote:
> >@@ -418,20 +423,23 @@ int intel_engine_enable_signaling(struct drm_i915_gem_request *request)
> >  	struct intel_engine_cs *engine = request->engine;
> >  	struct intel_breadcrumbs *b = &engine->breadcrumbs;
> >  	struct rb_node *parent, **p;
> >-	struct signal *signal;
> >  	bool first, wakeup;
> >
> >  	if (unlikely(IS_ERR(b->signaler)))
> >  		return PTR_ERR(b->signaler);
> >
> >-	signal = kmalloc(sizeof(*signal), GFP_ATOMIC);
> >-	if (unlikely(!signal))
> >-		return -ENOMEM;
> >+	if (unlikely(READ_ONCE(request->signaling.wait.task)))
> >+		return 0;
> 
> Hmm it will depend on following patches whether this is safe. I
> don't like the explosion of READ_ONCE and smp_store_mb's in these
> patches. Something is bound to be broken.

This one is trivial :)
 
> You even check it below under the lock. So I am not sure this
> optimisation is worth it. Maybe leave it for later?

I was just worrying about contention since the breadcrumbs lock is being
used to guard both trees and contention in the waiters is noticeable.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre