diff mbox series

[v5,4/6] drm/amdgpu: alloc and init vm::task_info from first submit

Message ID 20241003124506.470931-5-pierre-eric.pelloux-prayer@amd.com (mailing list archive)
State New, archived
Headers show
Series DRM_SET_CLIENT_NAME ioctl | expand

Commit Message

Pierre-Eric Pelloux-Prayer Oct. 3, 2024, 12:43 p.m. UTC
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.

Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 29 ++++++++++++--------------
 1 file changed, 13 insertions(+), 16 deletions(-)

Comments

Christian König Oct. 8, 2024, 8:13 a.m. UTC | #1
Am 03.10.24 um 14:43 schrieb Pierre-Eric Pelloux-Prayer:
> 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.
>
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 29 ++++++++++++--------------
>   1 file changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index e20d19ae01b2..6cd5bd5362d4 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);
>   	}
> @@ -2361,6 +2361,10 @@ static int amdgpu_vm_create_task_info(struct amdgpu_vm *vm)
>   	if (!vm->task_info)
>   		return -ENOMEM;
>   
> +	/* Set process attributes now. */
> +	vm->task_info->tgid = current->group_leader->pid;
> +	get_task_comm(vm->task_info->process_name, current->group_leader);
> +
>   	kref_init(&vm->task_info->refcount);
>   	return 0;
>   }
> @@ -2372,20 +2376,16 @@ 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->pid == current->pid)
> +	if (!vm->task_info) {
> +		if (amdgpu_vm_create_task_info(vm))
> +			return;
> +	} else if (vm->task_info->pid == current->pid) {
>   		return;
> +	}
>   
> +	/* Update task attributes. */
>   	vm->task_info->pid = current->pid;
>   	get_task_comm(vm->task_info->task_name, current);
> -
> -	if (current->group_leader->mm != current->mm)
> -		return;
> -
> -	vm->task_info->tgid = current->group_leader->pid;
> -	get_task_comm(vm->task_info->process_name, current->group_leader);
>   }
>   
>   /**
> @@ -2482,10 +2482,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");
> -
>   	amdgpu_bo_unreserve(vm->root.bo);
>   	amdgpu_bo_unref(&root_bo);
>   
> @@ -2608,7 +2604,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);

That NULL check should probably be moved into amdgpu_vm_put_task_info().

Apart from that the patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>.

Regards,
Christian.

>   	amdgpu_vm_set_pasid(adev, vm, 0);
>   	dma_fence_wait(vm->last_unlocked, false);
>   	dma_fence_put(vm->last_unlocked);
Pierre-Eric Pelloux-Prayer Oct. 9, 2024, 7:52 a.m. UTC | #2
Le 08/10/2024 à 10:13, Christian König a écrit :
> Am 03.10.24 um 14:43 schrieb Pierre-Eric Pelloux-Prayer:
>> 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.
>>
>> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 29 ++++++++++++--------------
>>   1 file changed, 13 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index e20d19ae01b2..6cd5bd5362d4 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);
>>       }
>> @@ -2361,6 +2361,10 @@ static int amdgpu_vm_create_task_info(struct amdgpu_vm *vm)
>>       if (!vm->task_info)
>>           return -ENOMEM;
>> +    /* Set process attributes now. */
>> +    vm->task_info->tgid = current->group_leader->pid;
>> +    get_task_comm(vm->task_info->process_name, current->group_leader);
>> +
>>       kref_init(&vm->task_info->refcount);
>>       return 0;
>>   }
>> @@ -2372,20 +2376,16 @@ 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->pid == current->pid)
>> +    if (!vm->task_info) {
>> +        if (amdgpu_vm_create_task_info(vm))
>> +            return;
>> +    } else if (vm->task_info->pid == current->pid) {
>>           return;
>> +    }
>> +    /* Update task attributes. */
>>       vm->task_info->pid = current->pid;
>>       get_task_comm(vm->task_info->task_name, current);
>> -
>> -    if (current->group_leader->mm != current->mm)
>> -        return;
>> -
>> -    vm->task_info->tgid = current->group_leader->pid;
>> -    get_task_comm(vm->task_info->process_name, current->group_leader);
>>   }
>>   /**
>> @@ -2482,10 +2482,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");
>> -
>>       amdgpu_bo_unreserve(vm->root.bo);
>>       amdgpu_bo_unref(&root_bo);
>> @@ -2608,7 +2604,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);
> 
> That NULL check should probably be moved into amdgpu_vm_put_task_info().
> 
> Apart from that the patch is Reviewed-by: Christian König <christian.koenig@amd.com>.

Fixed in attached patch.

Pierre-Eric

> 
> Regards,
> Christian.
> 
>>       amdgpu_vm_set_pasid(adev, vm, 0);
>>       dma_fence_wait(vm->last_unlocked, false);
>>       dma_fence_put(vm->last_unlocked);
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index e20d19ae01b2..6cd5bd5362d4 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);
 	}
@@ -2361,6 +2361,10 @@  static int amdgpu_vm_create_task_info(struct amdgpu_vm *vm)
 	if (!vm->task_info)
 		return -ENOMEM;
 
+	/* Set process attributes now. */
+	vm->task_info->tgid = current->group_leader->pid;
+	get_task_comm(vm->task_info->process_name, current->group_leader);
+
 	kref_init(&vm->task_info->refcount);
 	return 0;
 }
@@ -2372,20 +2376,16 @@  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->pid == current->pid)
+	if (!vm->task_info) {
+		if (amdgpu_vm_create_task_info(vm))
+			return;
+	} else if (vm->task_info->pid == current->pid) {
 		return;
+	}
 
+	/* Update task attributes. */
 	vm->task_info->pid = current->pid;
 	get_task_comm(vm->task_info->task_name, current);
-
-	if (current->group_leader->mm != current->mm)
-		return;
-
-	vm->task_info->tgid = current->group_leader->pid;
-	get_task_comm(vm->task_info->process_name, current->group_leader);
 }
 
 /**
@@ -2482,10 +2482,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");
-
 	amdgpu_bo_unreserve(vm->root.bo);
 	amdgpu_bo_unref(&root_bo);
 
@@ -2608,7 +2604,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);