[PATCH v4 6/6] drm/amdgpu: use drm_file::name in task_info::process_desc
Tvrtko Ursulin
tvrtko.ursulin at igalia.com
Mon Sep 30 09:24:49 UTC 2024
On 27/09/2024 09:48, Pierre-Eric Pelloux-Prayer wrote:
> If a drm_file name is set append it to the process name.
>
> This information is useful with the virtio/native-context driver: this
> allows the guest applications identifier to visible in amdgpu's output.
>
> The output in amdgpu_vm_info/amdgpu_gem_info looks like this:
> pid:12255 Process:glxgears/test-set-fd-name ----------
>
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 1 +
> .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 3 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 26 +++++++++++++++----
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 2 +-
> drivers/gpu/drm/amd/amdkfd/kfd_process.c | 3 +++
> 6 files changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index f9d119448442..ad909173e419 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -299,6 +299,7 @@ int amdgpu_amdkfd_gpuvm_set_vm_pasid(struct amdgpu_device *adev,
> struct amdgpu_vm *avm, u32 pasid);
> int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev,
> struct amdgpu_vm *avm,
> + struct drm_file *filp,
> void **process_info,
> struct dma_fence **ef);
> void amdgpu_amdkfd_gpuvm_release_process_vm(struct amdgpu_device *adev,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 6d5fd371d5ce..172882af6705 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1558,6 +1558,7 @@ int amdgpu_amdkfd_gpuvm_set_vm_pasid(struct amdgpu_device *adev,
>
> int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev,
> struct amdgpu_vm *avm,
> + struct drm_file *filp,
> void **process_info,
> struct dma_fence **ef)
> {
> @@ -1577,7 +1578,7 @@ int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev,
> if (ret)
> return ret;
>
> - amdgpu_vm_set_task_info(avm);
> + amdgpu_vm_set_task_info(avm, filp);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 891128ecee6d..5d43e24906d2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -1178,7 +1178,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
> }
>
> /* Use this opportunity to fill in task info for the vm */
> - amdgpu_vm_set_task_info(vm);
> + amdgpu_vm_set_task_info(vm, p->filp);
>
> if (adev->debug_vm) {
> /* Invalidate all BOs to test for userspace bugs */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index cec0a5cffcc8..f6e2be6d4e9e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2355,25 +2355,40 @@ amdgpu_vm_get_task_info_pasid(struct amdgpu_device *adev, u32 pasid)
> amdgpu_vm_get_vm_from_pasid(adev, pasid));
> }
>
> -static int amdgpu_vm_create_task_info(struct amdgpu_vm *vm)
> +static int amdgpu_vm_create_task_info(struct amdgpu_vm *vm, struct drm_file *filp)
> {
> char process_name[TASK_COMM_LEN];
> - int desc_len;
> + size_t desc_len;
Nit - would be nicer to avoid the churn from patch to patch by starting
with the correct type in the previous patch.
>
> get_task_comm(process_name, current->group_leader);
> desc_len = strlen(process_name);
>
> + mutex_lock(&filp->client_name_lock);
> + if (filp->client_name)
> + desc_len += 1 + strlen(filp->client_name);
> +
> vm->task_info = kzalloc(
> struct_size(vm->task_info, process_desc, desc_len + 1),
> GFP_KERNEL);
>
> - if (!vm->task_info)
> + if (!vm->task_info) {
> + mutex_unlock(&filp->client_name_lock);
> return -ENOMEM;
> + }
>
> /* Set process attributes now. */
> vm->task_info->tgid = current->group_leader->pid;
> strscpy(vm->task_info->process_desc, process_name, desc_len + 1);
>
> + if (filp->client_name) {
> + size_t p_len = strlen(process_name);
Another nit is that you are taking this strlen twice. Maybe cache it in
a top level local so it looks cleaner.
But those are just nits to make the series look more polished.
Fundamentals look fine to me so up to you if you want to respin or not.
Regards,
Tvrtko
> +
> + vm->task_info->process_desc[p_len] = '/';
> + strscpy(&vm->task_info->process_desc[p_len + 1],
> + filp->client_name, (desc_len + 1) - (p_len + 1));
> + }
> + mutex_unlock(&filp->client_name_lock);
> +
> kref_init(&vm->task_info->refcount);
> return 0;
> }
> @@ -2382,11 +2397,12 @@ static int amdgpu_vm_create_task_info(struct amdgpu_vm *vm)
> * amdgpu_vm_set_task_info - Sets VMs task info.
> *
> * @vm: vm for which to set the info
> + * @filp: drm_file instance
> */
> -void amdgpu_vm_set_task_info(struct amdgpu_vm *vm)
> +void amdgpu_vm_set_task_info(struct amdgpu_vm *vm, struct drm_file *filp)
> {
> if (!vm->task_info) {
> - if (amdgpu_vm_create_task_info(vm))
> + if (amdgpu_vm_create_task_info(vm, filp))
> return;
> } else if (vm->task_info->pid == current->pid) {
> return;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 44da250217be..8df3dece54c2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -561,7 +561,7 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
> u32 vmid, u32 node_id, uint64_t addr, uint64_t ts,
> bool write_fault);
>
> -void amdgpu_vm_set_task_info(struct amdgpu_vm *vm);
> +void amdgpu_vm_set_task_info(struct amdgpu_vm *vm, struct drm_file *filp);
>
> void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
> struct amdgpu_vm *vm);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index a902950cc060..e473fe433d3f 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -1654,6 +1654,7 @@ int kfd_process_device_init_vm(struct kfd_process_device *pdd,
> struct file *drm_file)
> {
> struct amdgpu_fpriv *drv_priv;
> + struct drm_file *filp;
> struct amdgpu_vm *avm;
> struct kfd_process *p;
> struct dma_fence *ef;
> @@ -1673,8 +1674,10 @@ int kfd_process_device_init_vm(struct kfd_process_device *pdd,
>
> p = pdd->process;
> dev = pdd->dev;
> + filp = drm_file->private_data;
>
> ret = amdgpu_amdkfd_gpuvm_acquire_process_vm(dev->adev, avm,
> + filp,
> &p->kgd_process_info,
> &ef);
> if (ret) {
More information about the dri-devel
mailing list