Am 25.10.18 um 11:35 schrieb Daniel Vetter:
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?

Who says we do any internal validation? ;)

But kidding aside, this code path is only taken after garbage collection has already cleaned up the original fence and we need to return a dummy.

I think we just never covered that in the testing.

Christian.


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 <[email protected]>
Cc: Maarten Lankhorst <[email protected]>
Cc: [email protected]
Cc: Christian König <[email protected]>
---
  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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to