[PATCH] drm: fix call_kern.cocci warnings v2

[PATCH] drm: fix call_kern.cocci warnings v2

[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.

Semantic patch information:
  The proposed change of converting the GFP_KERNEL is not necessarily the
  correct one.  It may be desired to unlock the lock, or to not call the
  function under the lock in the first place.

Generated by: scripts/coccinelle/locks/call_kern.cocci

v2:
syncobj->timeline still needs protect.

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>
---
 drivers/gpu/drm/drm_syncobj.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index b7eaa603f368..b843d4c2778c 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -111,15 +111,16 @@ static struct dma_fence
 				      uint64_t point)
 {
 	struct drm_syncobj_signal_pt *signal_pt;
+	struct dma_fence *f = NULL;
+	struct drm_syncobj_stub_fence *fence =
+		kzalloc(sizeof(struct drm_syncobj_stub_fence),
+			GFP_KERNEL);
 
+	if (!fence)
+		return 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,
@@ -128,6 +129,7 @@ static struct dma_fence
 			       point);
 
 		dma_fence_signal(&fence->base);
+		spin_unlock(&syncobj->pt_lock);
 		return &fence->base;
 	}
 
@@ -137,9 +139,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);
+	kfree(fence);
+	return f;
 }
 
 static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj,
@@ -166,9 +171,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 +382,9 @@ drm_syncobj_point_get(struct drm_syncobj *syncobj, u64 point, u64 flags,
 		if (ret < 0)
 			return ret;
 	}
-	spin_lock(&syncobj->pt_lock);
 	*fence = drm_syncobj_find_signal_pt_for_point(syncobj, point);
 	if (!*fence)
 		ret = -EINVAL;
-	spin_unlock(&syncobj->pt_lock);
 	return ret;
 }
 
-- 
2.17.1

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

Re: [PATCH] drm: fix call_kern.cocci warnings v2

[Christian König]

Am 25.10.18 um 10:53 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.
>
> Semantic patch information:
>    The proposed change of converting the GFP_KERNEL is not necessarily the
>    correct one.  It may be desired to unlock the lock, or to not call the
>    function under the lock in the first place.
>
> Generated by: scripts/coccinelle/locks/call_kern.cocci

That comment is no longer true.

>
> v2:
> syncobj->timeline still needs protect.
>
> 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>
> ---
>   drivers/gpu/drm/drm_syncobj.c | 25 +++++++++++++------------
>   1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index b7eaa603f368..b843d4c2778c 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -111,15 +111,16 @@ static struct dma_fence
>   				      uint64_t point)
>   {
>   	struct drm_syncobj_signal_pt *signal_pt;
> +	struct dma_fence *f = NULL;
> +	struct drm_syncobj_stub_fence *fence =
> +		kzalloc(sizeof(struct drm_syncobj_stub_fence),
> +			GFP_KERNEL);
>   
> +	if (!fence)
> +		return NULL;
> +	spin_lock(&syncobj->pt_lock);

How about using a single static stub fence like I suggested?

Christian.

>   	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,
> @@ -128,6 +129,7 @@ static struct dma_fence
>   			       point);
>   
>   		dma_fence_signal(&fence->base);
> +		spin_unlock(&syncobj->pt_lock);
>   		return &fence->base;
>   	}
>   
> @@ -137,9 +139,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);
> +	kfree(fence);
> +	return f;
>   }
>   
>   static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj,
> @@ -166,9 +171,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 +382,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;
>   }
>   

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

Re: [PATCH] drm: fix call_kern.cocci warnings v2

[zhoucm1]

[-- Attachment #1.1: Type: text/plain, Size: 696 bytes --]



On 2018年10月25日 16:56, Christian König wrote:
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -111,15 +111,16 @@ static struct dma_fence
>>                         uint64_t point)
>>   {
>>       struct drm_syncobj_signal_pt *signal_pt;
>> +    struct dma_fence *f = NULL;
>> +    struct drm_syncobj_stub_fence *fence =
>> +        kzalloc(sizeof(struct drm_syncobj_stub_fence),
>> +            GFP_KERNEL);
>>   +    if (!fence)
>> +        return NULL;
>> +    spin_lock(&syncobj->pt_lock);
>
> How about using a single static stub fence like I suggested? 
Sorry, I don't get your meanings, how to do that?

Thanks,
David

[-- Attachment #1.2: Type: text/html, Size: 1389 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH] drm: fix call_kern.cocci warnings v2

[Koenig, Christian]

[-- Attachment #1.1: Type: text/plain, Size: 1067 bytes --]

Am 25.10.18 um 11:03 schrieb zhoucm1:


On 2018年10月25日 16:56, Christian König wrote:
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -111,15 +111,16 @@ static struct dma_fence
                        uint64_t point)
  {
      struct drm_syncobj_signal_pt *signal_pt;
+    struct dma_fence *f = NULL;
+    struct drm_syncobj_stub_fence *fence =
+        kzalloc(sizeof(struct drm_syncobj_stub_fence),
+            GFP_KERNEL);
  +    if (!fence)
+        return NULL;
+    spin_lock(&syncobj->pt_lock);

How about using a single static stub fence like I suggested?
Sorry, I don't get your meanings, how to do that?

Add a new function drm_syncobj_stub_fence_init() which is called from drm_core_init() when the module is loaded.

In drm_syncobj_stub_fence_init() you initialize one static stub_fence which is then used over and over again.

Since its reference count never goes down to zero it should never be freed. In doubt maybe add a .free callback which just calls BUG() to catch reference count issues.

Christian.


Thanks,
David


[-- Attachment #1.2: Type: text/html, Size: 2180 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH] drm: fix call_kern.cocci warnings v2

[Maarten Lankhorst]

Op 25-10-18 om 10:53 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.
>
> Semantic patch information:
>   The proposed change of converting the GFP_KERNEL is not necessarily the
>   correct one.  It may be desired to unlock the lock, or to not call the
>   function under the lock in the first place.
>
> Generated by: scripts/coccinelle/locks/call_kern.cocci
>
> v2:
> syncobj->timeline still needs protect.
>
> 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>
> ---
>  drivers/gpu/drm/drm_syncobj.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index b7eaa603f368..b843d4c2778c 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -111,15 +111,16 @@ static struct dma_fence
>  				      uint64_t point)
>  {
>  	struct drm_syncobj_signal_pt *signal_pt;
> +	struct dma_fence *f = NULL;
> +	struct drm_syncobj_stub_fence *fence =
> +		kzalloc(sizeof(struct drm_syncobj_stub_fence),
> +			GFP_KERNEL);
>  
> +	if (!fence)
> +		return 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,
> @@ -128,6 +129,7 @@ static struct dma_fence
>  			       point);
>  
>  		dma_fence_signal(&fence->base);
> +		spin_unlock(&syncobj->pt_lock);
>  		return &fence->base;
>  	}
>  
> @@ -137,9 +139,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);
> +	kfree(fence);
> +	return f;
>  }
>  
>  static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj,
> @@ -166,9 +171,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);
Maybe slightly offtopic, but seems that this function should return an error if fence == NULL,
at least when it's a result of -ENOMEM.
It would be nice if this was sent as a separate patch, instead of waiting indefinitely..


> @@ -379,11 +382,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;
There's no way to make a destinction between -ENOMEM and -EINVAL, I think it would be good to use ERR_PTR, and then
return PTR_ERR_OR_ZERO()..

I would also change the subject line to something more descriptive, rather than the generic one it's now. But otherwise looks good to me. :)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm: fix call_kern.cocci warnings v2

[zhoucm1]

[-- Attachment #1.1: Type: text/plain, Size: 1347 bytes --]



On 2018年10月25日 17:11, Koenig, Christian wrote:
> Am 25.10.18 um 11:03 schrieb zhoucm1:
>>
>>
>>
>> On 2018年10月25日 16:56, Christian König wrote:
>>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>>> @@ -111,15 +111,16 @@ static struct dma_fence
>>>>                         uint64_t point)
>>>>   {
>>>>       struct drm_syncobj_signal_pt *signal_pt;
>>>> +    struct dma_fence *f = NULL;
>>>> +    struct drm_syncobj_stub_fence *fence =
>>>> +        kzalloc(sizeof(struct drm_syncobj_stub_fence),
>>>> +            GFP_KERNEL);
>>>>   +    if (!fence)
>>>> +        return NULL;
>>>> +    spin_lock(&syncobj->pt_lock);
>>>
>>> How about using a single static stub fence like I suggested? 
>> Sorry, I don't get your meanings, how to do that?
>
> Add a new function drm_syncobj_stub_fence_init() which is called from 
> drm_core_init() when the module is loaded.
>
> In drm_syncobj_stub_fence_init() you initialize one static stub_fence 
> which is then used over and over again.
Seems it would not work, we could need more than one stub fence.

David
>
> Since its reference count never goes down to zero it should never be 
> freed. In doubt maybe add a .free callback which just calls BUG() to 
> catch reference count issues.
>
> Christian.
>
>>
>> Thanks,
>> David
>


[-- Attachment #1.2: Type: text/html, Size: 2791 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH] drm: fix call_kern.cocci warnings v2

[Koenig, Christian]

[-- Attachment #1.1: Type: text/plain, Size: 1352 bytes --]

Am 25.10.18 um 11:20 schrieb zhoucm1:


On 2018年10月25日 17:11, Koenig, Christian wrote:
Am 25.10.18 um 11:03 schrieb zhoucm1:


On 2018年10月25日 16:56, Christian König wrote:
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -111,15 +111,16 @@ static struct dma_fence
                        uint64_t point)
  {
      struct drm_syncobj_signal_pt *signal_pt;
+    struct dma_fence *f = NULL;
+    struct drm_syncobj_stub_fence *fence =
+        kzalloc(sizeof(struct drm_syncobj_stub_fence),
+            GFP_KERNEL);
  +    if (!fence)
+        return NULL;
+    spin_lock(&syncobj->pt_lock);

How about using a single static stub fence like I suggested?
Sorry, I don't get your meanings, how to do that?

Add a new function drm_syncobj_stub_fence_init() which is called from drm_core_init() when the module is loaded.

In drm_syncobj_stub_fence_init() you initialize one static stub_fence which is then used over and over again.
Seems it would not work, we could need more than one stub fence.

Mhm, why? I mean it is just a signaled fence, context and sequence number are irrelevant.

Christian.


David

Since its reference count never goes down to zero it should never be freed. In doubt maybe add a .free callback which just calls BUG() to catch reference count issues.

Christian.


Thanks,
David




[-- Attachment #1.2: Type: text/html, Size: 2989 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH] drm: fix call_kern.cocci warnings v2

[zhoucm1]

[-- Attachment #1.1: Type: text/plain, Size: 1801 bytes --]



On 2018年10月25日 17:23, Koenig, Christian wrote:
> Am 25.10.18 um 11:20 schrieb zhoucm1:
>>
>>
>>
>> On 2018年10月25日 17:11, Koenig, Christian wrote:
>>> Am 25.10.18 um 11:03 schrieb zhoucm1:
>>>>
>>>>
>>>>
>>>> On 2018年10月25日 16:56, Christian König wrote:
>>>>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>>>>> @@ -111,15 +111,16 @@ static struct dma_fence
>>>>>>                         uint64_t point)
>>>>>>   {
>>>>>>       struct drm_syncobj_signal_pt *signal_pt;
>>>>>> +    struct dma_fence *f = NULL;
>>>>>> +    struct drm_syncobj_stub_fence *fence =
>>>>>> +        kzalloc(sizeof(struct drm_syncobj_stub_fence),
>>>>>> +            GFP_KERNEL);
>>>>>>   +    if (!fence)
>>>>>> +        return NULL;
>>>>>> +    spin_lock(&syncobj->pt_lock);
>>>>>
>>>>> How about using a single static stub fence like I suggested? 
>>>> Sorry, I don't get your meanings, how to do that?
>>>
>>> Add a new function drm_syncobj_stub_fence_init() which is called 
>>> from drm_core_init() when the module is loaded.
>>>
>>> In drm_syncobj_stub_fence_init() you initialize one static 
>>> stub_fence which is then used over and over again.
>> Seems it would not work, we could need more than one stub fence.
>
> Mhm, why? I mean it is just a signaled fence,

If A gets the global stub fence, doesn't put it yet, then B is coming, 
how does B re-use the global stub fence?  anything I misunderstand?

David
> context and sequence number are irrelevant.
>
> Christian.
>
>>
>> David
>>>
>>> Since its reference count never goes down to zero it should never be 
>>> freed. In doubt maybe add a .free callback which just calls BUG() to 
>>> catch reference count issues.
>>>
>>> Christian.
>>>
>>>>
>>>> Thanks,
>>>> David
>>>
>>
>


[-- Attachment #1.2: Type: text/html, Size: 4115 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH] drm: fix call_kern.cocci warnings v2

[Koenig, Christian]

[-- Attachment #1.1: Type: text/plain, Size: 1824 bytes --]

Am 25.10.18 um 11:28 schrieb zhoucm1:


On 2018年10月25日 17:23, Koenig, Christian wrote:
Am 25.10.18 um 11:20 schrieb zhoucm1:


On 2018年10月25日 17:11, Koenig, Christian wrote:
Am 25.10.18 um 11:03 schrieb zhoucm1:


On 2018年10月25日 16:56, Christian König wrote:
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -111,15 +111,16 @@ static struct dma_fence
                        uint64_t point)
  {
      struct drm_syncobj_signal_pt *signal_pt;
+    struct dma_fence *f = NULL;
+    struct drm_syncobj_stub_fence *fence =
+        kzalloc(sizeof(struct drm_syncobj_stub_fence),
+            GFP_KERNEL);
  +    if (!fence)
+        return NULL;
+    spin_lock(&syncobj->pt_lock);

How about using a single static stub fence like I suggested?
Sorry, I don't get your meanings, how to do that?

Add a new function drm_syncobj_stub_fence_init() which is called from drm_core_init() when the module is loaded.

In drm_syncobj_stub_fence_init() you initialize one static stub_fence which is then used over and over again.
Seems it would not work, we could need more than one stub fence.

Mhm, why? I mean it is just a signaled fence,

If A gets the global stub fence, doesn't put it yet, then B is coming, how does B re-use the global stub fence?  anything I misunderstand?

dma_fence_get()? The whole thing is reference counted, every time you need it you grab another reference.

Since we globally initialize it the reference never becomes zero, so it is never released.

Christian.


David
context and sequence number are irrelevant.

Christian.


David

Since its reference count never goes down to zero it should never be freed. In doubt maybe add a .free callback which just calls BUG() to catch reference count issues.

Christian.


Thanks,
David






[-- Attachment #1.2: Type: text/html, Size: 3999 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH] drm: fix call_kern.cocci warnings v2

[zhoucm1]

[-- Attachment #1.1: Type: text/plain, Size: 2363 bytes --]



On 2018年10月25日 17:30, Koenig, Christian wrote:
> Am 25.10.18 um 11:28 schrieb zhoucm1:
>>
>>
>>
>> On 2018年10月25日 17:23, Koenig, Christian wrote:
>>> Am 25.10.18 um 11:20 schrieb zhoucm1:
>>>>
>>>>
>>>>
>>>> On 2018年10月25日 17:11, Koenig, Christian wrote:
>>>>> Am 25.10.18 um 11:03 schrieb zhoucm1:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 2018年10月25日 16:56, Christian König wrote:
>>>>>>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>>>>>>> @@ -111,15 +111,16 @@ static struct dma_fence
>>>>>>>>                         uint64_t point)
>>>>>>>>   {
>>>>>>>>       struct drm_syncobj_signal_pt *signal_pt;
>>>>>>>> +    struct dma_fence *f = NULL;
>>>>>>>> +    struct drm_syncobj_stub_fence *fence =
>>>>>>>> +        kzalloc(sizeof(struct drm_syncobj_stub_fence),
>>>>>>>> +            GFP_KERNEL);
>>>>>>>>   +    if (!fence)
>>>>>>>> +        return NULL;
>>>>>>>> +    spin_lock(&syncobj->pt_lock);
>>>>>>>
>>>>>>> How about using a single static stub fence like I suggested? 
>>>>>> Sorry, I don't get your meanings, how to do that?
>>>>>
>>>>> Add a new function drm_syncobj_stub_fence_init() which is called 
>>>>> from drm_core_init() when the module is loaded.
>>>>>
>>>>> In drm_syncobj_stub_fence_init() you initialize one static 
>>>>> stub_fence which is then used over and over again.
>>>> Seems it would not work, we could need more than one stub fence.
>>>
>>> Mhm, why? I mean it is just a signaled fence,
>>
>> If A gets the global stub fence, doesn't put it yet, then B is 
>> coming, how does B re-use the global stub fence?  anything I 
>> misunderstand?
>
> dma_fence_get()? The whole thing is reference counted, every time you 
> need it you grab another reference.
>
> Since we globally initialize it the reference never becomes zero, so 
> it is never released.
I got your means now, always a signaled fence, no need re-initialize it, 
that's ok.

Thanks,
David

>
> Christian.
>
>>
>> David
>>> context and sequence number are irrelevant.
>>>
>>> Christian.
>>>
>>>>
>>>> David
>>>>>
>>>>> Since its reference count never goes down to zero it should never 
>>>>> be freed. In doubt maybe add a .free callback which just calls 
>>>>> BUG() to catch reference count issues.
>>>>>
>>>>> Christian.
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>
>>>>
>>>
>>
>


[-- Attachment #1.2: Type: text/html, Size: 5662 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

✓ Fi.CI.BAT: success for drm: fix call_kern.cocci warnings (rev2)

[Patchwork]

== Series Details ==

Series: drm: fix call_kern.cocci warnings (rev2)
URL   : https://patchwork.freedesktop.org/series/51498/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5033 -> Patchwork_10574 =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10574 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10574, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

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

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10574:

  === IGT changes ===

    ==== Warnings ====

    igt@pm_rpm@module-reload:
      fi-hsw-4770r:       SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@debugfs_test@read_all_entries:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106097)

    igt@drv_module_reload@basic-reload:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106725, fdo#106248)

    igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
      fi-glk-j4005:       PASS -> FAIL (fdo#106765)

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-glk-j4005:       PASS -> FAIL (fdo#100368)

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

    igt@pm_rpm@module-reload:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#107726)

    
    ==== Possible fixes ====

    igt@drv_module_reload@basic-reload-inject:
      fi-hsw-4770r:       DMESG-WARN (fdo#107425, fdo#107924) -> PASS

    igt@drv_selftest@live_hangcheck:
      fi-kbl-7560u:       INCOMPLETE (fdo#108044) -> PASS

    igt@kms_flip@basic-flip-vs-dpms:
      fi-skl-6700hq:      DMESG-WARN (fdo#105998) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
  fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097
  fdo#106248 https://bugs.freedesktop.org/show_bug.cgi?id=106248
  fdo#106725 https://bugs.freedesktop.org/show_bug.cgi?id=106725
  fdo#106765 https://bugs.freedesktop.org/show_bug.cgi?id=106765
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107425 https://bugs.freedesktop.org/show_bug.cgi?id=107425
  fdo#107726 https://bugs.freedesktop.org/show_bug.cgi?id=107726
  fdo#107924 https://bugs.freedesktop.org/show_bug.cgi?id=107924
  fdo#108044 https://bugs.freedesktop.org/show_bug.cgi?id=108044


== Participating hosts (47 -> 42) ==

  Missing    (5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-u 


== Build changes ==

    * Linux: CI_DRM_5033 -> Patchwork_10574

  CI_DRM_5033: f935e4c7634781e6ffef10bb8a1c93225ac42d90 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4691: d445be01f5edc7e7a324444c73e221c9ed75602e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10574: baf18168e28f745bc632702f6029c30cecd253d2 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

baf18168e28f drm: fix call_kern.cocci warnings v2

== Logs ==

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