All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
@ 2018-10-26  6:20 Chunming Zhou
  2018-10-26  6:55 ` ✓ Fi.CI.BAT: success for drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock (rev2) Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Chunming Zhou @ 2018-10-26  6:20 UTC (permalink / raw)
  To: dri-devel; +Cc: Chunming Zhou, intel-gfx, Christian König, Julia Lawall

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.

v4:
Don't need moving lock.
Don't expose func.

v5:
rename func and directly return.

Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with
lock debug kernel options enabled.

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>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
CC: Julia Lawall <julia.lawall@lip6.fr>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/drm_syncobj.c | 36 ++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index b7eaa603f368..d1c6f21c72b5 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -80,6 +80,23 @@ struct drm_syncobj_signal_pt {
 	struct list_head list;
 };
 
+static DEFINE_SPINLOCK(signaled_fence_lock);
+static struct dma_fence signaled_fence;
+
+static struct dma_fence *drm_syncobj_get_stub_fence(void)
+{
+	spin_lock(&signaled_fence_lock);
+	if (!signaled_fence.ops) {
+		dma_fence_init(&signaled_fence,
+			       &drm_syncobj_stub_fence_ops,
+			       &signaled_fence_lock,
+			       0, 0);
+		dma_fence_signal_locked(&signaled_fence);
+	}
+	spin_unlock(&signaled_fence_lock);
+
+	return dma_fence_get(&signaled_fence);
+}
 /**
  * drm_syncobj_find - lookup and reference a sync object.
  * @file_private: drm file private pointer
@@ -113,23 +130,8 @@ static struct dma_fence
 	struct drm_syncobj_signal_pt *signal_pt;
 
 	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;
-	}
+	    (point <= syncobj->timeline))
+		return drm_syncobj_get_stub_fence();
 
 	list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) {
 		if (point > signal_pt->value)
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.BAT: success for drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock (rev2)
  2018-10-26  6:20 [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock Chunming Zhou
@ 2018-10-26  6:55 ` Patchwork
  2018-10-26  7:43 ` [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock Christian König
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-10-26  6:55 UTC (permalink / raw)
  To: Chunming Zhou; +Cc: intel-gfx

== Series Details ==

Series: drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock (rev2)
URL   : https://patchwork.freedesktop.org/series/51525/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5038 -> Patchwork_10597 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/51525/revisions/2/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10597 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_sanitycheck:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#107726) +1

    igt@gem_exec_suspend@basic-s3:
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    
    ==== Possible fixes ====

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       DMESG-WARN (fdo#102614) -> PASS

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#103191, fdo#107362) -> PASS

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107726 https://bugs.freedesktop.org/show_bug.cgi?id=107726


== Participating hosts (46 -> 43) ==

  Additional (1): fi-icl-u 
  Missing    (4): fi-bsw-cyan fi-ilk-m540 fi-byt-squawks fi-icl-u2 


== Build changes ==

    * Linux: CI_DRM_5038 -> Patchwork_10597

  CI_DRM_5038: 96ecfb04d5acfcc565068c09afd6d0d713b2ddef @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4695: 81b66cf2806d6a8e9516580fb31879677487d32b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10597: aa42e49ecef5c92ab6fb2121ede206cdb799178d @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

aa42e49ecef5 drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10597/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
  2018-10-26  6:20 [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock Chunming Zhou
  2018-10-26  6:55 ` ✓ Fi.CI.BAT: success for drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock (rev2) Patchwork
@ 2018-10-26  7:43 ` Christian König
  2018-10-26  8:28   ` zhoucm1
  2018-10-26 11:49 ` Maarten Lankhorst
  2018-10-26 13:08 ` ✓ Fi.CI.IGT: success for drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock (rev2) Patchwork
  3 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2018-10-26  7:43 UTC (permalink / raw)
  To: Chunming Zhou, dri-devel; +Cc: Christian König, Julia Lawall, intel-gfx

Am 26.10.18 um 08:20 schrieb 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.
>
> v4:
> Don't need moving lock.
> Don't expose func.
>
> v5:
> rename func and directly return.
>
> Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with
> lock debug kernel options enabled.
>
> 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>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> CC: Julia Lawall <julia.lawall@lip6.fr>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/drm_syncobj.c | 36 ++++++++++++++++++-----------------
>   1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index b7eaa603f368..d1c6f21c72b5 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -80,6 +80,23 @@ struct drm_syncobj_signal_pt {
>   	struct list_head list;
>   };
>   
> +static DEFINE_SPINLOCK(signaled_fence_lock);
> +static struct dma_fence signaled_fence;
> +
> +static struct dma_fence *drm_syncobj_get_stub_fence(void)
> +{
> +	spin_lock(&signaled_fence_lock);
> +	if (!signaled_fence.ops) {
> +		dma_fence_init(&signaled_fence,
> +			       &drm_syncobj_stub_fence_ops,
> +			       &signaled_fence_lock,
> +			       0, 0);
> +		dma_fence_signal_locked(&signaled_fence);
> +	}
> +	spin_unlock(&signaled_fence_lock);
> +
> +	return dma_fence_get(&signaled_fence);
> +}
>   /**
>    * drm_syncobj_find - lookup and reference a sync object.
>    * @file_private: drm file private pointer
> @@ -113,23 +130,8 @@ static struct dma_fence
>   	struct drm_syncobj_signal_pt *signal_pt;
>   
>   	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;
> -	}
> +	    (point <= syncobj->timeline))
> +		return drm_syncobj_get_stub_fence();
>   
>   	list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) {
>   		if (point > signal_pt->value)

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
  2018-10-26  7:43 ` [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock Christian König
@ 2018-10-26  8:28   ` zhoucm1
  2018-10-26 12:13     ` Koenig, Christian
  0 siblings, 1 reply; 10+ messages in thread
From: zhoucm1 @ 2018-10-26  8:28 UTC (permalink / raw)
  To: christian.koenig, Chunming Zhou, dri-devel
  Cc: Christian König, Julia Lawall, intel-gfx

Thanks, Could you help to submit to drm-misc again?

-David


On 2018年10月26日 15:43, Christian König wrote:
> Am 26.10.18 um 08:20 schrieb 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.
>>
>> v4:
>> Don't need moving lock.
>> Don't expose func.
>>
>> v5:
>> rename func and directly return.
>>
>> Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with
>> lock debug kernel options enabled.
>>
>> 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>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> CC: Julia Lawall <julia.lawall@lip6.fr>
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> Reviewed-by: Christian König <christian.koenig@amd.com>
>
>> ---
>>   drivers/gpu/drm/drm_syncobj.c | 36 ++++++++++++++++++-----------------
>>   1 file changed, 19 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_syncobj.c 
>> b/drivers/gpu/drm/drm_syncobj.c
>> index b7eaa603f368..d1c6f21c72b5 100644
>> --- a/drivers/gpu/drm/drm_syncobj.c
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -80,6 +80,23 @@ struct drm_syncobj_signal_pt {
>>       struct list_head list;
>>   };
>>   +static DEFINE_SPINLOCK(signaled_fence_lock);
>> +static struct dma_fence signaled_fence;
>> +
>> +static struct dma_fence *drm_syncobj_get_stub_fence(void)
>> +{
>> +    spin_lock(&signaled_fence_lock);
>> +    if (!signaled_fence.ops) {
>> +        dma_fence_init(&signaled_fence,
>> +                   &drm_syncobj_stub_fence_ops,
>> +                   &signaled_fence_lock,
>> +                   0, 0);
>> +        dma_fence_signal_locked(&signaled_fence);
>> +    }
>> +    spin_unlock(&signaled_fence_lock);
>> +
>> +    return dma_fence_get(&signaled_fence);
>> +}
>>   /**
>>    * drm_syncobj_find - lookup and reference a sync object.
>>    * @file_private: drm file private pointer
>> @@ -113,23 +130,8 @@ static struct dma_fence
>>       struct drm_syncobj_signal_pt *signal_pt;
>>         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;
>> -    }
>> +        (point <= syncobj->timeline))
>> +        return drm_syncobj_get_stub_fence();
>>         list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) {
>>           if (point > signal_pt->value)
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
  2018-10-26  6:20 [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock Chunming Zhou
  2018-10-26  6:55 ` ✓ Fi.CI.BAT: success for drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock (rev2) Patchwork
  2018-10-26  7:43 ` [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock Christian König
@ 2018-10-26 11:49 ` Maarten Lankhorst
  2018-10-26 13:08 ` ✓ Fi.CI.IGT: success for drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock (rev2) Patchwork
  3 siblings, 0 replies; 10+ messages in thread
From: Maarten Lankhorst @ 2018-10-26 11:49 UTC (permalink / raw)
  To: Chunming Zhou, dri-devel; +Cc: Christian König, Julia Lawall, intel-gfx

Op 26-10-18 om 08:20 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.
>
> v4:
> Don't need moving lock.
> Don't expose func.
>
> v5:
> rename func and directly return.
>
> Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with
> lock debug kernel options enabled.
>
> 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>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> CC: Julia Lawall <julia.lawall@lip6.fr>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/drm_syncobj.c | 36 ++++++++++++++++++-----------------
>  1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index b7eaa603f368..d1c6f21c72b5 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -80,6 +80,23 @@ struct drm_syncobj_signal_pt {
>  	struct list_head list;
>  };
>  
> +static DEFINE_SPINLOCK(signaled_fence_lock);
> +static struct dma_fence signaled_fence;
> +
> +static struct dma_fence *drm_syncobj_get_stub_fence(void)
> +{
> +	spin_lock(&signaled_fence_lock);
> +	if (!signaled_fence.ops) {
> +		dma_fence_init(&signaled_fence,
> +			       &drm_syncobj_stub_fence_ops,
> +			       &signaled_fence_lock,
> +			       0, 0);
> +		dma_fence_signal_locked(&signaled_fence);
> +	}
> +	spin_unlock(&signaled_fence_lock);
Could this be used by drm_syncobj_assign_null_handle too? Maybe as a separate patch?

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
  2018-10-26  8:28   ` zhoucm1
@ 2018-10-26 12:13     ` Koenig, Christian
  0 siblings, 0 replies; 10+ messages in thread
From: Koenig, Christian @ 2018-10-26 12:13 UTC (permalink / raw)
  To: Zhou, David(ChunMing); +Cc: Christian König, Julia Lawall, intel-gfx

Am 26.10.18 um 10:28 schrieb zhoucm1:
> Thanks, Could you help to submit to drm-misc again?

Done.

Christian.

>
> -David
>
>
> On 2018年10月26日 15:43, Christian König wrote:
>> Am 26.10.18 um 08:20 schrieb 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.
>>>
>>> v4:
>>> Don't need moving lock.
>>> Don't expose func.
>>>
>>> v5:
>>> rename func and directly return.
>>>
>>> Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with
>>> lock debug kernel options enabled.
>>>
>>> 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>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> CC: Julia Lawall <julia.lawall@lip6.fr>
>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>>
>> Reviewed-by: Christian König <christian.koenig@amd.com>
>>
>>> ---
>>>   drivers/gpu/drm/drm_syncobj.c | 36 
>>> ++++++++++++++++++-----------------
>>>   1 file changed, 19 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c 
>>> b/drivers/gpu/drm/drm_syncobj.c
>>> index b7eaa603f368..d1c6f21c72b5 100644
>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>> @@ -80,6 +80,23 @@ struct drm_syncobj_signal_pt {
>>>       struct list_head list;
>>>   };
>>>   +static DEFINE_SPINLOCK(signaled_fence_lock);
>>> +static struct dma_fence signaled_fence;
>>> +
>>> +static struct dma_fence *drm_syncobj_get_stub_fence(void)
>>> +{
>>> +    spin_lock(&signaled_fence_lock);
>>> +    if (!signaled_fence.ops) {
>>> +        dma_fence_init(&signaled_fence,
>>> +                   &drm_syncobj_stub_fence_ops,
>>> +                   &signaled_fence_lock,
>>> +                   0, 0);
>>> +        dma_fence_signal_locked(&signaled_fence);
>>> +    }
>>> +    spin_unlock(&signaled_fence_lock);
>>> +
>>> +    return dma_fence_get(&signaled_fence);
>>> +}
>>>   /**
>>>    * drm_syncobj_find - lookup and reference a sync object.
>>>    * @file_private: drm file private pointer
>>> @@ -113,23 +130,8 @@ static struct dma_fence
>>>       struct drm_syncobj_signal_pt *signal_pt;
>>>         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;
>>> -    }
>>> +        (point <= syncobj->timeline))
>>> +        return drm_syncobj_get_stub_fence();
>>>         list_for_each_entry(signal_pt, &syncobj->signal_pt_list, 
>>> list) {
>>>           if (point > signal_pt->value)
>>
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.IGT: success for drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock (rev2)
  2018-10-26  6:20 [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock Chunming Zhou
                   ` (2 preceding siblings ...)
  2018-10-26 11:49 ` Maarten Lankhorst
@ 2018-10-26 13:08 ` Patchwork
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-10-26 13:08 UTC (permalink / raw)
  To: Chunming Zhou; +Cc: intel-gfx

== Series Details ==

Series: drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock (rev2)
URL   : https://patchwork.freedesktop.org/series/51525/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5038_full -> Patchwork_10597_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10597_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10597_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_10597_full:

  === IGT changes ===

    ==== Warnings ====

    igt@pm_rc6_residency@rc6-accuracy:
      shard-kbl:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in Patchwork_10597_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_busy@close-race:
      shard-apl:          PASS -> DMESG-FAIL (fdo#108561)

    igt@kms_busy@basic-modeset-a:
      shard-apl:          PASS -> DMESG-WARN (fdo#108549) +9

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
      shard-skl:          NOTRUN -> DMESG-WARN (fdo#107956) +2

    igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
      shard-glk:          PASS -> FAIL (fdo#108145)

    igt@kms_cursor_crc@cursor-128x42-sliding:
      shard-apl:          PASS -> FAIL (fdo#103232) +1

    igt@kms_cursor_crc@cursor-64x64-sliding:
      shard-glk:          PASS -> FAIL (fdo#103232) +2

    igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
      shard-glk:          PASS -> DMESG-WARN (fdo#106538, fdo#105763)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
      shard-glk:          PASS -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-cpu:
      shard-apl:          PASS -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbc-1p-rte:
      shard-glk:          PASS -> FAIL (fdo#103167, fdo#105682)

    igt@kms_frontbuffer_tracking@fbcpsr-stridechange:
      shard-skl:          NOTRUN -> FAIL (fdo#105683)

    igt@kms_plane_alpha_blend@pipe-c-alpha-basic:
      shard-skl:          NOTRUN -> FAIL (fdo#108145, fdo#107815)

    igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
      shard-skl:          NOTRUN -> FAIL (fdo#108145) +2

    igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
      shard-skl:          NOTRUN -> FAIL (fdo#108146) +2

    igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
      shard-apl:          PASS -> FAIL (fdo#103166)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)
      shard-kbl:          PASS -> FAIL (fdo#99912)

    igt@kms_vblank@pipe-b-ts-continuation-modeset-hang:
      shard-apl:          PASS -> DMESG-FAIL (fdo#108549)

    igt@pm_backlight@fade_with_suspend:
      shard-skl:          NOTRUN -> FAIL (fdo#107847)

    igt@pm_rpm@modeset-non-lpsp:
      shard-skl:          NOTRUN -> INCOMPLETE (fdo#107807) +2

    
    ==== Possible fixes ====

    igt@kms_busy@extended-modeset-hang-newfb-render-a:
      shard-kbl:          DMESG-WARN (fdo#107956) -> PASS

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
      shard-hsw:          DMESG-WARN (fdo#107956) -> PASS

    igt@kms_ccs@pipe-a-crc-primary-basic:
      shard-skl:          FAIL (fdo#107725) -> PASS

    igt@kms_cursor_crc@cursor-256x85-random:
      shard-apl:          FAIL (fdo#103232) -> PASS +1

    igt@kms_cursor_crc@cursor-64x21-onscreen:
      shard-glk:          FAIL (fdo#103232) -> PASS +2

    igt@kms_cursor_crc@cursor-64x21-random:
      shard-apl:          DMESG-WARN (fdo#108549) -> PASS +10

    igt@kms_cursor_crc@cursor-64x64-suspend:
      shard-apl:          FAIL (fdo#103232, fdo#103191) -> PASS

    igt@kms_frontbuffer_tracking@fbc-2p-rte:
      shard-glk:          FAIL (fdo#103167, fdo#105682) -> PASS

    igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-render:
      shard-skl:          FAIL (fdo#103167) -> PASS

    igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
      shard-apl:          FAIL (fdo#103166) -> PASS +1

    igt@kms_vblank@pipe-b-ts-continuation-modeset-rpm:
      shard-apl:          DMESG-FAIL (fdo#108549) -> PASS

    igt@perf@polling:
      shard-hsw:          FAIL (fdo#102252) -> PASS

    
    ==== Warnings ====

    igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
      shard-apl:          DMESG-FAIL (fdo#108145, fdo#108549) -> FAIL (fdo#108145)

    
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
  fdo#105683 https://bugs.freedesktop.org/show_bug.cgi?id=105683
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#107725 https://bugs.freedesktop.org/show_bug.cgi?id=107725
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
  fdo#107847 https://bugs.freedesktop.org/show_bug.cgi?id=107847
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108146 https://bugs.freedesktop.org/show_bug.cgi?id=108146
  fdo#108549 https://bugs.freedesktop.org/show_bug.cgi?id=108549
  fdo#108561 https://bugs.freedesktop.org/show_bug.cgi?id=108561
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 6) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_5038 -> Patchwork_10597

  CI_DRM_5038: 96ecfb04d5acfcc565068c09afd6d0d713b2ddef @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4695: 81b66cf2806d6a8e9516580fb31879677487d32b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10597: aa42e49ecef5c92ab6fb2121ede206cdb799178d @ 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_10597/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
  2018-10-25 15:08 [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock Chunming Zhou
  2018-10-25 16:31 ` Chris Wilson
@ 2018-10-25 18:58 ` Christian König
  1 sibling, 0 replies; 10+ messages in thread
From: Christian König @ 2018-10-25 18:58 UTC (permalink / raw)
  To: Chunming Zhou, dri-devel; +Cc: Julia Lawall, intel-gfx

Am 25.10.18 um 17:08 schrieb 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.
>
> v4:
> Don't need moving lock.
> Don't expose func.
>
> Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with
> lock debug kernel options enabled.
>
> 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>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> CC: Julia Lawall <julia.lawall@lip6.fr>
> ---
>   drivers/gpu/drm/drm_syncobj.c | 41 ++++++++++++++++++++---------------
>   1 file changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index b7eaa603f368..fab0a2cf672e 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -80,6 +80,23 @@ struct drm_syncobj_signal_pt {
>   	struct list_head list;
>   };
>   
> +static DEFINE_SPINLOCK(signaled_fence_lock);
> +static struct dma_fence signaled_fence;
> +
> +static struct dma_fence *drm_syncobj_signaled_fence_get(void)

Maybe name that drm_syncobj_get_stub_fence().

> +{
> +	spin_lock(&signaled_fence_lock);
> +	if (!signaled_fence.ops) {
> +		dma_fence_init(&signaled_fence,
> +			       &drm_syncobj_stub_fence_ops,
> +			       &signaled_fence_lock,
> +			       0, 0);
> +		dma_fence_signal_locked(&signaled_fence);
> +	}
> +	spin_unlock(&signaled_fence_lock);
> +
> +	return dma_fence_get(&signaled_fence);
> +}
>   /**
>    * drm_syncobj_find - lookup and reference a sync object.
>    * @file_private: drm file private pointer
> @@ -111,24 +128,12 @@ static struct dma_fence
>   				      uint64_t point)
>   {
>   	struct drm_syncobj_signal_pt *signal_pt;
> +	struct dma_fence *f = NULL;
>   
>   	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;
> +		f = drm_syncobj_signaled_fence_get();
> +		goto out;
>   	}
>   
>   	list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) {
> @@ -137,9 +142,11 @@ 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);
> +		goto out;
>   	}
> -	return NULL;
> +out:
> +	return f;

I think we can drop this change now and just return directly.

Christian.

>   }
>   
>   static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj,

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
  2018-10-25 15:08 [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock Chunming Zhou
@ 2018-10-25 16:31 ` Chris Wilson
  2018-10-25 18:58 ` Christian König
  1 sibling, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2018-10-25 16:31 UTC (permalink / raw)
  To: dri-devel; +Cc: Christian König, Chunming Zhou, intel-gfx, Julia Lawall

Quoting Chunming Zhou (2018-10-25 16:08:31)
> 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.
> 
> v4:
> Don't need moving lock.
> Don't expose func.
> 
> Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with
> lock debug kernel options enabled.
> 
> 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>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> CC: Julia Lawall <julia.lawall@lip6.fr>
> ---
> -       return NULL;
> +out:
> +       return f;

As it reduced to just a return, I'd probably have gone with multiple
returns in this instance. Still the compiler should have done the
equivalent and jumped to a single ret.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
@ 2018-10-25 15:08 Chunming Zhou
  2018-10-25 16:31 ` Chris Wilson
  2018-10-25 18:58 ` Christian König
  0 siblings, 2 replies; 10+ messages in thread
From: Chunming Zhou @ 2018-10-25 15:08 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx, Christian König, Julia Lawall

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.

v4:
Don't need moving lock.
Don't expose func.

Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with
lock debug kernel options enabled.

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>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
CC: Julia Lawall <julia.lawall@lip6.fr>
---
 drivers/gpu/drm/drm_syncobj.c | 41 ++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index b7eaa603f368..fab0a2cf672e 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -80,6 +80,23 @@ struct drm_syncobj_signal_pt {
 	struct list_head list;
 };
 
+static DEFINE_SPINLOCK(signaled_fence_lock);
+static struct dma_fence signaled_fence;
+
+static struct dma_fence *drm_syncobj_signaled_fence_get(void)
+{
+	spin_lock(&signaled_fence_lock);
+	if (!signaled_fence.ops) {
+		dma_fence_init(&signaled_fence,
+			       &drm_syncobj_stub_fence_ops,
+			       &signaled_fence_lock,
+			       0, 0);
+		dma_fence_signal_locked(&signaled_fence);
+	}
+	spin_unlock(&signaled_fence_lock);
+
+	return dma_fence_get(&signaled_fence);
+}
 /**
  * drm_syncobj_find - lookup and reference a sync object.
  * @file_private: drm file private pointer
@@ -111,24 +128,12 @@ static struct dma_fence
 				      uint64_t point)
 {
 	struct drm_syncobj_signal_pt *signal_pt;
+	struct dma_fence *f = NULL;
 
 	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;
+		f = drm_syncobj_signaled_fence_get();
+		goto out;
 	}
 
 	list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) {
@@ -137,9 +142,11 @@ 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);
+		goto out;
 	}
-	return NULL;
+out:
+	return f;
 }
 
 static void drm_syncobj_add_callback_locked(struct drm_syncobj *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] 10+ messages in thread

end of thread, other threads:[~2018-10-26 13:08 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-26  6:20 [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock Chunming Zhou
2018-10-26  6:55 ` ✓ Fi.CI.BAT: success for drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock (rev2) Patchwork
2018-10-26  7:43 ` [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock Christian König
2018-10-26  8:28   ` zhoucm1
2018-10-26 12:13     ` Koenig, Christian
2018-10-26 11:49 ` Maarten Lankhorst
2018-10-26 13:08 ` ✓ Fi.CI.IGT: success for drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock (rev2) Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2018-10-25 15:08 [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock Chunming Zhou
2018-10-25 16:31 ` Chris Wilson
2018-10-25 18:58 ` Christian König

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.