linux-kernel - Re: [PATCH] gpu/drm/amd/amdkfd/kdf_queue.c: removal of kfd_queue_buffer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CADnq5_NzpnnKPxV2RFX=TVAw9JbRByQbDbCZBaNHhXVjK4fWSw@mail.gmail.com>
Date: Mon, 25 Aug 2025 13:23:19 -0400
From: Alex Deucher <alexdeucher@...il.com>
To: Alex Tran <alex.t.tran@...il.com>
Cc: Felix.Kuehling@....com, alexander.deucher@....com, 
	christian.koenig@....com, airlied@...il.com, simona@...ll.ch, 
	amd-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] gpu/drm/amd/amdkfd/kdf_queue.c: removal of kfd_queue_buffer_put

On Sat, Aug 23, 2025 at 6:11 AM Alex Tran <alex.t.tran@...il.com> wrote:
>
> Removed kfd_queue_buffer_put to call amdgpu_bo_unref directly.
>
> Signed-off-by: Alex Tran <alex.t.tran@...il.com>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h            |  1 -
>  .../drm/amd/amdkfd/kfd_process_queue_manager.c   |  2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_queue.c           | 16 +++++-----------
>  3 files changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 67694bcd9464..2bc0365b2ce9 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -1312,7 +1312,6 @@ void print_queue_properties(struct queue_properties *q);
>  void print_queue(struct queue *q);
>  int kfd_queue_buffer_get(struct amdgpu_vm *vm, void __user *addr, struct amdgpu_bo **pbo,
>                          u64 expected_size);
> -void kfd_queue_buffer_put(struct amdgpu_bo **bo);
>  int kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct queue_properties *properties);
>  int kfd_queue_release_buffers(struct kfd_process_device *pdd, struct queue_properties *properties);
>  void kfd_queue_unref_bo_va(struct amdgpu_vm *vm, struct amdgpu_bo **bo);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> index 7fbb5c274ccc..b13817e4a829 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> @@ -610,7 +610,7 @@ int pqm_update_queue_properties(struct process_queue_manager *pqm,
>                 }
>
>                 kfd_queue_unref_bo_va(vm, &pqn->q->properties.ring_bo);
> -               kfd_queue_buffer_put(&pqn->q->properties.ring_bo);
> +               amdgpu_bo_unref(&pqn->q->properties.ring_bo);
>                 amdgpu_bo_unreserve(vm->root.bo);
>
>                 pqn->q->properties.ring_bo = p->ring_bo;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_queue.c b/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
> index a65c67cf56ff..dd529e37c0e6 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
> @@ -224,12 +224,6 @@ int kfd_queue_buffer_get(struct amdgpu_vm *vm, void __user *addr, struct amdgpu_
>         return -EINVAL;
>  }
>
> -/* FIXME: remove this function, just call amdgpu_bo_unref directly */

Despite what this comment says, I think it makes sense to keep this
around so that we have a matching set of put and get functions so it's
clear why we are putting the reference.  That said if Felix and Harish
are ok with it, that's fine with me.

Alex

> -void kfd_queue_buffer_put(struct amdgpu_bo **bo)
> -{
> -       amdgpu_bo_unref(bo);
> -}
> -
>  int kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct queue_properties *properties)
>  {
>         struct kfd_topology_device *topo_dev;
> @@ -343,11 +337,11 @@ int kfd_queue_release_buffers(struct kfd_process_device *pdd, struct queue_prope
>         struct kfd_topology_device *topo_dev;
>         u32 total_cwsr_size;
>
> -       kfd_queue_buffer_put(&properties->wptr_bo);
> -       kfd_queue_buffer_put(&properties->rptr_bo);
> -       kfd_queue_buffer_put(&properties->ring_bo);
> -       kfd_queue_buffer_put(&properties->eop_buf_bo);
> -       kfd_queue_buffer_put(&properties->cwsr_bo);
> +       amdgpu_bo_unref(&properties->wptr_bo);
> +       amdgpu_bo_unref(&properties->rptr_bo);
> +       amdgpu_bo_unref(&properties->ring_bo);
> +       amdgpu_bo_unref(&properties->eop_buf_bo);
> +       amdgpu_bo_unref(&properties->cwsr_bo);
>
>         topo_dev = kfd_topology_device_by_id(pdd->dev->id);
>         if (!topo_dev)
> --
> 2.50.1
>