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 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
