Re: [PATCH 04/11] drm/amdgpu: rework amdgpu_userq_signal_ioctl

["Christian König" <christian.koenig@amd.com>]

Hi guys,

On 4/23/26 11:58, Liang, Prike wrote:
...
>> -static int amdgpu_userq_fence_alloc(struct amdgpu_userq_fence **userq_fence)
>> +static int amdgpu_userq_fence_alloc(struct amdgpu_usermode_queue *userq,
>> +                                 struct amdgpu_userq_fence **pfence)
>>  {
>> -     *userq_fence = kmalloc(sizeof(**userq_fence), GFP_ATOMIC);
>> -     return *userq_fence ? 0 : -ENOMEM;
>> +     struct amdgpu_userq_fence_driver *fence_drv = userq->fence_drv;
>> +     struct amdgpu_userq_fence *userq_fence;
>> +     unsigned long count;
> We must initialize count; otherwise, it may contain a garbage value, which can cause amdgpu_userq_fence_alloc() to fail and,
> in turn, make userq fence emission fail.

I've got the same comment from both Sunil and Prike but as far as I can see  and that is actually incorrect.

> 
>> +     userq_fence = kmalloc(sizeof(*userq_fence), GFP_KERNEL);
>> +     if (!userq_fence)
>> +             return -ENOMEM;
>> +
>> +     /*
>> +      * Get the next unused entry, since we fill from the start this can be
>> +      * used as size to allocate the array.
>> +      */
>> +     mutex_lock(&userq->fence_drv_lock);
>> +     xa_find(&userq->fence_drv_xa, &count, ULONG_MAX, XA_FREE_MARK);

The count should be initialized here. But could be that this doesn't work.

Did you guys got a KASAN warning or something like that?

>> +
>> +     userq_fence->fence_drv_array = kvmalloc_array(count, sizeof(fence_drv),
>> +                                                   GFP_KERNEL);
>> +     if (!userq_fence->fence_drv_array) {
>> +             mutex_unlock(&userq->fence_drv_lock);
>> +             kfree(userq_fence);
>> +             return -ENOMEM;
>> +     }
>> +
>> +     userq_fence->fence_drv_array_count = count;
>> +     xa_extract(&userq->fence_drv_xa, (void **)userq_fence->fence_drv_array,
>> +                0, ULONG_MAX, count, XA_PRESENT);
> We may need to assign the userq_fence->fence_drv_array_count the exact copied number from the xa_extract().

Interresting point. Why could that differ ?

Thanks for the comments,
Christian.