[Intel-gfx] [PATCH] drm: fix call_kern.cocci warnings v2

[Christian König]

Am 25.10.18 um 10:53 schrieb Chunming Zhou:
> 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.
>
> 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

That comment is no longer true.

>
> v2:
> syncobj->timeline still needs protect.
>
> Signed-off-by: Chunming Zhou <david1.zhou at amd.com>
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Cc: intel-gfx at lists.freedesktop.org
> Cc: Christian König <easy2remember.chk at googlemail.com>
> ---
>   drivers/gpu/drm/drm_syncobj.c | 25 +++++++++++++------------
>   1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index b7eaa603f368..b843d4c2778c 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -111,15 +111,16 @@ static struct dma_fence
>   				      uint64_t point)
>   {
>   	struct drm_syncobj_signal_pt *signal_pt;
> +	struct dma_fence *f = NULL;
> +	struct drm_syncobj_stub_fence *fence =
> +		kzalloc(sizeof(struct drm_syncobj_stub_fence),
> +			GFP_KERNEL);
>   
> +	if (!fence)
> +		return NULL;
> +	spin_lock(&syncobj->pt_lock);

How about using a single static stub fence like I suggested?

Christian.

>   	if ((syncobj->type == DRM_SYNCOBJ_TYPE_TIMELINE) &&
>   	    (point <= syncobj->timeline)) {
> -		struct drm_syncobj_stub_fence *fence =
> -			kzalloc(sizeof(struct drm_syncobj_stub_fence),
> -				GFP_KERNEL);
> -
> -		if (!fence)
> -			return NULL;
>   		spin_lock_init(&fence->lock);
>   		dma_fence_init(&fence->base,
>   			       &drm_syncobj_stub_fence_ops,
> @@ -128,6 +129,7 @@ static struct dma_fence
>   			       point);
>   
>   		dma_fence_signal(&fence->base);
> +		spin_unlock(&syncobj->pt_lock);
>   		return &fence->base;
>   	}
>   
> @@ -137,9 +139,12 @@ static struct dma_fence
>   		if ((syncobj->type == DRM_SYNCOBJ_TYPE_BINARY) &&
>   		    (point != signal_pt->value))
>   			continue;
> -		return dma_fence_get(&signal_pt->fence_array->base);
> +		f = dma_fence_get(&signal_pt->fence_array->base);
> +		break;
>   	}
> -	return NULL;
> +	spin_unlock(&syncobj->pt_lock);
> +	kfree(fence);
> +	return f;
>   }
>   
>   static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj,
> @@ -166,9 +171,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 +382,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;
>   }
>