Re: [Patch v1 2/4] drm/amdgpu/userq: no need to use local variable here for return

["Khatri, Sunil" <sukhatri@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 1957 bytes --]


On 26-03-2026 05:39 pm, Christian König wrote:
>
> On 3/26/26 09:55, Sunil Khatri wrote:
>> In function amdgpu_userq_restore_worker use directly
>> the function's return value in the if condition instead
>> of local variable ret.
>>
>> Signed-off-by: Sunil Khatri<sunil.khatri@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 7 ++-----
>>   1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> index 2a1832fce6d2..2b07c3941927 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> @@ -1222,20 +1222,17 @@ static void amdgpu_userq_restore_worker(struct work_struct *work)
>>   	struct amdgpu_userq_mgr *uq_mgr = work_to_uq_mgr(work, resume_work.work);
>>   	struct amdgpu_fpriv *fpriv = uq_mgr_to_fpriv(uq_mgr);
>>   	struct dma_fence *ev_fence;
>> -	int ret;
>>   
>>   	ev_fence = amdgpu_evf_mgr_get_fence(&fpriv->evf_mgr);
>>   	if (!dma_fence_is_signaled(ev_fence))
>>   		goto put_fence;
>>   
>> -	ret = amdgpu_userq_vm_validate(uq_mgr);
>> -	if (ret) {
>> +	if (amdgpu_userq_vm_validate(uq_mgr)) {
>>   		drm_file_err(uq_mgr->file, "Failed to validate BOs to restore\n");
> Again, probably a good idea to print the error code here.
since this is a void function and all the functions called here are 
already printing the failures and we could just remove the if and let 
the called function print error.
>
> Regards,
> Christian.
>
>>   		goto put_fence;
>>   	}
>>   
>> -	ret = amdgpu_userq_restore_all(uq_mgr);
>> -	if (ret)
>> +	if (amdgpu_userq_restore_all(uq_mgr))
>>   		drm_file_err(uq_mgr->file, "Failed to restore all queues\n");
Could avoid printing this error all together as we are printing the 
error in the function. We could update the error message.
if(ret)
drm_file_err(uq_mgr->file, "Failed to map all the queues\n");
returnret;


>>   
>>   put_fence:

[-- Attachment #2: Type: text/html, Size: 4039 bytes --]