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@xxxxxxx>
---
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);
