[PATCH v2] drm/amdgpu: initialize ret before UMC error record loop

[PATCH v2] drm/amdgpu: initialize ret before UMC error record loop

[Ruoyu Wang]

umc_v12_0_fill_error_record() returns ret after walking the pages
reported by amdgpu_umc_lookup_bad_pages_in_a_row(). That helper can
return zero entries, leaving the loop skipped and ret uninitialized.

Set ret to 0 immediately before the loop so the zero-page path reports a
deterministic status instead of returning stack data.

Suggested-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
v2:
- Set ret immediately before the loop instead of initializing it in the
  declaration, as suggested by Christian.
- Avoid describing the zero-entry case as an allocation-failure fix.

 drivers/gpu/drm/amd/amdgpu/umc_v12_0.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c b/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
index 14092150336a5..4842340d9d65f 100644
--- a/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
@@ -690,6 +690,7 @@ static int umc_v12_0_fill_error_record(struct amdgpu_device *adev,
 	count = amdgpu_umc_lookup_bad_pages_in_a_row(adev,
 				ecc_err->pa_pfn << AMDGPU_GPU_PAGE_SHIFT,
 				page_pfn, ARRAY_SIZE(page_pfn));
+	ret = 0;
 
 	for (i = 0; i < count; i++) {
 		ret = amdgpu_umc_fill_error_record(err_data,
-- 
2.51.0

Re: [PATCH v2] drm/amdgpu: initialize ret before UMC error record loop

[Christian König]

On 6/17/26 21:33, Ruoyu Wang wrote:
> umc_v12_0_fill_error_record() returns ret after walking the pages
> reported by amdgpu_umc_lookup_bad_pages_in_a_row(). That helper can
> return zero entries, leaving the loop skipped and ret uninitialized.
> 
> Set ret to 0 immediately before the loop so the zero-page path reports a
> deterministic status instead of returning stack data.
> 
> Suggested-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>

Acked-by: Christian König <christian.koenig@amd.com>

Someone more familiar with ras should take a look as well, could be that returning -ENOENT is more appropriate here.

> ---
> v2:
> - Set ret immediately before the loop instead of initializing it in the
>   declaration, as suggested by Christian.
> - Avoid describing the zero-entry case as an allocation-failure fix.
> 
>  drivers/gpu/drm/amd/amdgpu/umc_v12_0.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c b/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
> index 14092150336a5..4842340d9d65f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
> @@ -690,6 +690,7 @@ static int umc_v12_0_fill_error_record(struct amdgpu_device *adev,
>         count = amdgpu_umc_lookup_bad_pages_in_a_row(adev,
>                                 ecc_err->pa_pfn << AMDGPU_GPU_PAGE_SHIFT,
>                                 page_pfn, ARRAY_SIZE(page_pfn));
> +       ret = 0;
> 
>         for (i = 0; i < count; i++) {
>                 ret = amdgpu_umc_fill_error_record(err_data,
> --
> 2.51.0

RE: [PATCH v2] drm/amdgpu: initialize ret before UMC error record loop

[Zhou1, Tao]

AMD General

[Tao] In order to align with the original logic, I prefer to "ret = -EINVAL"

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Christian
> König
> Sent: Thursday, June 18, 2026 3:37 PM
> To: Ruoyu Wang <ruoyuw560@gmail.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; David Airlie <airlied@gmail.com>; Simona Vetter
> <simona@ffwll.ch>; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> linux-kernel@vger.kernel.org; Zhang, Hawking <Hawking.Zhang@amd.com>; Lazar,
> Lijo <Lijo.Lazar@amd.com>
> Subject: Re: [PATCH v2] drm/amdgpu: initialize ret before UMC error record loop
>
> On 6/17/26 21:33, Ruoyu Wang wrote:
> > umc_v12_0_fill_error_record() returns ret after walking the pages
> > reported by amdgpu_umc_lookup_bad_pages_in_a_row(). That helper can
> > return zero entries, leaving the loop skipped and ret uninitialized.
> >
> > Set ret to 0 immediately before the loop so the zero-page path reports
> > a deterministic status instead of returning stack data.
> >
> > Suggested-by: Christian König <christian.koenig@amd.com>
> > Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
>
> Acked-by: Christian König <christian.koenig@amd.com>
>
> Someone more familiar with ras should take a look as well, could be that returning -
> ENOENT is more appropriate here.
>
> > ---
> > v2:
> > - Set ret immediately before the loop instead of initializing it in the
> >   declaration, as suggested by Christian.
> > - Avoid describing the zero-entry case as an allocation-failure fix.
> >
> >  drivers/gpu/drm/amd/amdgpu/umc_v12_0.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
> > b/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
> > index 14092150336a5..4842340d9d65f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
> > @@ -690,6 +690,7 @@ static int umc_v12_0_fill_error_record(struct
> amdgpu_device *adev,
> >         count = amdgpu_umc_lookup_bad_pages_in_a_row(adev,
> >                                 ecc_err->pa_pfn << AMDGPU_GPU_PAGE_SHIFT,
> >                                 page_pfn, ARRAY_SIZE(page_pfn));
> > +       ret = 0;
> >
> >         for (i = 0; i < count; i++) {
> >                 ret = amdgpu_umc_fill_error_record(err_data,
> > --
> > 2.51.0