[PATCH] drm/amdgpu: Make amdgpu_dma_buf_attach() return paths consistent

[PATCH] drm/amdgpu: Make amdgpu_dma_buf_attach() return paths consistent

[Srinivasan Shanmugam]

amdgpu_dma_buf_attach() locks bo->tbo.base.resv before updating the BO
sharing state and unlocks it before returning.

Return the local status variable after the unlock so the function has a
single consistent success return path, which avoids the Smatch warning
about inconsistent reservation lock handling.

Fixes the below:
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c:111 amdgpu_dma_buf_attach() warn: inconsistent returns 'bo->tbo.base.resv'.

Fixes: 6e6db2722c28 ("drm/amdgpu: add independent DMA-buf export v8")
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 656c267dbe58..9cf240ad5471 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -108,7 +108,7 @@ static int amdgpu_dma_buf_attach(struct dma_buf *dmabuf,
 
 	dma_resv_unlock(bo->tbo.base.resv);
 
-	return 0;
+	return r;
 }
 
 /**
-- 
2.34.1

Re: [PATCH] drm/amdgpu: Make amdgpu_dma_buf_attach() return paths consistent

[Christian König]

On 3/12/26 14:44, Srinivasan Shanmugam wrote:
> amdgpu_dma_buf_attach() locks bo->tbo.base.resv before updating the BO
> sharing state and unlocks it before returning.
> 
> Return the local status variable after the unlock so the function has a
> single consistent success return path, which avoids the Smatch warning
> about inconsistent reservation lock handling.

Mhm, that doesn't looks correct to me.

> 
> Fixes the below:
> drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c:111 amdgpu_dma_buf_attach() warn: inconsistent returns 'bo->tbo.base.resv'.

As far as I can see neither that warning nor the fact that returning the error (which is always 0 at that point) would fix it makes sense to me.

What exactly is going on here?

Regards,
Christian.

> 
> Fixes: 6e6db2722c28 ("drm/amdgpu: add independent DMA-buf export v8")
> Cc: Dan Carpenter <dan.carpenter@linaro.org>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> index 656c267dbe58..9cf240ad5471 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> @@ -108,7 +108,7 @@ static int amdgpu_dma_buf_attach(struct dma_buf *dmabuf,
>  
>  	dma_resv_unlock(bo->tbo.base.resv);
>  
> -	return 0;
> +	return r;
>  }
>  
>  /**

RE: [PATCH] drm/amdgpu: Make amdgpu_dma_buf_attach() return paths consistent

[SHANMUGAM, SRINIVASAN]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig@amd.com>
> Sent: Thursday, March 12, 2026 7:34 PM
> To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>;
> Deucher, Alexander <Alexander.Deucher@amd.com>
> Cc: amd-gfx@lists.freedesktop.org; Dan Carpenter <dan.carpenter@linaro.org>
> Subject: Re: [PATCH] drm/amdgpu: Make amdgpu_dma_buf_attach() return paths
> consistent
>
> On 3/12/26 14:44, Srinivasan Shanmugam wrote:
> > amdgpu_dma_buf_attach() locks bo->tbo.base.resv before updating the BO
> > sharing state and unlocks it before returning.
> >
> > Return the local status variable after the unlock so the function has
> > a single consistent success return path, which avoids the Smatch
> > warning about inconsistent reservation lock handling.
>
> Mhm, that doesn't looks correct to me.
>
> >
> > Fixes the below:
> > drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c:111 amdgpu_dma_buf_attach()
> warn: inconsistent returns 'bo->tbo.base.resv'.
>
> As far as I can see neither that warning nor the fact that returning the error (which is
> always 0 at that point) would fix it makes sense to me.
>
> What exactly is going on here?

Hi Christian,

Thanks for taking a look.

My understanding was that Smatch was complaining about inconsistent lock
state tracking for bo->tbo.base.resv. The function locks the reservation
object with:

r = dma_resv_lock(bo->tbo.base.resv, NULL);
if (r)
        return r;

and later unlocks it before returning:

dma_resv_unlock(bo->tbo.base.resv);
return 0;

I assumed Smatch was confused by the different return paths and suggested
returning r instead of 0 so that the function consistently returns the
same variable after the unlock

the proposed change does not actually address the reported warning.
r is always 0 after a successful dma_resv_lock(), so returning
r instead of 0 does not change the behavior.

So this looks like a Smatch false positive rather than a real issue.
I'll drop this patch.

Thanks for pointing this out.

Regards,
Srini

>
> Regards,
> Christian.
>

Re: [PATCH] drm/amdgpu: Make amdgpu_dma_buf_attach() return paths consistent

[Dan Carpenter]

On Thu, Mar 12, 2026 at 07:14:15PM +0530, Srinivasan Shanmugam wrote:
> amdgpu_dma_buf_attach() locks bo->tbo.base.resv before updating the BO
> sharing state and unlocks it before returning.
> 
> Return the local status variable after the unlock so the function has a
> single consistent success return path, which avoids the Smatch warning
> about inconsistent reservation lock handling.
> 
> Fixes the below:
> drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c:111 amdgpu_dma_buf_attach() warn: inconsistent returns 'bo->tbo.base.resv'.
> 

This seems like a false positive to me and I don't like this solution
very much really... :/

I don't get this warning on my system on linux-next.  I have tested the
released version with and without the database...  :/

regards,
dan carpenter