Re: [PATCH] drm: fix call_kern.cocci warnings v3
Op 25-10-18 om 14:01 schreef Chunming Zhou:
>
> 在 2018/10/25 18:36, Maarten Lankhorst 写道:
>> Op 25-10-18 om 12:21 schreef 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.
>>>
>>> Generated by: scripts/coccinelle/locks/call_kern.cocci
>>>
>>> v2:
>>> syncobj->timeline still needs protect.
>>>
>>> v3:
>>> use a global signaled fence instead of re-allocation.
>>>
>>> Signed-off-by: Chunming Zhou
>>> Cc: Maarten Lankhorst
>>> Cc: intel-...@lists.freedesktop.org
>>> Cc: Christian König
>>> ---
>>> drivers/gpu/drm/drm_drv.c | 2 ++
>>> drivers/gpu/drm/drm_syncobj.c | 52 +--
>>> include/drm/drm_syncobj.h | 1 +
>>> 3 files changed, 34 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>>> index 36e8e9cbec52..0a6f1023d6c3 100644
>>> --- a/drivers/gpu/drm/drm_drv.c
>>> +++ b/drivers/gpu/drm/drm_drv.c
>>> @@ -37,6 +37,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>>
>>> #include "drm_crtc_internal.h"
>>> #include "drm_legacy.h"
>>> @@ -1003,6 +1004,7 @@ static int __init drm_core_init(void)
>>> if (ret < 0)
>>> goto error;
>>>
>>> + drm_syncobj_stub_fence_init();
>>> drm_core_init_complete = true;
>>>
>>> DRM_DEBUG("Initialized\n");
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>>> index b7eaa603f368..6b3f5a06e4d3 100644
>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>> @@ -80,6 +80,27 @@ struct drm_syncobj_signal_pt {
>>> struct list_head list;
>>> };
>>>
>>> +static struct drm_syncobj_stub_fence stub_signaled_fence;
>>> +static void global_stub_fence_release(struct dma_fence *fence)
>>> +{
>>> + /* it is impossible to come here */
>>> + BUG();
>>> +}
>> WARN_ON_ONCE(1)? No need to halt the machine.
>>
>>> +static const struct dma_fence_ops global_stub_fence_ops = {
>>> + .get_driver_name = drm_syncobj_stub_fence_get_name,
>>> + .get_timeline_name = drm_syncobj_stub_fence_get_name,
>>> + .release = global_stub_fence_release,
>>> +};
>>> +
>>> +void drm_syncobj_stub_fence_init(void)
>>> +{
>>> + spin_lock_init(&stub_signaled_fence.lock);
>>> + dma_fence_init(&stub_signaled_fence.base,
>>> + &global_stub_fence_ops,
>>> + &stub_signaled_fence.lock,
>>> + 0, 0);
>>> + dma_fence_signal(&stub_signaled_fence.base);
>>> +}
>>> /**
>>>* drm_syncobj_find - lookup and reference a sync object.
>>>* @file_private: drm file private pointer
>>> @@ -111,24 +132,14 @@ static struct dma_fence
>>> uint64_t point)
>>> {
>>> struct drm_syncobj_signal_pt *signal_pt;
>>> + struct dma_fence *f = NULL;
>>>
>>> + spin_lock(&syncobj->pt_lock);
>>> 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,
>>> - &fence->lock,
>>> - syncobj->timeline_context,
>>> - point);
>>> -
>>> - dma_fence_signal(&fence->base);
>>> - return &fence->base;
>>> + dma_fence_get(&stub_signaled_fence.base);
>>> + spin_unlock(&syncobj->pt_lock);
>>> + return &stub_signaled_fence.base;
>>> }
>>>
>>> list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) {
>>> @@ -137,9 +148,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);
>>> +
>>> + return f;
>>> }
>>>
>>> static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj,
>>> @@ -166,9 +180,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
Re: [Intel-gfx] [PATCH] drm: fix call_kern.cocci warnings v3
Give this a nice summary,
drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
Quoting Chunming Zhou (2018-10-25 11:21:05)
> 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.
>
> Generated by: scripts/coccinelle/locks/call_kern.cocci
>
> v2:
> syncobj->timeline still needs protect.
>
> v3:
> use a global signaled fence instead of re-allocation.
>
Good practice, would be to add Testcase: (and as penance for the bug,
write it ;)
> Signed-off-by: Chunming Zhou
> Cc: Maarten Lankhorst
> Cc: intel-...@lists.freedesktop.org
> Cc: Christian König
> ---
> drivers/gpu/drm/drm_drv.c | 2 ++
> drivers/gpu/drm/drm_syncobj.c | 52 +--
> include/drm/drm_syncobj.h | 1 +
> 3 files changed, 34 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 36e8e9cbec52..0a6f1023d6c3 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -37,6 +37,7 @@
> #include
> #include
> #include
> +#include
>
> #include "drm_crtc_internal.h"
> #include "drm_legacy.h"
> @@ -1003,6 +1004,7 @@ static int __init drm_core_init(void)
> if (ret < 0)
> goto error;
>
> + drm_syncobj_stub_fence_init();
> drm_core_init_complete = true;
>
> DRM_DEBUG("Initialized\n");
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index b7eaa603f368..6b3f5a06e4d3 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -80,6 +80,27 @@ struct drm_syncobj_signal_pt {
> struct list_head list;
> };
>
> +static struct drm_syncobj_stub_fence stub_signaled_fence;
> +static void global_stub_fence_release(struct dma_fence *fence)
> +{
> + /* it is impossible to come here */
> + BUG();
BUG() is overkill, kasan will complain for us anyway, so we can just use
the default release function.
> +}
> +static const struct dma_fence_ops global_stub_fence_ops = {
> + .get_driver_name = drm_syncobj_stub_fence_get_name,
> + .get_timeline_name = drm_syncobj_stub_fence_get_name,
> + .release = global_stub_fence_release,
> +};
> +
> +void drm_syncobj_stub_fence_init(void)
I think we can avoid having this exposed by:
static DECLARE_SPINLOCK(signaled_fence_lock);
static dma_fence signaled_fence;
static struct dma_fence *signaled_fence_get(void)
{
spin_lock(&signaled_fenced_lock);
if (!signaled_fence.ops) {
dma_fence_init(&signaled_fence,
&signaled_fence_ops,
&signaled_fence_lock,
0, 0);
dma_fence_signal_locked(&signaled_fence.base);
}
spin_unlock(&signaled_fenced_lock);
return dma_fence_get(&signaled_fence);
}
> /**
> * drm_syncobj_find - lookup and reference a sync object.
> * @file_private: drm file private pointer
> @@ -111,24 +132,14 @@ static struct dma_fence
> uint64_t point)
> {
> struct drm_syncobj_signal_pt *signal_pt;
> + struct dma_fence *f = NULL;
>
> + spin_lock(&syncobj->pt_lock);
> 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,
> - &fence->lock,
> - syncobj->timeline_context,
> - point);
> -
> - dma_fence_signal(&fence->base);
> - return &fence->base;
> + dma_fence_get(&stub_signaled_fence.base);
> + spin_unlock(&syncobj->pt_lock);
> + return &stub_signaled_fence.base;
f = signaled_fence_get();
goto unlock;
> }
>
> list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) {
> @@ -137,9 +148,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;
unlock:
> + spin_unlock(&syncobj->pt_lock);
> +
> + return f;
> }
___
dri-devel mailing list
dri-devel@lists.
Re: [PATCH] drm: fix call_kern.cocci warnings v3
在 2018/10/25 18:36, Maarten Lankhorst 写道:
> Op 25-10-18 om 12:21 schreef 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.
>>
>> Generated by: scripts/coccinelle/locks/call_kern.cocci
>>
>> v2:
>> syncobj->timeline still needs protect.
>>
>> v3:
>> use a global signaled fence instead of re-allocation.
>>
>> Signed-off-by: Chunming Zhou
>> Cc: Maarten Lankhorst
>> Cc: intel-...@lists.freedesktop.org
>> Cc: Christian König
>> ---
>> drivers/gpu/drm/drm_drv.c | 2 ++
>> drivers/gpu/drm/drm_syncobj.c | 52 +--
>> include/drm/drm_syncobj.h | 1 +
>> 3 files changed, 34 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index 36e8e9cbec52..0a6f1023d6c3 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -37,6 +37,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include "drm_crtc_internal.h"
>> #include "drm_legacy.h"
>> @@ -1003,6 +1004,7 @@ static int __init drm_core_init(void)
>> if (ret < 0)
>> goto error;
>>
>> +drm_syncobj_stub_fence_init();
>> drm_core_init_complete = true;
>>
>> DRM_DEBUG("Initialized\n");
>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>> index b7eaa603f368..6b3f5a06e4d3 100644
>> --- a/drivers/gpu/drm/drm_syncobj.c
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -80,6 +80,27 @@ struct drm_syncobj_signal_pt {
>> struct list_head list;
>> };
>>
>> +static struct drm_syncobj_stub_fence stub_signaled_fence;
>> +static void global_stub_fence_release(struct dma_fence *fence)
>> +{
>> +/* it is impossible to come here */
>> +BUG();
>> +}
> WARN_ON_ONCE(1)? No need to halt the machine.
>
>> +static const struct dma_fence_ops global_stub_fence_ops = {
>> +.get_driver_name = drm_syncobj_stub_fence_get_name,
>> +.get_timeline_name = drm_syncobj_stub_fence_get_name,
>> +.release = global_stub_fence_release,
>> +};
>> +
>> +void drm_syncobj_stub_fence_init(void)
>> +{
>> +spin_lock_init(&stub_signaled_fence.lock);
>> +dma_fence_init(&stub_signaled_fence.base,
>> + &global_stub_fence_ops,
>> + &stub_signaled_fence.lock,
>> + 0, 0);
>> +dma_fence_signal(&stub_signaled_fence.base);
>> +}
>> /**
>>* drm_syncobj_find - lookup and reference a sync object.
>>* @file_private: drm file private pointer
>> @@ -111,24 +132,14 @@ static struct dma_fence
>>uint64_t point)
>> {
>> struct drm_syncobj_signal_pt *signal_pt;
>> +struct dma_fence *f = NULL;
>>
>> +spin_lock(&syncobj->pt_lock);
>> 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,
>> - &fence->lock,
>> - syncobj->timeline_context,
>> - point);
>> -
>> -dma_fence_signal(&fence->base);
>> -return &fence->base;
>> +dma_fence_get(&stub_signaled_fence.base);
>> +spin_unlock(&syncobj->pt_lock);
>> +return &stub_signaled_fence.base;
>> }
>>
>> list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) {
>> @@ -137,9 +148,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);
>> +
>> +return f;
>> }
>>
>> static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj,
>> @@ -166,9 +180,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 +391,9 @@ drm_syncobj_point_get(struct drm_syncobj *syncobj, u64
>> point, u64 flags,
>>
Re: [PATCH] drm: fix call_kern.cocci warnings v3
Am 25.10.18 um 12:36 schrieb Maarten Lankhorst:
> Op 25-10-18 om 12:21 schreef 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.
>>
>> Generated by: scripts/coccinelle/locks/call_kern.cocci
>>
>> v2:
>> syncobj->timeline still needs protect.
>>
>> v3:
>> use a global signaled fence instead of re-allocation.
>>
>> Signed-off-by: Chunming Zhou
>> Cc: Maarten Lankhorst
>> Cc: intel-...@lists.freedesktop.org
>> Cc: Christian König
>> ---
>> drivers/gpu/drm/drm_drv.c | 2 ++
>> drivers/gpu/drm/drm_syncobj.c | 52 +--
>> include/drm/drm_syncobj.h | 1 +
>> 3 files changed, 34 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index 36e8e9cbec52..0a6f1023d6c3 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -37,6 +37,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include "drm_crtc_internal.h"
>> #include "drm_legacy.h"
>> @@ -1003,6 +1004,7 @@ static int __init drm_core_init(void)
>> if (ret < 0)
>> goto error;
>>
>> +drm_syncobj_stub_fence_init();
>> drm_core_init_complete = true;
>>
>> DRM_DEBUG("Initialized\n");
>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>> index b7eaa603f368..6b3f5a06e4d3 100644
>> --- a/drivers/gpu/drm/drm_syncobj.c
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -80,6 +80,27 @@ struct drm_syncobj_signal_pt {
>> struct list_head list;
>> };
>>
>> +static struct drm_syncobj_stub_fence stub_signaled_fence;
>> +static void global_stub_fence_release(struct dma_fence *fence)
>> +{
>> +/* it is impossible to come here */
>> +BUG();
>> +}
> WARN_ON_ONCE(1)? No need to halt the machine.
>
>> +static const struct dma_fence_ops global_stub_fence_ops = {
>> +.get_driver_name = drm_syncobj_stub_fence_get_name,
>> +.get_timeline_name = drm_syncobj_stub_fence_get_name,
>> +.release = global_stub_fence_release,
>> +};
>> +
>> +void drm_syncobj_stub_fence_init(void)
>> +{
>> +spin_lock_init(&stub_signaled_fence.lock);
>> +dma_fence_init(&stub_signaled_fence.base,
>> + &global_stub_fence_ops,
>> + &stub_signaled_fence.lock,
>> + 0, 0);
>> +dma_fence_signal(&stub_signaled_fence.base);
>> +}
>> /**
>>* drm_syncobj_find - lookup and reference a sync object.
>>* @file_private: drm file private pointer
>> @@ -111,24 +132,14 @@ static struct dma_fence
>>uint64_t point)
>> {
>> struct drm_syncobj_signal_pt *signal_pt;
>> +struct dma_fence *f = NULL;
>>
>> +spin_lock(&syncobj->pt_lock);
>> 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,
>> - &fence->lock,
>> - syncobj->timeline_context,
>> - point);
>> -
>> -dma_fence_signal(&fence->base);
>> -return &fence->base;
>> +dma_fence_get(&stub_signaled_fence.base);
>> +spin_unlock(&syncobj->pt_lock);
>> +return &stub_signaled_fence.base;
>> }
>>
>> list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) {
>> @@ -137,9 +148,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);
>> +
>> +return f;
>> }
>>
>> static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj,
>> @@ -166,9 +180,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 +391,9 @@ drm_syncobj_point_get(struct drm_syncobj *syncobj, u64
>> point, u64 flags,
>>
Re: [PATCH] drm: fix call_kern.cocci warnings v3
Op 25-10-18 om 12:21 schreef 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.
>
> Generated by: scripts/coccinelle/locks/call_kern.cocci
>
> v2:
> syncobj->timeline still needs protect.
>
> v3:
> use a global signaled fence instead of re-allocation.
>
> Signed-off-by: Chunming Zhou
> Cc: Maarten Lankhorst
> Cc: intel-...@lists.freedesktop.org
> Cc: Christian König
> ---
> drivers/gpu/drm/drm_drv.c | 2 ++
> drivers/gpu/drm/drm_syncobj.c | 52 +--
> include/drm/drm_syncobj.h | 1 +
> 3 files changed, 34 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 36e8e9cbec52..0a6f1023d6c3 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -37,6 +37,7 @@
> #include
> #include
> #include
> +#include
>
> #include "drm_crtc_internal.h"
> #include "drm_legacy.h"
> @@ -1003,6 +1004,7 @@ static int __init drm_core_init(void)
> if (ret < 0)
> goto error;
>
> + drm_syncobj_stub_fence_init();
> drm_core_init_complete = true;
>
> DRM_DEBUG("Initialized\n");
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index b7eaa603f368..6b3f5a06e4d3 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -80,6 +80,27 @@ struct drm_syncobj_signal_pt {
> struct list_head list;
> };
>
> +static struct drm_syncobj_stub_fence stub_signaled_fence;
> +static void global_stub_fence_release(struct dma_fence *fence)
> +{
> + /* it is impossible to come here */
> + BUG();
> +}
WARN_ON_ONCE(1)? No need to halt the machine.
> +static const struct dma_fence_ops global_stub_fence_ops = {
> + .get_driver_name = drm_syncobj_stub_fence_get_name,
> + .get_timeline_name = drm_syncobj_stub_fence_get_name,
> + .release = global_stub_fence_release,
> +};
> +
> +void drm_syncobj_stub_fence_init(void)
> +{
> + spin_lock_init(&stub_signaled_fence.lock);
> + dma_fence_init(&stub_signaled_fence.base,
> +&global_stub_fence_ops,
> +&stub_signaled_fence.lock,
> +0, 0);
> + dma_fence_signal(&stub_signaled_fence.base);
> +}
> /**
> * drm_syncobj_find - lookup and reference a sync object.
> * @file_private: drm file private pointer
> @@ -111,24 +132,14 @@ static struct dma_fence
> uint64_t point)
> {
> struct drm_syncobj_signal_pt *signal_pt;
> + struct dma_fence *f = NULL;
>
> + spin_lock(&syncobj->pt_lock);
> 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,
> -&fence->lock,
> -syncobj->timeline_context,
> -point);
> -
> - dma_fence_signal(&fence->base);
> - return &fence->base;
> + dma_fence_get(&stub_signaled_fence.base);
> + spin_unlock(&syncobj->pt_lock);
> + return &stub_signaled_fence.base;
> }
>
> list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) {
> @@ -137,9 +148,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);
> +
> + return f;
> }
>
> static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj,
> @@ -166,9 +180,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 +391,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_
[PATCH] drm: fix call_kern.cocci warnings v3
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.
Generated by: scripts/coccinelle/locks/call_kern.cocci
v2:
syncobj->timeline still needs protect.
v3:
use a global signaled fence instead of re-allocation.
Signed-off-by: Chunming Zhou
Cc: Maarten Lankhorst
Cc: intel-...@lists.freedesktop.org
Cc: Christian König
---
drivers/gpu/drm/drm_drv.c | 2 ++
drivers/gpu/drm/drm_syncobj.c | 52 +--
include/drm/drm_syncobj.h | 1 +
3 files changed, 34 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 36e8e9cbec52..0a6f1023d6c3 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include "drm_crtc_internal.h"
#include "drm_legacy.h"
@@ -1003,6 +1004,7 @@ static int __init drm_core_init(void)
if (ret < 0)
goto error;
+ drm_syncobj_stub_fence_init();
drm_core_init_complete = true;
DRM_DEBUG("Initialized\n");
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index b7eaa603f368..6b3f5a06e4d3 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -80,6 +80,27 @@ struct drm_syncobj_signal_pt {
struct list_head list;
};
+static struct drm_syncobj_stub_fence stub_signaled_fence;
+static void global_stub_fence_release(struct dma_fence *fence)
+{
+ /* it is impossible to come here */
+ BUG();
+}
+static const struct dma_fence_ops global_stub_fence_ops = {
+ .get_driver_name = drm_syncobj_stub_fence_get_name,
+ .get_timeline_name = drm_syncobj_stub_fence_get_name,
+ .release = global_stub_fence_release,
+};
+
+void drm_syncobj_stub_fence_init(void)
+{
+ spin_lock_init(&stub_signaled_fence.lock);
+ dma_fence_init(&stub_signaled_fence.base,
+ &global_stub_fence_ops,
+ &stub_signaled_fence.lock,
+ 0, 0);
+ dma_fence_signal(&stub_signaled_fence.base);
+}
/**
* drm_syncobj_find - lookup and reference a sync object.
* @file_private: drm file private pointer
@@ -111,24 +132,14 @@ static struct dma_fence
uint64_t point)
{
struct drm_syncobj_signal_pt *signal_pt;
+ struct dma_fence *f = NULL;
+ spin_lock(&syncobj->pt_lock);
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,
- &fence->lock,
- syncobj->timeline_context,
- point);
-
- dma_fence_signal(&fence->base);
- return &fence->base;
+ dma_fence_get(&stub_signaled_fence.base);
+ spin_unlock(&syncobj->pt_lock);
+ return &stub_signaled_fence.base;
}
list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) {
@@ -137,9 +148,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);
+
+ return f;
}
static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj,
@@ -166,9 +180,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 +391,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;
}
diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
index 29244cbcd05e..63cfd154
