drm: fix call_kern.cocci warnings
diff mbox series

Message ID 20181025083634.7178-1-david1.zhou@amd.com
State New
Headers show
Series
  • drm: fix call_kern.cocci warnings
Related show

Commit Message

Zhou, David(ChunMing) Oct. 25, 2018, 8:36 a.m. UTC
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

Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: Christian König <easy2remember.chk@googlemail.com>
---
 drivers/gpu/drm/drm_syncobj.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Daniel Vetter Oct. 25, 2018, 9:35 a.m. UTC | #1
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@amd.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: Christian König <easy2remember.chk@googlemail.com>
> ---
>  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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patch
diff mbox series

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;
 }