[PATCH v3] drm/amdkfd: change kfd process kref count at creation

[PATCH v3] drm/amdkfd: change kfd process kref count at creation

[Xiaogang.Chen]

From: Xiaogang Chen <xiaogang.chen@amd.com>

kfd process kref count(process->ref) is initialized to 1 by kref_init. After
it is created not need to increase its kref. Instad add kfd process kref at kfd
process mmu notifier allocation since we already decrease the kref at
free_notifier of mmu_notifier_ops, so pair them.

When user process opens kfd node multiple times the kfd process kref is
increased each time to balance with kfd node close operation.

Signed-off-by: Xiaogang Chen Xiaogang.Chen@amd.com
---
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index d07acf1b2f93..d871d5320297 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -850,8 +850,10 @@ struct kfd_process *kfd_create_process(struct task_struct *thread)
 		goto out;
 	}
 
-	/* A prior open of /dev/kfd could have already created the process. */
-	process = find_process(thread, false);
+	/* A prior open of /dev/kfd could have already created the process.
+	 * find_process will increase process kref in this case
+	 */
+	process = find_process(thread, true);
 	if (process) {
 		pr_debug("Process already found\n");
 	} else {
@@ -899,8 +901,6 @@ struct kfd_process *kfd_create_process(struct task_struct *thread)
 		init_waitqueue_head(&process->wait_irq_drain);
 	}
 out:
-	if (!IS_ERR(process))
-		kref_get(&process->ref);
 	mutex_unlock(&kfd_processes_mutex);
 	mmput(thread->mm);
 
@@ -1186,10 +1186,8 @@ static void kfd_process_ref_release(struct kref *ref)
 
 static struct mmu_notifier *kfd_process_alloc_notifier(struct mm_struct *mm)
 {
-	int idx = srcu_read_lock(&kfd_processes_srcu);
-	struct kfd_process *p = find_process_by_mm(mm);
-
-	srcu_read_unlock(&kfd_processes_srcu, idx);
+	/* This increments p->ref counter if kfd process p exists */
+	struct kfd_process *p = kfd_lookup_process_by_mm(mm);
 
 	return p ? &p->mmu_notifier : ERR_PTR(-ESRCH);
 }
-- 
2.25.1

Re: [PATCH v3] drm/amdkfd: change kfd process kref count at creation

[Chen, Xiaogang]

ping

On 10/28/2024 3:43 PM, Xiaogang.Chen wrote:
> From: Xiaogang Chen <xiaogang.chen@amd.com>
>
> kfd process kref count(process->ref) is initialized to 1 by kref_init. After
> it is created not need to increase its kref. Instad add kfd process kref at kfd
> process mmu notifier allocation since we already decrease the kref at
> free_notifier of mmu_notifier_ops, so pair them.
>
> When user process opens kfd node multiple times the kfd process kref is
> increased each time to balance with kfd node close operation.
>
> Signed-off-by: Xiaogang Chen Xiaogang.Chen@amd.com
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c | 14 ++++++--------
>   1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index d07acf1b2f93..d871d5320297 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -850,8 +850,10 @@ struct kfd_process *kfd_create_process(struct task_struct *thread)
>   		goto out;
>   	}
>   
> -	/* A prior open of /dev/kfd could have already created the process. */
> -	process = find_process(thread, false);
> +	/* A prior open of /dev/kfd could have already created the process.
> +	 * find_process will increase process kref in this case
> +	 */
> +	process = find_process(thread, true);
>   	if (process) {
>   		pr_debug("Process already found\n");
>   	} else {
> @@ -899,8 +901,6 @@ struct kfd_process *kfd_create_process(struct task_struct *thread)
>   		init_waitqueue_head(&process->wait_irq_drain);
>   	}
>   out:
> -	if (!IS_ERR(process))
> -		kref_get(&process->ref);
>   	mutex_unlock(&kfd_processes_mutex);
>   	mmput(thread->mm);
>   
> @@ -1186,10 +1186,8 @@ static void kfd_process_ref_release(struct kref *ref)
>   
>   static struct mmu_notifier *kfd_process_alloc_notifier(struct mm_struct *mm)
>   {
> -	int idx = srcu_read_lock(&kfd_processes_srcu);
> -	struct kfd_process *p = find_process_by_mm(mm);
> -
> -	srcu_read_unlock(&kfd_processes_srcu, idx);
> +	/* This increments p->ref counter if kfd process p exists */
> +	struct kfd_process *p = kfd_lookup_process_by_mm(mm);
>   
>   	return p ? &p->mmu_notifier : ERR_PTR(-ESRCH);
>   }

Re: [PATCH v3] drm/amdkfd: change kfd process kref count at creation

[Felix Kuehling]

On 2024-10-28 16:43, Xiaogang.Chen wrote:
> From: Xiaogang Chen <xiaogang.chen@amd.com>
>
> kfd process kref count(process->ref) is initialized to 1 by kref_init. After
> it is created not need to increase its kref. Instad add kfd process kref at kfd
> process mmu notifier allocation since we already decrease the kref at
> free_notifier of mmu_notifier_ops, so pair them.
>
> When user process opens kfd node multiple times the kfd process kref is
> increased each time to balance with kfd node close operation.
>
> Signed-off-by: Xiaogang Chen Xiaogang.Chen@amd.com

Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>


> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c | 14 ++++++--------
>   1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index d07acf1b2f93..d871d5320297 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -850,8 +850,10 @@ struct kfd_process *kfd_create_process(struct task_struct *thread)
>   		goto out;
>   	}
>   
> -	/* A prior open of /dev/kfd could have already created the process. */
> -	process = find_process(thread, false);
> +	/* A prior open of /dev/kfd could have already created the process.
> +	 * find_process will increase process kref in this case
> +	 */
> +	process = find_process(thread, true);
>   	if (process) {
>   		pr_debug("Process already found\n");
>   	} else {
> @@ -899,8 +901,6 @@ struct kfd_process *kfd_create_process(struct task_struct *thread)
>   		init_waitqueue_head(&process->wait_irq_drain);
>   	}
>   out:
> -	if (!IS_ERR(process))
> -		kref_get(&process->ref);
>   	mutex_unlock(&kfd_processes_mutex);
>   	mmput(thread->mm);
>   
> @@ -1186,10 +1186,8 @@ static void kfd_process_ref_release(struct kref *ref)
>   
>   static struct mmu_notifier *kfd_process_alloc_notifier(struct mm_struct *mm)
>   {
> -	int idx = srcu_read_lock(&kfd_processes_srcu);
> -	struct kfd_process *p = find_process_by_mm(mm);
> -
> -	srcu_read_unlock(&kfd_processes_srcu, idx);
> +	/* This increments p->ref counter if kfd process p exists */
> +	struct kfd_process *p = kfd_lookup_process_by_mm(mm);
>   
>   	return p ? &p->mmu_notifier : ERR_PTR(-ESRCH);
>   }