[PATCH v3 4/6] drm/amdgpu: alloc and init vm::task_info from first submit
Tvrtko Ursulin
tvrtko.ursulin at igalia.com
Mon Sep 23 10:58:15 UTC 2024
On 20/09/2024 10:06, Pierre-Eric Pelloux-Prayer wrote:
> This will allow to use flexible array to store the process name and
> other information.
>
> This also means that process name will be determined once and for all,
> instead of at each submit.
But the pid and others can still change? By design?
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index e20d19ae01b2..690676cab022 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2331,7 +2331,7 @@ amdgpu_vm_get_task_info_vm(struct amdgpu_vm *vm)
> {
> struct amdgpu_task_info *ti = NULL;
>
> - if (vm) {
> + if (vm && vm->task_info) {
> ti = vm->task_info;
> kref_get(&vm->task_info->refcount);
> }
> @@ -2372,8 +2372,12 @@ static int amdgpu_vm_create_task_info(struct amdgpu_vm *vm)
> */
> void amdgpu_vm_set_task_info(struct amdgpu_vm *vm)
> {
> - if (!vm->task_info)
> - return;
> + if (!vm->task_info) {
> + if (amdgpu_vm_create_task_info(vm))
> + return;
> +
> + get_task_comm(vm->task_info->process_name, current->group_leader);
> + }
>
> if (vm->task_info->pid == current->pid)
This ends up relying on vm->task_info->pid being zero due kzalloc right?
> return;
> @@ -2385,7 +2389,6 @@ void amdgpu_vm_set_task_info(struct amdgpu_vm *vm)
> return;
>
> vm->task_info->tgid = current->group_leader->pid;
> - get_task_comm(vm->task_info->process_name, current->group_leader);
> }
I wonder how many of the task_info fields you want to set once instead
of per submission. Like a fully one shot like the below be what you want?
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index a060c28f0877..da492223a8b5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2349,16 +2349,6 @@ 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)
-{
- vm->task_info = kzalloc(sizeof(struct amdgpu_task_info), GFP_KERNEL);
- if (!vm->task_info)
- return -ENOMEM;
-
- kref_init(&vm->task_info->refcount);
- return 0;
-}
-
/**
* amdgpu_vm_set_task_info - Sets VMs task info.
*
@@ -2366,20 +2356,28 @@ static int amdgpu_vm_create_task_info(struct
amdgpu_vm *vm)
*/
void amdgpu_vm_set_task_info(struct amdgpu_vm *vm)
{
- if (!vm->task_info)
- return;
+ struct amdgpu_task_info *task_info = vm->task_info;
+
+ if (!task_info) {
+ task_info = kzalloc(sizeof(struct amdgpu_task_info),
+ GFP_KERNEL);
+ if (!task_info)
+ return;
- if (vm->task_info->pid == current->pid)
+ kref_init(&task_info->refcount);
+ } else {
return;
+ }
- vm->task_info->pid = current->pid;
- get_task_comm(vm->task_info->task_name, current);
+ task_info->pid = current->pid;
+ get_task_comm(task_info->task_name, current);
- if (current->group_leader->mm != current->mm)
- return;
+ if (current->group_leader->mm == current->mm) {
+ task_info->tgid = current->group_leader->pid;
+ get_task_comm(task_info->process_name, current->group_leader);
+ }
- vm->task_info->tgid = current->group_leader->pid;
- get_task_comm(vm->task_info->process_name, current->group_leader);
+ vm->task_info = task_info;
}
/**
End result is code like this:
void amdgpu_vm_set_task_info(struct amdgpu_vm *vm)
{
struct amdgpu_task_info *task_info = vm->task_info;
if (!task_info) {
task_info = kzalloc(sizeof(struct amdgpu_task_info),
GFP_KERNEL);
if (!task_info)
return;
kref_init(&task_info->refcount);
} else {
return;
}
task_info->pid = current->pid;
get_task_comm(task_info->task_name, current);
if (current->group_leader->mm == current->mm) {
task_info->tgid = current->group_leader->pid;
get_task_comm(task_info->process_name, current->group_leader);
}
vm->task_info = task_info;
}
?
>
> /**
> @@ -2482,7 +2485,6 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> if (r)
> goto error_free_root;
>
> - r = amdgpu_vm_create_task_info(vm);
> if (r)
> DRM_DEBUG("Failed to create task info for VM\n");
Two more lines to delete here.
Regards,
Tvrtko
>
> @@ -2608,7 +2610,8 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
>
> root = amdgpu_bo_ref(vm->root.bo);
> amdgpu_bo_reserve(root, true);
> - amdgpu_vm_put_task_info(vm->task_info);
> + if (vm->task_info)
> + amdgpu_vm_put_task_info(vm->task_info);
> amdgpu_vm_set_pasid(adev, vm, 0);
> dma_fence_wait(vm->last_unlocked, false);
> dma_fence_put(vm->last_unlocked);
More information about the dri-devel
mailing list