Re: [PATCH 01/55] drm/i915: Reduce breadcrumb lock coverage for intel_engine_enable_signaling()

Joonas Lahtinen
2016-07-26T05:07:56Z

On ma, 2016-07-25 at 18:31 +0100, Chris Wilson wrote:
> Since intel_engine_enable_signaling() is now only called via
> fence_enable_sw_signaling(), we can rely on it to provide serialisation
> and run-once for us and so make ourselves slightly simpler.
>

Done in previous series, copying here;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Link: http://patchwork.freedesktop.org/patch/msgid/1469432687-22756-2-git-send-email-chris@chris-wilson.co.uk
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_breadcrumbs.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> index d893ccdd62ac..90867446f1a5 100644
> --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
> +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> @@ -480,19 +480,15 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request)
>   struct rb_node *parent, **p;
>   bool first, wakeup;
>
> - if (unlikely(READ_ONCE(request->signaling.wait.tsk)))
> - return;
> -
> - spin_lock(&b->lock);
> - if (unlikely(request->signaling.wait.tsk)) {
> - wakeup = false;
> - goto unlock;
> - }
> + /* locked by fence_enable_sw_signaling() */
> + assert_spin_locked(&request->lock);
>
>   request->signaling.wait.tsk = b->signaler;
>   request->signaling.wait.seqno = request->fence.seqno;
>   i915_gem_request_get(request);
>
> + spin_lock(&b->lock);
> +
>   /* First add ourselves into the list of waiters, but register our
>    * bottom-half as the signaller thread. As per usual, only the oldest
>    * waiter (not just signaller) is tasked as the bottom-half waking
> @@ -525,7 +521,6 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request)
>   if (first)
>   smp_store_mb(b->first_signal, request);
>
> -unlock:
>   spin_unlock(&b->lock);
>
>   if (wakeup)
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation