On 2018年10月25日 17:38, Christian König wrote:
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? ;)
It's my mistake, I switched to new kernel and didn't check if these
DEBUG options are enabled or not.
I shouldn't take this kind mistake, anyway.
Sorry for that,
David
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 <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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
