Re: [PATCH] drm: fix deadlock of syncobj v3

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Chunming Zhou (2018-10-22 10:08:59)
> v2:
> add a mutex between sync_cb execution and free.
> v3:
> clearly separating the roles for pt_lock and cb_mutex (Chris)
> 
> Signed-off-by: Chunming Zhou <david1.zhou@xxxxxxx>
> Cc: Daniel Vetter <daniel@xxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Christian König <christian.koenig@xxxxxxx>
> ---
>  drivers/gpu/drm/drm_syncobj.c | 51 ++++++++++++++++++-----------------
>  include/drm/drm_syncobj.h     |  8 ++++--
>  2 files changed, 33 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 57bf6006394d..827d2ea5bb79 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -125,23 +125,24 @@ static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
>         if (!ret)
>                 return 1;
>  
> -       spin_lock(&syncobj->lock);
> +       spin_lock(&syncobj->pt_lock);
>         /* 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.
>          */
>         if (!list_empty(&syncobj->signal_pt_list)) {
> -               spin_unlock(&syncobj->lock);
> +               spin_unlock(&syncobj->pt_lock);
>                 drm_syncobj_search_fence(syncobj, 0, 0, fence);
>                 if (*fence)
>                         return 1;
> -               spin_lock(&syncobj->lock);
>         } else {
> +               spin_unlock(&syncobj->pt_lock);
>                 *fence = NULL;
> +               mutex_lock(&syncobj->cb_mutex);
>                 drm_syncobj_add_callback_locked(syncobj, cb, func);
> +               mutex_unlock(&syncobj->cb_mutex);

There's no guard should the fence be installed after our check but before
we add the callback. Aiui, the cb_mutex should be taken outside of the
pt_lock around this if() block.

>                 ret = 0;
>         }
> -       spin_unlock(&syncobj->lock);
>  
>         return ret;
>  }
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux