[Public]

> -----Original Message-----
> From: Clement, Sunday <[email protected]>
> Sent: Monday, October 27, 2025 2:08 PM
> To: [email protected]
> Cc: Kasiviswanathan, Harish <[email protected]>; Kim, Jonathan
> <[email protected]>; Kuehling, Felix <[email protected]>; Clement,
> Sunday <[email protected]>
> Subject: [PATCH] drm/amdkfd: Fix Unchecked Return Values
>
> Properly Check for return values from calls to debug functions in
> runtime_disable().
>
> Signed-off-by: Sunday Clement <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index 0f0719528bcc..8d096f52127c 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -2826,7 +2826,7 @@ static int runtime_enable(struct kfd_process *p,
> uint64_t r_debug,
>
>  static int runtime_disable(struct kfd_process *p)
>  {
> -     int i = 0, ret;
> +     int i = 0, ret = 0;
>       bool was_enabled = p->runtime_info.runtime_state ==
> DEBUG_RUNTIME_STATE_ENABLED;
>
>       p->runtime_info.runtime_state = DEBUG_RUNTIME_STATE_DISABLED;
> @@ -2872,14 +2872,14 @@ static int runtime_disable(struct kfd_process *p)
>                                       pdd->dev->vm_info.last_vmid_kfd);
>
>                       if (!pdd->dev->kfd->shared_resources.enable_mes)
> -                             debug_refresh_runlist(pdd->dev->dqm);
> +                             ret = debug_refresh_runlist(pdd->dev->dqm);
>                       else
> -                             kfd_dbg_set_mes_debug_mode(pdd,
> +                             ret = kfd_dbg_set_mes_debug_mode(pdd,

This doesn't really check anything because you'll return success if it's not 
the last device that fails to debug unset in the loop.
But you can't break out of the loop on error either because you need to rewind 
debug mode state as much as possible.

Maybe just store the last non-zero return somewhere then just return that value 
after the loop.

Jon

>
> !kfd_dbg_has_cwsr_workaround(pdd->dev));
>               }
>       }
>
> -     return 0;
> +     return ret;
>  }
>
>  static int kfd_ioctl_runtime_enable(struct file *filep, struct kfd_process 
> *p, void
> *data)
> --
> 2.43.0

Reply via email to