[Intel-gfx] [PATCH] drm: fix deadlock of syncobj v5

[Chris Wilson]

Quoting Chunming Zhou (2018-10-23 08:57:54)
> v2:
> add a mutex between sync_cb execution and free.
> v3:
> clearly separating the roles for pt_lock and cb_mutex (Chris)
> v4:
> the cb_mutex should be taken outside of the pt_lock around this if() block. (Chris)
> v5:
> fix a corner case
> 
> Tested by syncobj_basic and syncobj_wait of igt.
> 
> Signed-off-by: Chunming Zhou <david1.zhou at amd.com>
> Cc: Daniel Vetter <daniel at ffwll.ch>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Christian König <christian.koenig at amd.com>
> Cc: intel-gfx at lists.freedesktop.org
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/drm_syncobj.c | 55 +++++++++++++++++++----------------
>  include/drm/drm_syncobj.h     |  8 +++--
>  2 files changed, 36 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 57bf6006394d..679a56791e34 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -125,23 +125,26 @@ static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
>         if (!ret)
>                 return 1;
>  
> -       spin_lock(&syncobj->lock);
> +       mutex_lock(&syncobj->cb_mutex);
>         /* We've already tried once to get a fence and failed.  Now that we
>          * have the lock, try one more time just to be sure we don't add a
>          * callback when a fence has already been set.
>          */
> +       spin_lock(&syncobj->pt_lock);
>         if (!list_empty(&syncobj->signal_pt_list)) {
> -               spin_unlock(&syncobj->lock);
> +               spin_unlock(&syncobj->pt_lock);
>                 drm_syncobj_search_fence(syncobj, 0, 0, fence);

Hmm, just thinking of other ways of tidying this up

mutex_lock(cb_lock);
spin_lock(pt_lock);
*fence = drm_syncobj_find_signal_pt_for_point();
spin_unlock(pt_list);
if (*!fence)
	drm_syncobj_add_callback_locked(syncobj, cb, func);
mutex_unlock(cb_lock);

i.e. get rid of the early return and we can even drop the int return here
as it is unimportant and unused.
-Chris