[Public] > -----Original Message----- > From: amd-gfx <[email protected]> On Behalf Of > Srinivasan Shanmugam > Sent: Sunday, July 9, 2023 11:05 PM > To: Koenig, Christian <[email protected]>; Deucher, Alexander > <[email protected]>; Kuehling, Felix <[email protected]> > Cc: SHANMUGAM, SRINIVASAN <[email protected]>; > [email protected] > Subject: [PATCH v2] drm/amdkfd: Fix stack size in > 'amdgpu_amdkfd_unmap_hiq' > > Allocate large local variable on heap to avoid exceeding the stack size: > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c: In function > ‘amdgpu_amdkfd_unmap_hiq’: > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c:868:1: warning: the > frame size of 1280 bytes is larger than 1024 bytes [-Wframe-larger-than=] > > Cc: Felix Kuehling <[email protected]> > Cc: Christian König <[email protected]> > Cc: Alex Deucher <[email protected]> > Signed-off-by: Srinivasan Shanmugam <[email protected]> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 34 +++++++++++++++--- > ---- > 1 file changed, 23 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > index 0040c63e2356..8c9a48062abf 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > @@ -836,33 +836,45 @@ int amdgpu_amdkfd_unmap_hiq(struct > amdgpu_device *adev, u32 doorbell_off, { > struct amdgpu_kiq *kiq = &adev->gfx.kiq[inst]; > struct amdgpu_ring *kiq_ring = &kiq->ring; > - struct amdgpu_ring_funcs ring_funcs; > - struct amdgpu_ring ring; > + struct amdgpu_ring_funcs *ring_funcs; > + struct amdgpu_ring *ring; > int r = 0; > > - if (!kiq->pmf || !kiq->pmf->kiq_unmap_queues) > - return -EINVAL; > + ring_funcs = kzalloc(sizeof(*ring_funcs), GFP_KERNEL); > + if (!ring_funcs) > + return -ENOMEM; > > - memset(&ring, 0x0, sizeof(struct amdgpu_ring)); > - memset(&ring_funcs, 0x0, sizeof(struct amdgpu_ring_funcs)); > + ring = kzalloc(sizeof(*ring), GFP_KERNEL); > + if (!ring) > + return -ENOMEM;
Should it free ring_funcs before 'return -ENOMEM'?
> - ring_funcs.type = AMDGPU_RING_TYPE_COMPUTE;
> - ring.doorbell_index = doorbell_off;
> - ring.funcs = &ring_funcs;
> + if (!kiq->pmf || !kiq->pmf->kiq_unmap_queues) {
> + r = -EINVAL;
> + goto free_ring;
> + }
It's suggested to keep the check of ' kiq->pmf' and
'kiq->pmf->kiq_unmap_queues' ahead of allocation of ring & ring_funcs.
Regards,
Guchun
> + ring_funcs->type = AMDGPU_RING_TYPE_COMPUTE;
> + ring->doorbell_index = doorbell_off;
> + ring->funcs = ring_funcs;
>
> spin_lock(&kiq->ring_lock);
>
> if (amdgpu_ring_alloc(kiq_ring, kiq->pmf->unmap_queues_size)) {
> spin_unlock(&kiq->ring_lock);
> - return -ENOMEM;
> + r = -ENOMEM;
> + goto free_ring;
> }
>
> - kiq->pmf->kiq_unmap_queues(kiq_ring, &ring, RESET_QUEUES, 0, 0);
> + kiq->pmf->kiq_unmap_queues(kiq_ring, ring, RESET_QUEUES, 0, 0);
>
> if (kiq_ring->sched.ready && !adev->job_hang)
> r = amdgpu_ring_test_helper(kiq_ring);
>
> spin_unlock(&kiq->ring_lock);
>
> +free_ring:
> + kfree(ring_funcs);
> + kfree(ring);
> +
> return r;
> }
> --
> 2.25.1
<<attachment: winmail.dat>>
