Re: [PATCH] drm/amdgpu: Allocate coredump ring buffers per ring

On Wed, Jul 29, 2026 at 7:43 AM Lijo Lazar <[email protected]> wrote:
>
> Allocate each ring buffer separately. A single allocation summing all
> ring sizes can exceed the page allocator's MAX_ORDER limit and fail;
> per-ring buffers stay small enough to satisfy. The existing allocation
> style doesn't capture any ring data if the huge allocation fails.
> Splitting into multiple allocations helps to capture as much data as
> possible for the core dump.
>
> A failed ring is left with a NULL buffer and skipped when formatting.
>
> Fixes: eea85914d15b ("drm/amdgpu: save ring content before resetting the device")
> Signed-off-by: Lijo Lazar <[email protected]>
> Assisted-by: Claude Code

Reviewed-by: Alex Deucher <[email protected]>

> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c  | 50 ++++++++++---------
>  .../gpu/drm/amd/amdgpu/amdgpu_dev_coredump.h  |  3 +-
>  2 files changed, 27 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c
> index 4dfea36997d4..87e15e39eb30 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c
> @@ -343,7 +343,7 @@ amdgpu_devcoredump_format(char *buffer, size_t count, struct amdgpu_coredump_inf
>         struct amdgpu_ip_block *ip_block;
>         struct amdgpu_ring *ring;
>         int ver, i, j;
> -       u32 ring_idx, off;
> +       u32 ring_idx;
>         bool sizing_pass;
>
>         sizing_pass = buffer == NULL;
> @@ -443,7 +443,6 @@ amdgpu_devcoredump_format(char *buffer, size_t count, struct amdgpu_coredump_inf
>                 for (i = 0; i < coredump->num_rings; i++) {
>                         ring_idx = coredump->rings[i].ring_index;
>                         ring = coredump->adev->rings[ring_idx];
> -                       off = coredump->rings[i].offset;
>
>                         drm_printf(&p, "ring name: %s\n", ring->name);
>                         drm_printf(&p, "Rptr: 0x%llx Wptr: 0x%llx RB mask: %x\n",
> @@ -452,12 +451,18 @@ amdgpu_devcoredump_format(char *buffer, size_t count, struct amdgpu_coredump_inf
>                                    ring->buf_mask);
>                         drm_printf(&p, "Ring size in dwords: %d\n",
>                                 ring->ring_size / 4);
> +
> +                       if (!coredump->rings[i].ring_dw) {
> +                               drm_printf(&p, "Ring contents unavailable\n");
> +                               continue;
> +                       }
> +
>                         drm_printf(&p, "Ring contents\n");
>                         drm_printf(&p, "Offset \t Value\n");
>
>                         for (j = 0; j < ring->ring_size; j += 4)
>                                 drm_printf(&p, "0x%x \t 0x%x\n", j,
> -                                          coredump->rings_dw[off + j / 4]);
> +                                          coredump->rings[i].ring_dw[j / 4]);
>                 }
>         }
>
> @@ -498,10 +503,12 @@ amdgpu_devcoredump_read(char *buffer, loff_t offset, size_t count,
>  static void amdgpu_devcoredump_free(void *data)
>  {
>         struct amdgpu_coredump_info *coredump = data;
> +       u32 i;
>
>         kvfree(coredump->formatted);
> +       for (i = 0; i < coredump->num_rings; i++)
> +               kvfree(coredump->rings[i].ring_dw);
>         kvfree(coredump->rings);
> -       kvfree(coredump->rings_dw);
>         kvfree(data);
>  }
>
> @@ -543,9 +550,9 @@ void amdgpu_coredump(struct amdgpu_device *adev, bool skip_vram_check,
>         struct amdgpu_coredump_info *coredump;
>         size_t size = sizeof(*coredump);
>         struct drm_sched_job *s_job;
> -       u64 total_ring_size, ring_count;
> +       u64 ring_count;
>         struct amdgpu_ring *ring;
> -       int i, off, idx;
> +       int i, idx;
>
>         /* No need to generate a new coredump if there's one in progress already. */
>         if (work_busy(&adev->coredump_work))
> @@ -585,7 +592,6 @@ void amdgpu_coredump(struct amdgpu_device *adev, bool skip_vram_check,
>
>         /* Dump ring content if memory allocation succeeds. */
>         ring_count = 0;
> -       total_ring_size = 0;
>         for (i = 0; i < adev->num_rings; i++) {
>                 ring = adev->rings[i];
>
> @@ -594,38 +600,34 @@ void amdgpu_coredump(struct amdgpu_device *adev, bool skip_vram_check,
>                     coredump->ring != ring)
>                         continue;
>
> -               total_ring_size += ring->ring_size;
>                 ring_count++;
>         }
> -       if (ring_count) {
> -               coredump->rings_dw = kvzalloc(total_ring_size, GFP_NOWAIT);
> +       if (ring_count)
>                 coredump->rings = kvcalloc(ring_count,
>                                            sizeof(struct amdgpu_coredump_ring),
>                                            GFP_NOWAIT);
> -       }
> -       if (coredump->rings && coredump->rings_dw) {
> -               for (i = 0, off = 0, idx = 0; i < adev->num_rings && idx < ring_count; i++) {
> +       if (coredump->rings) {
> +               for (i = 0, idx = 0; i < adev->num_rings && idx < ring_count; i++) {
> +                       struct amdgpu_coredump_ring *cdump_ring;
> +
>                         ring = adev->rings[i];
>
>                         if (atomic_read(&ring->fence_drv.last_seq) == ring->fence_drv.sync_seq &&
>                             coredump->ring != ring)
>                                 continue;
>
> -                       coredump->rings[idx].ring_index = ring->idx;
> -                       coredump->rings[idx].rptr = amdgpu_ring_get_rptr(ring);
> -                       coredump->rings[idx].wptr = amdgpu_ring_get_wptr(ring);
> -                       coredump->rings[idx].offset = off;
> +                       cdump_ring = &coredump->rings[idx];
> +
> +                       cdump_ring->ring_dw = kvzalloc(ring->ring_size, GFP_NOWAIT);
> +                       if (cdump_ring->ring_dw)
> +                               memcpy(cdump_ring->ring_dw, ring->ring, ring->ring_size);
>
> -                       memcpy(&coredump->rings_dw[off], ring->ring, ring->ring_size);
> -                       off += ring->ring_size / 4;
> +                       cdump_ring->ring_index = ring->idx;
> +                       cdump_ring->rptr = amdgpu_ring_get_rptr(ring);
> +                       cdump_ring->wptr = amdgpu_ring_get_wptr(ring);
>                         idx++;
>                 }
>                 coredump->num_rings = idx;
> -       } else {
> -               kvfree(coredump->rings_dw);
> -               kvfree(coredump->rings);
> -               coredump->rings_dw = NULL;
> -               coredump->rings = NULL;
>         }
>
>         coredump->adev = adev;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.h
> index 63f27337c09a..cb874d8c3394 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.h
> @@ -34,8 +34,8 @@
>  struct amdgpu_coredump_ring {
>         u64                             rptr;
>         u64                             wptr;
> +       u32                             *ring_dw;
>         u32                             ring_index;
> -       u32                             offset;
>  };
>
>  struct amdgpu_coredump_ib_info {
> @@ -53,7 +53,6 @@ struct amdgpu_coredump_info {
>         struct amdgpu_ring              *ring;
>
>         struct amdgpu_coredump_ring     *rings;
> -       u32                             *rings_dw;
>         u32                             num_rings;
>
>         /* Readable form of coredevdump, generate once to speed up
> --
> 2.49.0
>