Re: [RFC PATCH v2 1/3] drm/syncobj: Add flag DRM_SYNCOBJ_QUERY_FLAGS_ERROR to query errors

[Yicong Hui <yiconghui@gmail.com>]

On 2/20/26 5:07 PM, Christian König wrote:
> On 2/20/26 18:05, Yicong Hui wrote:
>>>> +
>>>>    /**
>>>>     * dma_fence_chain_find_seqno - find fence chain node by seqno
>>>>     * @pfence: pointer to the chain node where to start
>>>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>>>> index 2d4ab745fdad..322f64b72775 100644
>>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>>> @@ -1654,14 +1654,17 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>>>>    {
>>>>        struct drm_syncobj_timeline_array *args = data;
>>>>        struct drm_syncobj **syncobjs;
>>>> +    unsigned int valid_flags = DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED |
>>>> +                   DRM_SYNCOBJ_QUERY_FLAGS_ERROR;
>>>>        uint64_t __user *points = u64_to_user_ptr(args->points);
>>>> +    uint64_t __user *handles = u64_to_user_ptr(args->handles);
>>>>        uint32_t i;
>>>>        int ret;
>>>>          if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>>>>            return -EOPNOTSUPP;
>>>>    -    if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
>>>> +    if (args->flags & ~valid_flags)
>>>>            return -EINVAL;
>>>>          if (args->count_handles == 0)
>>>> @@ -1680,6 +1683,22 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>>>>            uint64_t point;
>>>
>>> Make a local variable "int error" here.
>>>
>>> ...
>>>
>>>> +            int64_t error = 0;
>>>
>>> The error code is an int and only 32bits.
>> Understood, will change that!
>>
>>>
>>>> +
>>>> +            if (fence)
>>>> +                error = dma_fence_chain_find_error(fence);
>>>> +
>>>> +            ret = copy_to_user(&handles[i], &error, sizeof(int64_t));
>>>
>>> The handles are also only 32bits!
>> Ah, that's my mistake. Was thrown off by the __u64 in the struct definition but it is obvious now that it is a u32. Fixed as well!
>>
>>>
>>>> +            ret = ret ? -EFAULT : 0;
>>>> +            if (ret) {
>>>> +                dma_fence_put(fence);
>>>> +                break;
>>>> +            }
>>>> +
>>>> +        }
>>>> +
>>>>            chain = to_dma_fence_chain(fence);
>>>>            if (chain) {
>>>
>>> In this code path whenever point is assigned something do a "error = dma_fence_get_status(fence);" and then eventually copy the error to userspace after copying the point.
>>>
>>
>> Hi! Were you thinking something that looks a little bit like this?
> 
> Yeah that looks like what I had in mind to.
> 
> Thanks,
> Christian.
> 

Hi! What should I do at this point? Send a v3, or..?

Thanks!
Yicong