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;
> }