[PATCH v1 5/7] drm/amdgpu: port some debugfs function to drm_printer
Christian König
christian.koenig at amd.com
Wed May 21 11:48:05 UTC 2025
On 5/21/25 11:49, Pierre-Eric Pelloux-Prayer wrote:
> Using the drm_printer interface will allow us to use these functions
> when generating the coredump.
This change in general is harmless, but you must be extremely careful to not grab locks in the core dump which somebody could hold while waiting for a reset to complete.
For example any BO lock is an absolute no-go.
Regards,
Christian.
>
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 5 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 5 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 38 +++++++++----------
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 5 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 42 ++++++++++-----------
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 5 +--
> 6 files changed, 52 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index 8e626f50b362..15ccb0eeb78e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -1773,12 +1773,15 @@ static int amdgpu_debugfs_vm_info_show(struct seq_file *m, void *unused)
> struct amdgpu_device *adev = m->private;
> struct drm_device *dev = adev_to_drm(adev);
> struct drm_file *file;
> + struct drm_printer p;
> int r;
>
> r = mutex_lock_interruptible(&dev->filelist_mutex);
> if (r)
> return r;
>
> + p = drm_seq_file_printer(m);
> +
> list_for_each_entry(file, &dev->filelist, lhead) {
> struct amdgpu_fpriv *fpriv = file->driver_priv;
> struct amdgpu_vm *vm = &fpriv->vm;
> @@ -1793,7 +1796,7 @@ static int amdgpu_debugfs_vm_info_show(struct seq_file *m, void *unused)
> r = amdgpu_bo_reserve(vm->root.bo, true);
> if (r)
> break;
> - amdgpu_debugfs_vm_bo_info(vm, m);
> + amdgpu_debugfs_vm_bo_info(vm, &p);
> amdgpu_bo_unreserve(vm->root.bo);
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 2c68118fe9fd..35457f44a089 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -1141,12 +1141,15 @@ static int amdgpu_debugfs_gem_info_show(struct seq_file *m, void *unused)
> struct amdgpu_device *adev = m->private;
> struct drm_device *dev = adev_to_drm(adev);
> struct drm_file *file;
> + struct drm_printer p;
> int r;
>
> r = mutex_lock_interruptible(&dev->filelist_mutex);
> if (r)
> return r;
>
> + p = drm_seq_file_printer(m);
> +
> list_for_each_entry(file, &dev->filelist, lhead) {
> struct task_struct *task;
> struct drm_gem_object *gobj;
> @@ -1170,7 +1173,7 @@ static int amdgpu_debugfs_gem_info_show(struct seq_file *m, void *unused)
> idr_for_each_entry(&file->object_idr, gobj, id) {
> struct amdgpu_bo *bo = gem_to_amdgpu_bo(gobj);
>
> - amdgpu_bo_print_info(id, bo, m);
> + amdgpu_bo_print_info(id, bo, &p);
> }
> spin_unlock(&file->table_lock);
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 3339703f40b7..568c2cd95703 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -1553,27 +1553,26 @@ uint32_t amdgpu_bo_get_preferred_domain(struct amdgpu_device *adev,
> return domain;
> }
>
> -#if defined(CONFIG_DEBUG_FS)
> -#define amdgpu_bo_print_flag(m, bo, flag) \
> +#define amdgpu_bo_print_flag(p, bo, flag) \
> do { \
> if (bo->flags & (AMDGPU_GEM_CREATE_ ## flag)) { \
> - seq_printf((m), " " #flag); \
> + drm_printf((p), " " #flag); \
> } \
> } while (0)
>
> /**
> - * amdgpu_bo_print_info - print BO info in debugfs file
> + * amdgpu_bo_print_info - print BO info in printer
> *
> * @id: Index or Id of the BO
> * @bo: Requested BO for printing info
> - * @m: debugfs file
> + * @p: drm_printer to use
> *
> * Print BO information in debugfs file
> *
> * Returns:
> * Size of the BO in bytes.
> */
> -u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct seq_file *m)
> +u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct drm_printer *p)
> {
> struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> struct dma_buf_attachment *attachment;
> @@ -1623,31 +1622,30 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct seq_file *m)
> }
>
> size = amdgpu_bo_size(bo);
> - seq_printf(m, "\t\t0x%08x: %12lld byte %s",
> - id, size, placement);
> + drm_printf(p, "\t\t0x%08x: %12lld byte %s",
> + id, size, placement);
>
> pin_count = READ_ONCE(bo->tbo.pin_count);
> if (pin_count)
> - seq_printf(m, " pin count %d", pin_count);
> + drm_printf(p, " pin count %d", pin_count);
>
> dma_buf = READ_ONCE(bo->tbo.base.dma_buf);
> attachment = READ_ONCE(bo->tbo.base.import_attach);
>
> if (attachment)
> - seq_printf(m, " imported from ino:%lu", file_inode(dma_buf->file)->i_ino);
> + drm_printf(p, " imported from ino:%lu", file_inode(dma_buf->file)->i_ino);
> else if (dma_buf)
> - seq_printf(m, " exported as ino:%lu", file_inode(dma_buf->file)->i_ino);
> + drm_printf(p, " exported as ino:%lu", file_inode(dma_buf->file)->i_ino);
>
> - amdgpu_bo_print_flag(m, bo, CPU_ACCESS_REQUIRED);
> - amdgpu_bo_print_flag(m, bo, NO_CPU_ACCESS);
> - amdgpu_bo_print_flag(m, bo, CPU_GTT_USWC);
> - amdgpu_bo_print_flag(m, bo, VRAM_CLEARED);
> - amdgpu_bo_print_flag(m, bo, VRAM_CONTIGUOUS);
> - amdgpu_bo_print_flag(m, bo, VM_ALWAYS_VALID);
> - amdgpu_bo_print_flag(m, bo, EXPLICIT_SYNC);
> + amdgpu_bo_print_flag(p, bo, CPU_ACCESS_REQUIRED);
> + amdgpu_bo_print_flag(p, bo, NO_CPU_ACCESS);
> + amdgpu_bo_print_flag(p, bo, CPU_GTT_USWC);
> + amdgpu_bo_print_flag(p, bo, VRAM_CLEARED);
> + amdgpu_bo_print_flag(p, bo, VRAM_CONTIGUOUS);
> + amdgpu_bo_print_flag(p, bo, VM_ALWAYS_VALID);
> + amdgpu_bo_print_flag(p, bo, EXPLICIT_SYNC);
>
> - seq_puts(m, "\n");
> + drm_puts(p, "\n");
>
> return size;
> }
> -#endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 375448627f7b..f411a8c3199c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -139,6 +139,8 @@ struct amdgpu_bo_vm {
> struct amdgpu_vm_bo_base entries[];
> };
>
> +struct drm_printer;
> +
> static inline struct amdgpu_bo *ttm_to_amdgpu_bo(struct ttm_buffer_object *tbo)
> {
> return container_of(tbo, struct amdgpu_bo, tbo);
> @@ -345,8 +347,9 @@ void amdgpu_sa_bo_free(struct drm_suballoc **sa_bo,
> #if defined(CONFIG_DEBUG_FS)
> void amdgpu_sa_bo_dump_debug_info(struct amdgpu_sa_manager *sa_manager,
> struct seq_file *m);
> -u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct seq_file *m);
> #endif
> +u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct drm_printer *p);
> +
> void amdgpu_debugfs_sa_init(struct amdgpu_device *adev);
>
> bool amdgpu_bo_support_uswc(u64 bo_flags);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 3911c78f8282..327ce9e883cc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2990,16 +2990,15 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
> return false;
> }
>
> -#if defined(CONFIG_DEBUG_FS)
> /**
> * amdgpu_debugfs_vm_bo_info - print BO info for the VM
> *
> * @vm: Requested VM for printing BO info
> - * @m: debugfs file
> + * @p: the drm_printer to use
> *
> * Print BO information in debugfs file for the VM
> */
> -void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, struct seq_file *m)
> +void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, struct drm_printer *p)
> {
> struct amdgpu_bo_va *bo_va, *tmp;
> u64 total_idle = 0;
> @@ -3017,74 +3016,73 @@ void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, struct seq_file *m)
> unsigned int id = 0;
>
> spin_lock(&vm->status_lock);
> - seq_puts(m, "\tIdle BOs:\n");
> + drm_puts(p, "\tIdle BOs:\n");
> list_for_each_entry_safe(bo_va, tmp, &vm->idle, base.vm_status) {
> if (!bo_va->base.bo)
> continue;
> - total_idle += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
> + total_idle += amdgpu_bo_print_info(id++, bo_va->base.bo, p);
> }
> total_idle_objs = id;
> id = 0;
>
> - seq_puts(m, "\tEvicted BOs:\n");
> + drm_puts(p, "\tEvicted BOs:\n");
> list_for_each_entry_safe(bo_va, tmp, &vm->evicted, base.vm_status) {
> if (!bo_va->base.bo)
> continue;
> - total_evicted += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
> + total_evicted += amdgpu_bo_print_info(id++, bo_va->base.bo, p);
> }
> total_evicted_objs = id;
> id = 0;
>
> - seq_puts(m, "\tRelocated BOs:\n");
> + drm_puts(p, "\tRelocated BOs:\n");
> list_for_each_entry_safe(bo_va, tmp, &vm->relocated, base.vm_status) {
> if (!bo_va->base.bo)
> continue;
> - total_relocated += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
> + total_relocated += amdgpu_bo_print_info(id++, bo_va->base.bo, p);
> }
> total_relocated_objs = id;
> id = 0;
>
> - seq_puts(m, "\tMoved BOs:\n");
> + drm_puts(p, "\tMoved BOs:\n");
> list_for_each_entry_safe(bo_va, tmp, &vm->moved, base.vm_status) {
> if (!bo_va->base.bo)
> continue;
> - total_moved += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
> + total_moved += amdgpu_bo_print_info(id++, bo_va->base.bo, p);
> }
> total_moved_objs = id;
> id = 0;
>
> - seq_puts(m, "\tInvalidated BOs:\n");
> + drm_puts(p, "\tInvalidated BOs:\n");
> list_for_each_entry_safe(bo_va, tmp, &vm->invalidated, base.vm_status) {
> if (!bo_va->base.bo)
> continue;
> - total_invalidated += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
> + total_invalidated += amdgpu_bo_print_info(id++, bo_va->base.bo, p);
> }
> total_invalidated_objs = id;
> id = 0;
>
> - seq_puts(m, "\tDone BOs:\n");
> + drm_puts(p, "\tDone BOs:\n");
> list_for_each_entry_safe(bo_va, tmp, &vm->done, base.vm_status) {
> if (!bo_va->base.bo)
> continue;
> - total_done += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
> + total_done += amdgpu_bo_print_info(id++, bo_va->base.bo, p);
> }
> spin_unlock(&vm->status_lock);
> total_done_objs = id;
>
> - seq_printf(m, "\tTotal idle size: %12lld\tobjs:\t%d\n", total_idle,
> + drm_printf(p, "\tTotal idle size: %12lld\tobjs:\t%d\n", total_idle,
> total_idle_objs);
> - seq_printf(m, "\tTotal evicted size: %12lld\tobjs:\t%d\n", total_evicted,
> + drm_printf(p, "\tTotal evicted size: %12lld\tobjs:\t%d\n", total_evicted,
> total_evicted_objs);
> - seq_printf(m, "\tTotal relocated size: %12lld\tobjs:\t%d\n", total_relocated,
> + drm_printf(p, "\tTotal relocated size: %12lld\tobjs:\t%d\n", total_relocated,
> total_relocated_objs);
> - seq_printf(m, "\tTotal moved size: %12lld\tobjs:\t%d\n", total_moved,
> + drm_printf(p, "\tTotal moved size: %12lld\tobjs:\t%d\n", total_moved,
> total_moved_objs);
> - seq_printf(m, "\tTotal invalidated size: %12lld\tobjs:\t%d\n", total_invalidated,
> + drm_printf(p, "\tTotal invalidated size: %12lld\tobjs:\t%d\n", total_invalidated,
> total_invalidated_objs);
> - seq_printf(m, "\tTotal done size: %12lld\tobjs:\t%d\n", total_done,
> + drm_printf(p, "\tTotal done size: %12lld\tobjs:\t%d\n", total_done,
> total_done_objs);
> }
> -#endif
>
> /**
> * amdgpu_vm_update_fault_cache - update cached fault into.
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index f3ad687125ad..0bf9ea018eb7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -43,6 +43,7 @@ struct amdgpu_bo_va;
> struct amdgpu_job;
> struct amdgpu_bo_list_entry;
> struct amdgpu_bo_vm;
> +struct drm_printer;
>
> /*
> * GPUVM handling
> @@ -598,9 +599,7 @@ void amdgpu_vm_pt_free_work(struct work_struct *work);
> void amdgpu_vm_pt_free_list(struct amdgpu_device *adev,
> struct amdgpu_vm_update_params *params);
>
> -#if defined(CONFIG_DEBUG_FS)
> -void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, struct seq_file *m);
> -#endif
> +void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, struct drm_printer *p);
>
> int amdgpu_vm_pt_map_tables(struct amdgpu_device *adev, struct amdgpu_vm *vm);
>
More information about the amd-gfx
mailing list