Re: [PATCH] drm/amdgpu: return reserve errors from amdgpu_gfx_kiq_init()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Memory and resource leak of the KIQ buffer object on the error path.
--

--- Patch [1]: [PATCH] drm/amdgpu: return reserve errors from amdgpu_gfx_kiq_init() ---
commit daee9bc709b5f169cbd5d109e86c30d44267139a
Author: Runyu Xiao <[email protected]>

drm/amdgpu: return reserve errors from amdgpu_gfx_kiq_init()

This patch updates amdgpu_gfx_kiq_init() to explicitly return the error code
if amdgpu_bo_reserve() fails. It also ensures that amdgpu_bo_unreserve() is
only called on the successful reservation path.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index a6f95ff47d24e..3e10a90071ffb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -371,8 +371,11 @@ int amdgpu_gfx_kiq_init(struct amdgpu_device *adev,
>  	memset(hpd, 0, hpd_size);
>  
>  	r = amdgpu_bo_reserve(kiq->eop_obj, true);
> -	if (unlikely(r != 0))
> +	if (unlikely(r != 0)) {
>  		dev_warn(adev->dev, "(%d) reserve kiq eop bo failed\n", r);
> +		amdgpu_bo_kunmap(kiq->eop_obj);
> +		return r;

[Severity: Medium]
Does this code leak the newly created buffer object on this error path?

Looking earlier in amdgpu_gfx_kiq_init(), kiq->eop_obj is allocated with
amdgpu_bo_create_kernel():

	r = amdgpu_bo_create_kernel(adev, hpd_size, PAGE_SIZE,
				    AMDGPU_GEM_DOMAIN_GTT, &kiq->eop_obj,
				    &kiq->eop_gpu_addr, (void **)&hpd);

If we return r directly here without calling amdgpu_bo_free_kernel(), 
it appears the buffer object is leaked. 

Since returning an error causes sw_init to fail, the IP block's status.sw
flag won't be set, and amdgpu_gfx_kiq_fini() will be bypassed during driver
teardown, leaving no way to clean up this allocation later.

> +	}
>  	amdgpu_bo_kunmap(kiq->eop_obj);
>  	amdgpu_bo_unreserve(kiq->eop_obj);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1