* [PATCH] drm: fix deadlock of syncobj v5
@ 2018-10-23 7:57 Chunming Zhou
2018-10-23 9:01 ` Chris Wilson
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Chunming Zhou @ 2018-10-23 7:57 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Christian König
v2:
add a mutex between sync_cb execution and free.
v3:
clearly separating the roles for pt_lock and cb_mutex (Chris)
v4:
the cb_mutex should be taken outside of the pt_lock around this if() block. (Chris)
v5:
fix a corner case
Tested by syncobj_basic and syncobj_wait of igt.
Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Christian König <christian.koenig@amd.com>
Cc: intel-gfx@lists.freedesktop.org
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/drm_syncobj.c | 55 +++++++++++++++++++----------------
include/drm/drm_syncobj.h | 8 +++--
2 files changed, 36 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 57bf6006394d..679a56791e34 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -125,23 +125,26 @@ static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
if (!ret)
return 1;
- spin_lock(&syncobj->lock);
+ mutex_lock(&syncobj->cb_mutex);
/* We've already tried once to get a fence and failed. Now that we
* have the lock, try one more time just to be sure we don't add a
* callback when a fence has already been set.
*/
+ spin_lock(&syncobj->pt_lock);
if (!list_empty(&syncobj->signal_pt_list)) {
- spin_unlock(&syncobj->lock);
+ spin_unlock(&syncobj->pt_lock);
drm_syncobj_search_fence(syncobj, 0, 0, fence);
- if (*fence)
+ if (*fence) {
+ mutex_unlock(&syncobj->cb_mutex);
return 1;
- spin_lock(&syncobj->lock);
+ }
} else {
+ spin_unlock(&syncobj->pt_lock);
*fence = NULL;
drm_syncobj_add_callback_locked(syncobj, cb, func);
ret = 0;
}
- spin_unlock(&syncobj->lock);
+ mutex_unlock(&syncobj->cb_mutex);
return ret;
}
@@ -150,43 +153,43 @@ void drm_syncobj_add_callback(struct drm_syncobj *syncobj,
struct drm_syncobj_cb *cb,
drm_syncobj_func_t func)
{
- spin_lock(&syncobj->lock);
+ mutex_lock(&syncobj->cb_mutex);
drm_syncobj_add_callback_locked(syncobj, cb, func);
- spin_unlock(&syncobj->lock);
+ mutex_unlock(&syncobj->cb_mutex);
}
void drm_syncobj_remove_callback(struct drm_syncobj *syncobj,
struct drm_syncobj_cb *cb)
{
- spin_lock(&syncobj->lock);
+ mutex_lock(&syncobj->cb_mutex);
list_del_init(&cb->node);
- spin_unlock(&syncobj->lock);
+ mutex_unlock(&syncobj->cb_mutex);
}
static void drm_syncobj_init(struct drm_syncobj *syncobj)
{
- spin_lock(&syncobj->lock);
+ spin_lock(&syncobj->pt_lock);
syncobj->timeline_context = dma_fence_context_alloc(1);
syncobj->timeline = 0;
syncobj->signal_point = 0;
init_waitqueue_head(&syncobj->wq);
INIT_LIST_HEAD(&syncobj->signal_pt_list);
- spin_unlock(&syncobj->lock);
+ spin_unlock(&syncobj->pt_lock);
}
static void drm_syncobj_fini(struct drm_syncobj *syncobj)
{
struct drm_syncobj_signal_pt *signal_pt = NULL, *tmp;
- spin_lock(&syncobj->lock);
+ spin_lock(&syncobj->pt_lock);
list_for_each_entry_safe(signal_pt, tmp,
&syncobj->signal_pt_list, list) {
list_del(&signal_pt->list);
dma_fence_put(&signal_pt->fence_array->base);
kfree(signal_pt);
}
- spin_unlock(&syncobj->lock);
+ spin_unlock(&syncobj->pt_lock);
}
static struct dma_fence
@@ -249,14 +252,14 @@ static int drm_syncobj_create_signal_pt(struct drm_syncobj *syncobj,
fences[num_fences++] = dma_fence_get(fence);
/* timeline syncobj must take this dependency */
if (syncobj->type == DRM_SYNCOBJ_TYPE_TIMELINE) {
- spin_lock(&syncobj->lock);
+ spin_lock(&syncobj->pt_lock);
if (!list_empty(&syncobj->signal_pt_list)) {
tail_pt = list_last_entry(&syncobj->signal_pt_list,
struct drm_syncobj_signal_pt, list);
fences[num_fences++] =
dma_fence_get(&tail_pt->fence_array->base);
}
- spin_unlock(&syncobj->lock);
+ spin_unlock(&syncobj->pt_lock);
}
signal_pt->fence_array = dma_fence_array_create(num_fences, fences,
syncobj->timeline_context,
@@ -266,16 +269,16 @@ static int drm_syncobj_create_signal_pt(struct drm_syncobj *syncobj,
goto fail;
}
- spin_lock(&syncobj->lock);
+ spin_lock(&syncobj->pt_lock);
if (syncobj->signal_point >= point) {
DRM_WARN("A later signal is ready!");
- spin_unlock(&syncobj->lock);
+ spin_unlock(&syncobj->pt_lock);
goto exist;
}
signal_pt->value = point;
list_add_tail(&signal_pt->list, &syncobj->signal_pt_list);
syncobj->signal_point = point;
- spin_unlock(&syncobj->lock);
+ spin_unlock(&syncobj->pt_lock);
wake_up_all(&syncobj->wq);
return 0;
@@ -294,7 +297,7 @@ static void drm_syncobj_garbage_collection(struct drm_syncobj *syncobj)
{
struct drm_syncobj_signal_pt *signal_pt, *tmp, *tail_pt;
- spin_lock(&syncobj->lock);
+ spin_lock(&syncobj->pt_lock);
tail_pt = list_last_entry(&syncobj->signal_pt_list,
struct drm_syncobj_signal_pt,
list);
@@ -315,7 +318,7 @@ static void drm_syncobj_garbage_collection(struct drm_syncobj *syncobj)
}
}
- spin_unlock(&syncobj->lock);
+ spin_unlock(&syncobj->pt_lock);
}
/**
* drm_syncobj_replace_fence - replace fence in a sync object.
@@ -344,13 +347,14 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj,
drm_syncobj_create_signal_pt(syncobj, fence, pt_value);
if (fence) {
struct drm_syncobj_cb *cur, *tmp;
+ LIST_HEAD(cb_list);
- spin_lock(&syncobj->lock);
+ mutex_lock(&syncobj->cb_mutex);
list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) {
list_del_init(&cur->node);
cur->func(syncobj, cur);
}
- spin_unlock(&syncobj->lock);
+ mutex_unlock(&syncobj->cb_mutex);
}
}
EXPORT_SYMBOL(drm_syncobj_replace_fence);
@@ -386,11 +390,11 @@ drm_syncobj_point_get(struct drm_syncobj *syncobj, u64 point, u64 flags,
if (ret < 0)
return ret;
}
- spin_lock(&syncobj->lock);
+ spin_lock(&syncobj->pt_lock);
*fence = drm_syncobj_find_signal_pt_for_point(syncobj, point);
if (!*fence)
ret = -EINVAL;
- spin_unlock(&syncobj->lock);
+ spin_unlock(&syncobj->pt_lock);
return ret;
}
@@ -500,7 +504,8 @@ int drm_syncobj_create(struct drm_syncobj **out_syncobj, uint32_t flags,
kref_init(&syncobj->refcount);
INIT_LIST_HEAD(&syncobj->cb_list);
- spin_lock_init(&syncobj->lock);
+ spin_lock_init(&syncobj->pt_lock);
+ mutex_init(&syncobj->cb_mutex);
if (flags & DRM_SYNCOBJ_CREATE_TYPE_TIMELINE)
syncobj->type = DRM_SYNCOBJ_TYPE_TIMELINE;
else
diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
index 5e8c5c027e09..29244cbcd05e 100644
--- a/include/drm/drm_syncobj.h
+++ b/include/drm/drm_syncobj.h
@@ -75,9 +75,13 @@ struct drm_syncobj {
*/
struct list_head cb_list;
/**
- * @lock: Protects syncobj list and write-locks &fence.
+ * @pt_lock: Protects pt list.
*/
- spinlock_t lock;
+ spinlock_t pt_lock;
+ /**
+ * @cb_mutex: Protects syncobj cb list.
+ */
+ struct mutex cb_mutex;
/**
* @file: A file backing for this syncobj.
*/
--
2.17.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: fix deadlock of syncobj v5
2018-10-23 7:57 [PATCH] drm: fix deadlock of syncobj v5 Chunming Zhou
@ 2018-10-23 9:01 ` Chris Wilson
2018-10-23 9:09 ` zhoucm1
2018-10-23 9:17 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2018-10-23 9:01 UTC (permalink / raw)
To: Chunming Zhou, dri-devel; +Cc: intel-gfx, Christian König
Quoting Chunming Zhou (2018-10-23 08:57:54)
> v2:
> add a mutex between sync_cb execution and free.
> v3:
> clearly separating the roles for pt_lock and cb_mutex (Chris)
> v4:
> the cb_mutex should be taken outside of the pt_lock around this if() block. (Chris)
> v5:
> fix a corner case
>
> Tested by syncobj_basic and syncobj_wait of igt.
>
> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: intel-gfx@lists.freedesktop.org
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/drm_syncobj.c | 55 +++++++++++++++++++----------------
> include/drm/drm_syncobj.h | 8 +++--
> 2 files changed, 36 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 57bf6006394d..679a56791e34 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -125,23 +125,26 @@ static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
> if (!ret)
> return 1;
>
> - spin_lock(&syncobj->lock);
> + mutex_lock(&syncobj->cb_mutex);
> /* We've already tried once to get a fence and failed. Now that we
> * have the lock, try one more time just to be sure we don't add a
> * callback when a fence has already been set.
> */
> + spin_lock(&syncobj->pt_lock);
> if (!list_empty(&syncobj->signal_pt_list)) {
> - spin_unlock(&syncobj->lock);
> + spin_unlock(&syncobj->pt_lock);
> drm_syncobj_search_fence(syncobj, 0, 0, fence);
Hmm, just thinking of other ways of tidying this up
mutex_lock(cb_lock);
spin_lock(pt_lock);
*fence = drm_syncobj_find_signal_pt_for_point();
spin_unlock(pt_list);
if (*!fence)
drm_syncobj_add_callback_locked(syncobj, cb, func);
mutex_unlock(cb_lock);
i.e. get rid of the early return and we can even drop the int return here
as it is unimportant and unused.
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: fix deadlock of syncobj v5
2018-10-23 9:01 ` Chris Wilson
@ 2018-10-23 9:09 ` zhoucm1
2018-10-23 9:11 ` Chris Wilson
0 siblings, 1 reply; 9+ messages in thread
From: zhoucm1 @ 2018-10-23 9:09 UTC (permalink / raw)
To: Chris Wilson, Chunming Zhou, dri-devel; +Cc: intel-gfx, Christian König
On 2018年10月23日 17:01, Chris Wilson wrote:
> Quoting Chunming Zhou (2018-10-23 08:57:54)
>> v2:
>> add a mutex between sync_cb execution and free.
>> v3:
>> clearly separating the roles for pt_lock and cb_mutex (Chris)
>> v4:
>> the cb_mutex should be taken outside of the pt_lock around this if() block. (Chris)
>> v5:
>> fix a corner case
>>
>> Tested by syncobj_basic and syncobj_wait of igt.
>>
>> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: intel-gfx@lists.freedesktop.org
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>> drivers/gpu/drm/drm_syncobj.c | 55 +++++++++++++++++++----------------
>> include/drm/drm_syncobj.h | 8 +++--
>> 2 files changed, 36 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>> index 57bf6006394d..679a56791e34 100644
>> --- a/drivers/gpu/drm/drm_syncobj.c
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -125,23 +125,26 @@ static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
>> if (!ret)
>> return 1;
>>
>> - spin_lock(&syncobj->lock);
>> + mutex_lock(&syncobj->cb_mutex);
>> /* We've already tried once to get a fence and failed. Now that we
>> * have the lock, try one more time just to be sure we don't add a
>> * callback when a fence has already been set.
>> */
>> + spin_lock(&syncobj->pt_lock);
>> if (!list_empty(&syncobj->signal_pt_list)) {
>> - spin_unlock(&syncobj->lock);
>> + spin_unlock(&syncobj->pt_lock);
>> drm_syncobj_search_fence(syncobj, 0, 0, fence);
> Hmm, just thinking of other ways of tidying this up
>
> mutex_lock(cb_lock);
> spin_lock(pt_lock);
> *fence = drm_syncobj_find_signal_pt_for_point();
> spin_unlock(pt_list);
> if (*!fence)
> drm_syncobj_add_callback_locked(syncobj, cb, func);
> mutex_unlock(cb_lock);
>
> i.e. get rid of the early return and we can even drop the int return here
> as it is unimportant and unused.
Yes, do you need I send v6? or you make a separate patch as a improvment?
Thanks,
David Zhou
> -Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: fix deadlock of syncobj v5
2018-10-23 9:09 ` zhoucm1
@ 2018-10-23 9:11 ` Chris Wilson
2018-10-23 9:27 ` Koenig, Christian
0 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2018-10-23 9:11 UTC (permalink / raw)
To: Chunming Zhou, dri-devel, zhoucm1; +Cc: intel-gfx, Christian König
Quoting zhoucm1 (2018-10-23 10:09:01)
>
>
> On 2018年10月23日 17:01, Chris Wilson wrote:
> > Quoting Chunming Zhou (2018-10-23 08:57:54)
> >> v2:
> >> add a mutex between sync_cb execution and free.
> >> v3:
> >> clearly separating the roles for pt_lock and cb_mutex (Chris)
> >> v4:
> >> the cb_mutex should be taken outside of the pt_lock around this if() block. (Chris)
> >> v5:
> >> fix a corner case
> >>
> >> Tested by syncobj_basic and syncobj_wait of igt.
> >>
> >> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
> >> Cc: Daniel Vetter <daniel@ffwll.ch>
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Christian König <christian.koenig@amd.com>
> >> Cc: intel-gfx@lists.freedesktop.org
> >> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> ---
> >> drivers/gpu/drm/drm_syncobj.c | 55 +++++++++++++++++++----------------
> >> include/drm/drm_syncobj.h | 8 +++--
> >> 2 files changed, 36 insertions(+), 27 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> >> index 57bf6006394d..679a56791e34 100644
> >> --- a/drivers/gpu/drm/drm_syncobj.c
> >> +++ b/drivers/gpu/drm/drm_syncobj.c
> >> @@ -125,23 +125,26 @@ static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
> >> if (!ret)
> >> return 1;
> >>
> >> - spin_lock(&syncobj->lock);
> >> + mutex_lock(&syncobj->cb_mutex);
> >> /* We've already tried once to get a fence and failed. Now that we
> >> * have the lock, try one more time just to be sure we don't add a
> >> * callback when a fence has already been set.
> >> */
> >> + spin_lock(&syncobj->pt_lock);
> >> if (!list_empty(&syncobj->signal_pt_list)) {
> >> - spin_unlock(&syncobj->lock);
> >> + spin_unlock(&syncobj->pt_lock);
> >> drm_syncobj_search_fence(syncobj, 0, 0, fence);
> > Hmm, just thinking of other ways of tidying this up
> >
> > mutex_lock(cb_lock);
> > spin_lock(pt_lock);
> > *fence = drm_syncobj_find_signal_pt_for_point();
> > spin_unlock(pt_list);
> > if (*!fence)
> > drm_syncobj_add_callback_locked(syncobj, cb, func);
> > mutex_unlock(cb_lock);
> >
> > i.e. get rid of the early return and we can even drop the int return here
> > as it is unimportant and unused.
> Yes, do you need I send v6? or you make a separate patch as a improvment?
Send it in reply, we still have some time before the shards catch up
with the ml ;)
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for drm: fix deadlock of syncobj v5
2018-10-23 7:57 [PATCH] drm: fix deadlock of syncobj v5 Chunming Zhou
2018-10-23 9:01 ` Chris Wilson
@ 2018-10-23 9:17 ` Patchwork
2018-10-23 9:17 ` ✗ Fi.CI.SPARSE: " Patchwork
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-10-23 9:17 UTC (permalink / raw)
To: Chunming Zhou; +Cc: intel-gfx
== Series Details ==
Series: drm: fix deadlock of syncobj v5
URL : https://patchwork.freedesktop.org/series/51365/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
9817489f632a drm: fix deadlock of syncobj v5
-:14: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#14:
the cb_mutex should be taken outside of the pt_lock around this if() block. (Chris)
-:223: CHECK:UNCOMMENTED_DEFINITION: spinlock_t definition without comment
#223: FILE: include/drm/drm_syncobj.h:80:
+ spinlock_t pt_lock;
total: 0 errors, 1 warnings, 1 checks, 186 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* ✗ Fi.CI.SPARSE: warning for drm: fix deadlock of syncobj v5
2018-10-23 7:57 [PATCH] drm: fix deadlock of syncobj v5 Chunming Zhou
2018-10-23 9:01 ` Chris Wilson
2018-10-23 9:17 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2018-10-23 9:17 ` Patchwork
2018-10-23 9:34 ` ✓ Fi.CI.BAT: success " Patchwork
2018-10-23 11:37 ` ✗ Fi.CI.IGT: failure " Patchwork
4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-10-23 9:17 UTC (permalink / raw)
To: Chunming Zhou; +Cc: intel-gfx
== Series Details ==
Series: drm: fix deadlock of syncobj v5
URL : https://patchwork.freedesktop.org/series/51365/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm: fix deadlock of syncobj v5
-O:drivers/gpu/drm/drm_syncobj.c:158:6: warning: symbol 'drm_syncobj_remove_callback' was not declared. Should it be static?
+drivers/gpu/drm/drm_syncobj.c:161:6: warning: symbol 'drm_syncobj_remove_callback' was not declared. Should it be static?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: fix deadlock of syncobj v5
2018-10-23 9:11 ` Chris Wilson
@ 2018-10-23 9:27 ` Koenig, Christian
0 siblings, 0 replies; 9+ messages in thread
From: Koenig, Christian @ 2018-10-23 9:27 UTC (permalink / raw)
To: Chris Wilson, Zhou, David(ChunMing), dri-devel; +Cc: intel-gfx
Am 23.10.18 um 11:11 schrieb Chris Wilson:
> Quoting zhoucm1 (2018-10-23 10:09:01)
>>
>> On 2018年10月23日 17:01, Chris Wilson wrote:
>>> Quoting Chunming Zhou (2018-10-23 08:57:54)
>>>> v2:
>>>> add a mutex between sync_cb execution and free.
>>>> v3:
>>>> clearly separating the roles for pt_lock and cb_mutex (Chris)
>>>> v4:
>>>> the cb_mutex should be taken outside of the pt_lock around this if() block. (Chris)
>>>> v5:
>>>> fix a corner case
>>>>
>>>> Tested by syncobj_basic and syncobj_wait of igt.
>>>>
>>>> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
>>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>>> Cc: Christian König <christian.koenig@amd.com>
>>>> Cc: intel-gfx@lists.freedesktop.org
>>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>>>> ---
>>>> drivers/gpu/drm/drm_syncobj.c | 55 +++++++++++++++++++----------------
>>>> include/drm/drm_syncobj.h | 8 +++--
>>>> 2 files changed, 36 insertions(+), 27 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>>>> index 57bf6006394d..679a56791e34 100644
>>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>>> @@ -125,23 +125,26 @@ static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
>>>> if (!ret)
>>>> return 1;
>>>>
>>>> - spin_lock(&syncobj->lock);
>>>> + mutex_lock(&syncobj->cb_mutex);
>>>> /* We've already tried once to get a fence and failed. Now that we
>>>> * have the lock, try one more time just to be sure we don't add a
>>>> * callback when a fence has already been set.
>>>> */
>>>> + spin_lock(&syncobj->pt_lock);
>>>> if (!list_empty(&syncobj->signal_pt_list)) {
>>>> - spin_unlock(&syncobj->lock);
>>>> + spin_unlock(&syncobj->pt_lock);
>>>> drm_syncobj_search_fence(syncobj, 0, 0, fence);
>>> Hmm, just thinking of other ways of tidying this up
>>>
>>> mutex_lock(cb_lock);
>>> spin_lock(pt_lock);
>>> *fence = drm_syncobj_find_signal_pt_for_point();
>>> spin_unlock(pt_list);
>>> if (*!fence)
>>> drm_syncobj_add_callback_locked(syncobj, cb, func);
>>> mutex_unlock(cb_lock);
>>>
>>> i.e. get rid of the early return and we can even drop the int return here
>>> as it is unimportant and unused.
>> Yes, do you need I send v6? or you make a separate patch as a improvment?
> Send it in reply, we still have some time before the shards catch up
> with the ml ;)
I'm idle anyway because I've locked myself out of the AMD VPN accidentally.
So just send me a ping when the v6 is ready to be committed and I can
push it to drm-misc-next.
Christian.
> -Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* ✓ Fi.CI.BAT: success for drm: fix deadlock of syncobj v5
2018-10-23 7:57 [PATCH] drm: fix deadlock of syncobj v5 Chunming Zhou
` (2 preceding siblings ...)
2018-10-23 9:17 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2018-10-23 9:34 ` Patchwork
2018-10-23 11:37 ` ✗ Fi.CI.IGT: failure " Patchwork
4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-10-23 9:34 UTC (permalink / raw)
To: Chunming Zhou; +Cc: intel-gfx
== Series Details ==
Series: drm: fix deadlock of syncobj v5
URL : https://patchwork.freedesktop.org/series/51365/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5020 -> Patchwork_10538 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/51365/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_10538 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_exec_suspend@basic-s3:
fi-kbl-soraka: NOTRUN -> INCOMPLETE (fdo#107859, fdo#107556, fdo#107774)
fi-blb-e6850: PASS -> INCOMPLETE (fdo#107718)
igt@kms_flip@basic-flip-vs-dpms:
fi-hsw-4770r: PASS -> DMESG-WARN (fdo#105602)
==== Possible fixes ====
igt@kms_frontbuffer_tracking@basic:
fi-hsw-peppy: DMESG-WARN (fdo#102614) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-byt-clapper: FAIL (fdo#103191, fdo#107362) -> PASS +1
igt@prime_vgem@basic-fence-flip:
fi-cfl-8700k: FAIL (fdo#104008) -> PASS
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556
fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
fdo#107774 https://bugs.freedesktop.org/show_bug.cgi?id=107774
fdo#107859 https://bugs.freedesktop.org/show_bug.cgi?id=107859
== Participating hosts (47 -> 44) ==
Additional (2): fi-kbl-soraka fi-icl-u
Missing (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600
== Build changes ==
* Linux: CI_DRM_5020 -> Patchwork_10538
CI_DRM_5020: 95151c25e0433a2fe771b8bc272f3f8fb54a7e27 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4686: 741bf7064c467df725c14cc0b3b8b50436f9ee09 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10538: 9817489f632a6b715a312301b87593c62faf63ae @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
9817489f632a drm: fix deadlock of syncobj v5
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10538/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* ✗ Fi.CI.IGT: failure for drm: fix deadlock of syncobj v5
2018-10-23 7:57 [PATCH] drm: fix deadlock of syncobj v5 Chunming Zhou
` (3 preceding siblings ...)
2018-10-23 9:34 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-10-23 11:37 ` Patchwork
4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-10-23 11:37 UTC (permalink / raw)
To: Chunming Zhou; +Cc: intel-gfx
== Series Details ==
Series: drm: fix deadlock of syncobj v5
URL : https://patchwork.freedesktop.org/series/51365/
State : failure
== Summary ==
= CI Bug Log - changes from CI_DRM_5020_full -> Patchwork_10538_full =
== Summary - FAILURE ==
Serious unknown changes coming with Patchwork_10538_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10538_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_10538_full:
=== IGT changes ===
==== Possible regressions ====
igt@kms_draw_crc@draw-method-xrgb8888-blt-ytiled:
shard-skl: NOTRUN -> FAIL
== Known issues ==
Here are the changes found in Patchwork_10538_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@drv_suspend@debugfs-reader:
shard-skl: PASS -> INCOMPLETE (fdo#104108)
igt@drv_suspend@shrink:
shard-snb: NOTRUN -> INCOMPLETE (fdo#105411, fdo#106886)
igt@gem_cpu_reloc@full:
shard-skl: NOTRUN -> INCOMPLETE (fdo#108073)
igt@gem_exec_schedule@pi-ringfull-bsd:
shard-skl: NOTRUN -> FAIL (fdo#103158) +2
igt@gem_partial_pwrite_pread@writes-after-reads:
shard-snb: NOTRUN -> INCOMPLETE (fdo#105411)
igt@kms_busy@extended-modeset-hang-newfb-render-b:
shard-skl: NOTRUN -> DMESG-WARN (fdo#107956) +1
igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
shard-snb: NOTRUN -> DMESG-WARN (fdo#107956) +1
igt@kms_busy@extended-pageflip-hang-newfb-render-b:
shard-apl: NOTRUN -> DMESG-WARN (fdo#107956) +1
shard-glk: NOTRUN -> DMESG-WARN (fdo#107956) +1
igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
shard-kbl: NOTRUN -> DMESG-WARN (fdo#107956)
shard-hsw: NOTRUN -> DMESG-WARN (fdo#107956)
igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
shard-hsw: PASS -> DMESG-WARN (fdo#107956)
igt@kms_cursor_crc@cursor-128x128-sliding:
shard-apl: PASS -> FAIL (fdo#103232) +1
igt@kms_cursor_crc@cursor-128x128-suspend:
shard-glk: PASS -> FAIL (fdo#103232) +1
igt@kms_cursor_crc@cursor-256x256-suspend:
shard-apl: NOTRUN -> FAIL (fdo#103191, fdo#103232)
igt@kms_cursor_crc@cursor-64x21-random:
shard-apl: NOTRUN -> FAIL (fdo#103232) +2
igt@kms_cursor_legacy@cursorb-vs-flipb-atomic-transitions-varying-size:
shard-glk: PASS -> DMESG-WARN (fdo#106538, fdo#105763)
igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-untiled:
shard-skl: NOTRUN -> FAIL (fdo#108145) +3
igt@kms_draw_crc@fill-fb:
shard-skl: NOTRUN -> FAIL (fdo#103184)
igt@kms_flip@2x-flip-vs-expired-vblank:
shard-glk: PASS -> FAIL (fdo#105363)
igt@kms_flip@flip-vs-expired-vblank:
shard-glk: PASS -> FAIL (fdo#105363, fdo#102887)
igt@kms_flip@flip-vs-modeset-vs-hang:
shard-apl: PASS -> INCOMPLETE (fdo#103927)
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff:
shard-kbl: NOTRUN -> FAIL (fdo#103167) +1
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
shard-apl: PASS -> FAIL (fdo#103167)
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
shard-apl: NOTRUN -> FAIL (fdo#103167) +3
shard-skl: NOTRUN -> FAIL (fdo#103167) +5
shard-glk: NOTRUN -> FAIL (fdo#103167) +1
igt@kms_frontbuffer_tracking@fbc-1p-rte:
shard-glk: PASS -> FAIL (fdo#105682, fdo#103167)
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
shard-glk: PASS -> FAIL (fdo#103167) +2
igt@kms_frontbuffer_tracking@fbcpsr-stridechange:
shard-skl: NOTRUN -> FAIL (fdo#105683)
igt@kms_plane@pixel-format-pipe-c-planes:
shard-apl: NOTRUN -> FAIL (fdo#103166)
shard-glk: NOTRUN -> FAIL (fdo#103166)
shard-kbl: NOTRUN -> FAIL (fdo#103166)
shard-skl: NOTRUN -> DMESG-FAIL (fdo#103166, fdo#106885)
igt@kms_plane_alpha_blend@pipe-b-alpha-7efc:
shard-skl: NOTRUN -> FAIL (fdo#108146)
igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
shard-apl: NOTRUN -> FAIL (fdo#108145) +1
igt@kms_plane_alpha_blend@pipe-b-alpha-transparant-fb:
shard-kbl: NOTRUN -> FAIL (fdo#108145) +1
shard-glk: NOTRUN -> FAIL (fdo#108145) +1
igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
shard-kbl: NOTRUN -> FAIL (fdo#108146)
shard-apl: NOTRUN -> FAIL (fdo#108146)
shard-glk: NOTRUN -> FAIL (fdo#108146)
igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
shard-glk: PASS -> FAIL (fdo#103166)
igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
shard-apl: PASS -> FAIL (fdo#103166)
igt@kms_rotation_crc@exhaust-fences:
shard-skl: NOTRUN -> DMESG-WARN (fdo#105748)
igt@kms_setmode@basic:
shard-skl: NOTRUN -> FAIL (fdo#99912)
igt@kms_sysfs_edid_timing:
shard-skl: NOTRUN -> FAIL (fdo#100047)
igt@pm_rpm@system-suspend:
shard-skl: NOTRUN -> INCOMPLETE (fdo#107773, fdo#107807, fdo#104108)
==== Possible fixes ====
igt@kms_chv_cursor_fail@pipe-b-256x256-top-edge:
shard-glk: DMESG-WARN (fdo#106538, fdo#105763) -> PASS +1
igt@kms_cursor_crc@cursor-256x256-sliding:
shard-apl: FAIL (fdo#103232) -> PASS
igt@kms_cursor_crc@cursor-64x64-onscreen:
shard-glk: FAIL (fdo#103232) -> PASS
igt@kms_flip@modeset-vs-vblank-race:
shard-kbl: FAIL (fdo#103060) -> PASS
igt@syncobj_wait@reset-during-wait-for-submit:
shard-glk: INCOMPLETE (k.org#198133, fdo#103359) -> PASS +6
igt@syncobj_wait@wait-all-for-submit-complex:
shard-skl: INCOMPLETE (fdo#108490) -> PASS +1
igt@syncobj_wait@wait-all-for-submit-snapshot:
shard-hsw: INCOMPLETE (fdo#103540) -> PASS +6
igt@syncobj_wait@wait-for-submit-complex:
shard-snb: INCOMPLETE (fdo#105411) -> PASS +1
igt@syncobj_wait@wait-for-submit-delayed-submit:
shard-kbl: INCOMPLETE (fdo#103665) -> PASS +6
igt@syncobj_wait@wait-for-submit-snapshot:
shard-apl: INCOMPLETE (fdo#103927) -> PASS +6
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
fdo#105683 https://bugs.freedesktop.org/show_bug.cgi?id=105683
fdo#105748 https://bugs.freedesktop.org/show_bug.cgi?id=105748
fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
fdo#106885 https://bugs.freedesktop.org/show_bug.cgi?id=106885
fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
fdo#108073 https://bugs.freedesktop.org/show_bug.cgi?id=108073
fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
fdo#108146 https://bugs.freedesktop.org/show_bug.cgi?id=108146
fdo#108490 https://bugs.freedesktop.org/show_bug.cgi?id=108490
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
== Participating hosts (6 -> 6) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_5020 -> Patchwork_10538
CI_DRM_5020: 95151c25e0433a2fe771b8bc272f3f8fb54a7e27 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4686: 741bf7064c467df725c14cc0b3b8b50436f9ee09 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10538: 9817489f632a6b715a312301b87593c62faf63ae @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10538/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-10-23 11:37 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-23 7:57 [PATCH] drm: fix deadlock of syncobj v5 Chunming Zhou
2018-10-23 9:01 ` Chris Wilson
2018-10-23 9:09 ` zhoucm1
2018-10-23 9:11 ` Chris Wilson
2018-10-23 9:27 ` Koenig, Christian
2018-10-23 9:17 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-10-23 9:17 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-10-23 9:34 ` ✓ Fi.CI.BAT: success " Patchwork
2018-10-23 11:37 ` ✗ Fi.CI.IGT: failure " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.