On 6/19/26 19:17, Srinivasan Shanmugam wrote:
> amdgpu_device_ip_soft_reset() allocates an array of AMDGPU_MAX_RINGS
> ring pointers on the stack. On 64-bit builds this consumes around 1280
> bytes and triggers:
> 
> warning: stack frame size (1304) exceeds limit (1024)
> 
> Move the temporary ring pointer array to heap allocation to reduce stack
> usage.

Clear NAK.

GFP_KERNEL allocations are forbidden in the reset path.

You could use GFP_NOWAIT or GFP_ATOMIC, but that should be avoided as well.

Why is that array necessary in the first place?

Regards,
Christian.

> 
> Fixes: a6319ac34a13 ("drm/amdgpu: Add IP block soft reset as a GPU recovery 
> method")
> Cc: Alex Deucher <[email protected]>
> Cc: Timur Kristóf <[email protected]>
> Signed-off-by: Srinivasan Shanmugam <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> index 65505bc50399..eeb9383b1010 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> @@ -524,7 +524,7 @@ int amdgpu_device_ip_soft_reset(struct amdgpu_ring 
> *guilty_ring,
>                               struct amdgpu_fence *guilty_fence)
>  {
>       struct amdgpu_device *adev = guilty_ring->adev;
> -     struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
> +     struct amdgpu_ring **rings;
>       struct amdgpu_ip_block *ip_block;
>       enum amd_ip_block_type ip_type;
>       u32 num_rings, ring_type_mask;
> @@ -539,6 +539,10 @@ int amdgpu_device_ip_soft_reset(struct amdgpu_ring 
> *guilty_ring,
>               return -EOPNOTSUPP;
>       }
>  
> +     rings = kcalloc(AMDGPU_MAX_RINGS, sizeof(*rings), GFP_KERNEL);
> +     if (!rings)
> +             return -ENOMEM;
> +
>       dev_err(adev->dev, "Starting %s IP block soft reset\n",
>               ip_block->version->funcs->name);
>  
> @@ -546,20 +550,25 @@ int amdgpu_device_ip_soft_reset(struct amdgpu_ring 
> *guilty_ring,
>       amdgpu_filter_rings(adev, ring_type_mask, rings, &num_rings);
>  
>       amdgpu_device_lock_reset_domain(adev->reset_domain);
> -     amdgpu_multi_ring_reset_helper_begin(rings, num_rings, guilty_ring, 
> guilty_fence);
> +     amdgpu_multi_ring_reset_helper_begin(rings, num_rings, guilty_ring,
> +                                          guilty_fence);
>  
>       r = ip_block->version->funcs->soft_reset(ip_block);
>  
> -     r = amdgpu_multi_ring_reset_helper_end(rings, num_rings, guilty_ring, 
> r);
> +     r = amdgpu_multi_ring_reset_helper_end(rings, num_rings, guilty_ring,
> +                                            r);
>       amdgpu_device_unlock_reset_domain(adev->reset_domain);
>  
>       if (r) {
>               dev_err(adev->dev, "Failed %s IP block soft reset: %d\n",
>                       ip_block->version->funcs->name, r);
> -             return r;
> +             goto out_free;
>       }
>  
>       dev_err(adev->dev, "Successful %s IP block soft reset\n",
>               ip_block->version->funcs->name);
> -     return 0;
> +
> +out_free:
> +     kfree(rings);
> +     return r;
>  }

Reply via email to