On 2024-10-09 14:08, Xiaogang.Chen wrote:
From: Xiaogang Chen <[email protected]>

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

That's not correct. kfd_create_process returns a struct kfd_process * reference. That gets stored by the caller in filep->private_data. That requires incrementing the reference count. You can have multiple references to the same struct kfd_process if user mode opens /dev/kfd multiple times. The reference is released in kfd_release. Your change breaks that use case.

The reference released in kfd_process_free_notifier is only one per process and it's the reference created by kref_init.

Regards,
  Felix



Signed-off-by: Xiaogang Chen <[email protected]>
---
  drivers/gpu/drm/amd/amdkfd/kfd_process.c | 8 +++++---
  1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index d07acf1b2f93..7c5471d7d743 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -899,8 +899,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);
@@ -1191,7 +1189,11 @@ static struct mmu_notifier *kfd_process_alloc_notifier(struct mm_struct *mm) srcu_read_unlock(&kfd_processes_srcu, idx); - return p ? &p->mmu_notifier : ERR_PTR(-ESRCH);
+       if (p) {
+               kref_get(&p->ref);
+               return &p->mmu_notifier;
+       }
+       return ERR_PTR(-ESRCH);
  }
static void kfd_process_free_notifier(struct mmu_notifier *mn)

Reply via email to