On Thu, Oct 25, 2018 at 04:36:34PM +0800, Chunming Zhou wrote:
> drivers/gpu/drm/drm_syncobj.c:202:4-14: ERROR: function drm_syncobj_find_signal_pt_for_point called on line 390 inside lock on line 389 but uses GFP_KERNEL
>
> Find functions that refer to GFP_KERNEL but are called with locks held.
Uh ... if your internal validation doesn't catch stuff like this (boils
down to a) run code b) with sleep debugging enabled) what exactly do you
do?
Not really inspiring confindence.
-Daniel
>
> Semantic patch information:
> The proposed change of converting the GFP_KERNEL is not necessarily the
> correct one. It may be desired to unlock the lock, or to not call the
> function under the lock in the first place.
>
> Generated by: scripts/coccinelle/locks/call_kern.cocci
>
> Signed-off-by: Chunming Zhou <david1.zhou@xxxxxxx>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Christian König <easy2remember.chk@xxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_syncobj.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index b7eaa603f368..c9099549ddcb 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -111,6 +111,7 @@ static struct dma_fence
> uint64_t point)
> {
> struct drm_syncobj_signal_pt *signal_pt;
> + struct dma_fence *fence = NULL;
>
> if ((syncobj->type == DRM_SYNCOBJ_TYPE_TIMELINE) &&
> (point <= syncobj->timeline)) {
> @@ -131,15 +132,18 @@ static struct dma_fence
> return &fence->base;
> }
>
> + spin_lock(&syncobj->pt_lock);
> list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) {
> if (point > signal_pt->value)
> continue;
> if ((syncobj->type == DRM_SYNCOBJ_TYPE_BINARY) &&
> (point != signal_pt->value))
> continue;
> - return dma_fence_get(&signal_pt->fence_array->base);
> + fence = dma_fence_get(&signal_pt->fence_array->base);
> + break;
> }
> - return NULL;
> + spin_unlock(&syncobj->pt_lock);
> + return fence;
> }
>
> static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj,
> @@ -166,9 +170,7 @@ static void drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
> }
>
> mutex_lock(&syncobj->cb_mutex);
> - spin_lock(&syncobj->pt_lock);
> *fence = drm_syncobj_find_signal_pt_for_point(syncobj, pt_value);
> - spin_unlock(&syncobj->pt_lock);
> if (!*fence)
> drm_syncobj_add_callback_locked(syncobj, cb, func);
> mutex_unlock(&syncobj->cb_mutex);
> @@ -379,11 +381,9 @@ drm_syncobj_point_get(struct drm_syncobj *syncobj, u64 point, u64 flags,
> if (ret < 0)
> return ret;
> }
> - spin_lock(&syncobj->pt_lock);
> *fence = drm_syncobj_find_signal_pt_for_point(syncobj, point);
> if (!*fence)
> ret = -EINVAL;
> - spin_unlock(&syncobj->pt_lock);
> return ret;
> }
>
> --
> 2.17.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
