AMD General > -----Original Message----- > From: SHANMUGAM, SRINIVASAN > Sent: Monday, June 22, 2026 2:32 PM > To: Koenig, Christian <[email protected]>; Deucher, Alexander > <[email protected]> > Cc: [email protected]; Timur Kristóf <[email protected]> > Subject: RE: [PATCH] drm/amdgpu: Reduce stack usage in IP block soft reset > > > > > -----Original Message----- > > From: Koenig, Christian <[email protected]> > > Sent: Monday, June 22, 2026 1:45 PM > > To: SHANMUGAM, SRINIVASAN <[email protected]>; > Deucher, > > Alexander <[email protected]> > > Cc: [email protected]; Timur Kristóf > > <[email protected]> > > Subject: Re: [PATCH] drm/amdgpu: Reduce stack usage in IP block soft > > reset > > > > > > > > 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? > > > Thanks Christian. > > Looks like the temporary array is currently only used to collect the subset > of rings > affected by the IP block soft reset and pass them to > amdgpu_multi_ring_reset_helper_begin() and > amdgpu_multi_ring_reset_helper_end(). > > amdgpu_filter_rings() simply copies matching entries from > adev->rings[] into a temporary array, and that array is only > iterated over by the multi-ring reset helpers. > > Looking at the implementation again, it seems possible to eliminate the > intermediate > array entirely and have the helpers iterate directly over adev->rings[] while > filtering > based on the ring type mask. > > May I kno pls, would that approach make sense? > > Regards, > Srini >
I took another look at the implementation. The temporary array is currently only used to collect the subset of affected rings and pass them to the multi-ring reset helpers. amdgpu_filter_rings() simply copies matching entries from adev->rings[] into a temporary array, and the helpers only iterate over that array. From what I can see, adev->rings[] appears to be a stable device ring list, so it seems possible to eliminate the intermediate array entirely and have the helpers iterate directly over adev->rings[] while applying the same ring_type_mask filter. Hi Timur, was the temporary array intended to provide snapshot semantics for the affected rings, or was it mainly introduced for convenience? If there is no snapshot requirement, I'll respin the patch by removing the temporary array altogether. Regards, Srini
