[PATCH v2] drm/syncobj: Deal with signalled fences in drm_syncobj_find_fence.

[PATCH v2] drm/syncobj: Deal with signalled fences in drm_syncobj_find_fence.

[Bas Nieuwenhuizen]

dma_fence_chain_find_seqno only ever returns the top fence in the
chain or an unsignalled fence. Hence if we request a seqno that
is already signalled it returns a NULL fence. Some callers are
not prepared to handle this, like the syncobj transfer functions
for example.

This behavior is "new" with timeline syncobj and it looks like
not all callers were updated. To fix this behavior make sure
that a successful drm_sync_find_fence always returns a non-NULL
fence.

v2: Move the fix to drm_syncobj_find_fence from the transfer
    functions.

Fixes: ea569910cbab ("drm/syncobj: add transition iotcls between binary and timeline v2")
Cc: stable@vger.kernel.org
Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
 drivers/gpu/drm/drm_syncobj.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index fdd2ec87cdd1..11be91b5709b 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -404,8 +404,17 @@ int drm_syncobj_find_fence(struct drm_file *file_private,
 
 	if (*fence) {
 		ret = dma_fence_chain_find_seqno(fence, point);
-		if (!ret)
+		if (!ret) {
+			/* If the requested seqno is already signaled
+			 * drm_syncobj_find_fence may return a NULL
+			 * fence. To make sure the recipient gets
+			 * signalled, use a new fence instead.
+			 */
+			if (!*fence)
+				*fence = dma_fence_get_stub();
+
 			goto out;
+		}
 		dma_fence_put(*fence);
 	} else {
 		ret = -EINVAL;
-- 
2.34.1

Re: [PATCH v2] drm/syncobj: Deal with signalled fences in drm_syncobj_find_fence.

[Christian König]

Am 08.12.21 um 03:39 schrieb Bas Nieuwenhuizen:
> dma_fence_chain_find_seqno only ever returns the top fence in the
> chain or an unsignalled fence. Hence if we request a seqno that
> is already signalled it returns a NULL fence. Some callers are
> not prepared to handle this, like the syncobj transfer functions
> for example.
>
> This behavior is "new" with timeline syncobj and it looks like
> not all callers were updated. To fix this behavior make sure
> that a successful drm_sync_find_fence always returns a non-NULL
> fence.
>
> v2: Move the fix to drm_syncobj_find_fence from the transfer
>      functions.
>
> Fixes: ea569910cbab ("drm/syncobj: add transition iotcls between binary and timeline v2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>

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

> ---
>   drivers/gpu/drm/drm_syncobj.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index fdd2ec87cdd1..11be91b5709b 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -404,8 +404,17 @@ int drm_syncobj_find_fence(struct drm_file *file_private,
>   
>   	if (*fence) {
>   		ret = dma_fence_chain_find_seqno(fence, point);
> -		if (!ret)
> +		if (!ret) {
> +			/* If the requested seqno is already signaled
> +			 * drm_syncobj_find_fence may return a NULL
> +			 * fence. To make sure the recipient gets
> +			 * signalled, use a new fence instead.
> +			 */
> +			if (!*fence)
> +				*fence = dma_fence_get_stub();
> +
>   			goto out;
> +		}
>   		dma_fence_put(*fence);
>   	} else {
>   		ret = -EINVAL;

Re: [PATCH v2] drm/syncobj: Deal with signalled fences in drm_syncobj_find_fence.

[Lionel Landwerlin]

On 08/12/2021 11:28, Christian König wrote:
> Am 08.12.21 um 03:39 schrieb Bas Nieuwenhuizen:
>> dma_fence_chain_find_seqno only ever returns the top fence in the
>> chain or an unsignalled fence. Hence if we request a seqno that
>> is already signalled it returns a NULL fence. Some callers are
>> not prepared to handle this, like the syncobj transfer functions
>> for example.
>>
>> This behavior is "new" with timeline syncobj and it looks like
>> not all callers were updated. To fix this behavior make sure
>> that a successful drm_sync_find_fence always returns a non-NULL
>> fence.
>>
>> v2: Move the fix to drm_syncobj_find_fence from the transfer
>>      functions.
>>
>> Fixes: ea569910cbab ("drm/syncobj: add transition iotcls between 
>> binary and timeline v2")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
>
> Reviewed-by: Christian König <christian.koenig@amd.com>


Thanks!


Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>


>
>> ---
>>   drivers/gpu/drm/drm_syncobj.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_syncobj.c 
>> b/drivers/gpu/drm/drm_syncobj.c
>> index fdd2ec87cdd1..11be91b5709b 100644
>> --- a/drivers/gpu/drm/drm_syncobj.c
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -404,8 +404,17 @@ int drm_syncobj_find_fence(struct drm_file 
>> *file_private,
>>         if (*fence) {
>>           ret = dma_fence_chain_find_seqno(fence, point);
>> -        if (!ret)
>> +        if (!ret) {
>> +            /* If the requested seqno is already signaled
>> +             * drm_syncobj_find_fence may return a NULL
>> +             * fence. To make sure the recipient gets
>> +             * signalled, use a new fence instead.
>> +             */
>> +            if (!*fence)
>> +                *fence = dma_fence_get_stub();
>> +
>>               goto out;
>> +        }
>>           dma_fence_put(*fence);
>>       } else {
>>           ret = -EINVAL;
>

Re: [PATCH v2] drm/syncobj: Deal with signalled fences in drm_syncobj_find_fence.

[Christian König]

Am 08.12.21 um 11:08 schrieb Lionel Landwerlin:
> On 08/12/2021 11:28, Christian König wrote:
>> Am 08.12.21 um 03:39 schrieb Bas Nieuwenhuizen:
>>> dma_fence_chain_find_seqno only ever returns the top fence in the
>>> chain or an unsignalled fence. Hence if we request a seqno that
>>> is already signalled it returns a NULL fence. Some callers are
>>> not prepared to handle this, like the syncobj transfer functions
>>> for example.
>>>
>>> This behavior is "new" with timeline syncobj and it looks like
>>> not all callers were updated. To fix this behavior make sure
>>> that a successful drm_sync_find_fence always returns a non-NULL
>>> fence.
>>>
>>> v2: Move the fix to drm_syncobj_find_fence from the transfer
>>>      functions.
>>>
>>> Fixes: ea569910cbab ("drm/syncobj: add transition iotcls between 
>>> binary and timeline v2")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
>>
>> Reviewed-by: Christian König <christian.koenig@amd.com>
>
>
> Thanks!
>
>
> Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

And pushed to drm-misc-fixes.

Thanks,
Christian.

>
>
>>
>>> ---
>>>   drivers/gpu/drm/drm_syncobj.c | 11 ++++++++++-
>>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c 
>>> b/drivers/gpu/drm/drm_syncobj.c
>>> index fdd2ec87cdd1..11be91b5709b 100644
>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>> @@ -404,8 +404,17 @@ int drm_syncobj_find_fence(struct drm_file 
>>> *file_private,
>>>         if (*fence) {
>>>           ret = dma_fence_chain_find_seqno(fence, point);
>>> -        if (!ret)
>>> +        if (!ret) {
>>> +            /* If the requested seqno is already signaled
>>> +             * drm_syncobj_find_fence may return a NULL
>>> +             * fence. To make sure the recipient gets
>>> +             * signalled, use a new fence instead.
>>> +             */
>>> +            if (!*fence)
>>> +                *fence = dma_fence_get_stub();
>>> +
>>>               goto out;
>>> +        }
>>>           dma_fence_put(*fence);
>>>       } else {
>>>           ret = -EINVAL;
>>
>

[PATCH v2] drm/syncobj: Deal with signalled fences in drm_syncobj_find_fence.

[Bas Nieuwenhuizen]

dma_fence_chain_find_seqno only ever returns the top fence in the
chain or an unsignalled fence. Hence if we request a seqno that
is already signalled it returns a NULL fence. Some callers are
not prepared to handle this, like the syncobj transfer functions
for example.

This behavior is "new" with timeline syncobj and it looks like
not all callers were updated. To fix this behavior make sure
that a successful drm_sync_find_fence always returns a non-NULL
fence.

v2: Move the fix to drm_syncobj_find_fence from the transfer
    functions.

Fixes: ea569910cbab ("drm/syncobj: add transition iotcls between binary and timeline v2")
Cc: stable@vger.kernel.org
Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
 drivers/gpu/drm/drm_syncobj.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index fdd2ec87cdd1..e772ca3e1e13 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -404,8 +404,17 @@ int drm_syncobj_find_fence(struct drm_file *file_private,
 
 	if (*fence) {
 		ret = dma_fence_chain_find_seqno(fence, point);
-		if (!ret)
+		if (!ret) {
+			/* If the requested seqno is already signaled
+			 * drm_syncobj_find_fence may return a NULL
+			 * fence. To make sure the recipient gets
+			 * signalled, use a new fence instead.
+			 */
+			if (!*fence)
+				*fence = dma_fence_get_stub();
+
 			goto out;
+		}
 		dma_fence_put(*fence);
 	} else {
 		ret = -EINVAL;
@@ -861,6 +870,7 @@ static int drm_syncobj_transfer_to_timeline(struct drm_file *file_private,
 				     &fence);
 	if (ret)
 		goto err;
+
 	chain = kzalloc(sizeof(struct dma_fence_chain), GFP_KERNEL);
 	if (!chain) {
 		ret = -ENOMEM;
@@ -890,6 +900,7 @@ drm_syncobj_transfer_to_binary(struct drm_file *file_private,
 				     args->src_point, args->flags, &fence);
 	if (ret)
 		goto err;
+
 	drm_syncobj_replace_fence(binary_syncobj, fence);
 	dma_fence_put(fence);
 err:
-- 
2.34.1