Re: [PATCH V5 08/18] amdkfd: identify a secondary kfd process by its id

["Zhu, Lingshan" <lingshan.zhu@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 6619 bytes --]

On 10/18/2025 7:16 AM, Felix Kuehling wrote:
> On 2025-10-17 04:42, Zhu Lingshan wrote:
>> This commit introduces a new id field for
>> struct kfd process, which helps identify
>> a kfd process among multiple contexts that
>> all belong to a single user space program.
>>
>> The sysfs entry of a secondary kfd process
>> is placed under the sysfs entry folder of
>> its primary kfd process.
>>
>> The naming format of the sysfs entry of a secondary
>> kfd process is "context_%u" where %u is the process id.
>>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  6 ++
>>   drivers/gpu/drm/amd/amdkfd/kfd_process.c | 78 +++++++++++++++++++++++-
>>   2 files changed, 81 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> index 919510f18249..9de658119cd9 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> @@ -1021,10 +1021,16 @@ struct kfd_process {
>>         /*kfd context id */
>>       u16 context_id;
>> +
>> +    /* The primary kfd_process allocating IDs for its secondary
>> kfd_process, 0 for primary kfd_process */
>> +    struct ida id_table;
>> +
>>   };
>>     #define KFD_PROCESS_TABLE_SIZE 8 /* bits: 256 entries */
>>   #define KFD_CONTEXT_ID_PRIMARY    0xFFFF
>> +#define KFD_CONTEXT_ID_MIN 0
>> +#define KFD_CONTEXT_ID_WIDTH 16
>>     extern DECLARE_HASHTABLE(kfd_processes_table,
>> KFD_PROCESS_TABLE_SIZE);
>>   extern struct srcu_struct kfd_processes_srcu;
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> index 995d27be06e3..157145c94314 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> @@ -827,6 +827,7 @@ static void
>> kfd_process_device_destroy_ib_mem(struct kfd_process_device *pdd)
>>     int kfd_create_process_sysfs(struct kfd_process *process)
>>   {
>> +    struct kfd_process *primary_process;
>>       int ret;
>>         if (process->kobj) {
>> @@ -839,9 +840,22 @@ int kfd_create_process_sysfs(struct kfd_process
>> *process)
>>           pr_warn("Creating procfs kobject failed");
>>           return -ENOMEM;
>>       }
>> -    ret = kobject_init_and_add(process->kobj, &procfs_type,
>> -                   procfs.kobj, "%d",
>> -                   (int)process->lead_thread->pid);
>> +
>> +    if (process->context_id == KFD_CONTEXT_ID_PRIMARY)
>> +        ret = kobject_init_and_add(process->kobj, &procfs_type,
>> +                       procfs.kobj, "%d",
>> +                       (int)process->lead_thread->pid);
>> +    else {
>> +        primary_process =
>> kfd_lookup_process_by_mm(process->lead_thread->mm);
>> +        if (!primary_process)
>> +            return -ESRCH;
>> +
>> +        ret = kobject_init_and_add(process->kobj, &procfs_type,
>> +                       primary_process->kobj, "context_%u",
>> +                       process->context_id);
>> +        kfd_unref_process(primary_process);
>> +    }
>> +
>>       if (ret) {
>>           pr_warn("Creating procfs pid directory failed");
>>           kobject_put(process->kobj);
>> @@ -863,6 +877,50 @@ int kfd_create_process_sysfs(struct kfd_process
>> *process)
>>       return 0;
>>   }
>>   +static int kfd_process_alloc_id(struct kfd_process *process)
>> +{
>> +    int ret;
>> +    struct kfd_process *primary_process;
>> +
>> +    /* already assign 0xFFFF when create */
>> +    if (process->context_id == KFD_CONTEXT_ID_PRIMARY)
>> +        return 0;
>> +
>> +    primary_process =
>> kfd_lookup_process_by_mm(process->lead_thread->mm);
>> +    if (!primary_process)
>> +        return -ESRCH;
>> +
>> +    /* id range: KFD_CONTEXT_ID_MIN to 0xFFFE */
>> +    ret = ida_alloc_range(&primary_process->id_table,
>> KFD_CONTEXT_ID_MIN,
>> +         (1 << KFD_CONTEXT_ID_WIDTH) - 2, GFP_KERNEL);
>
> This would be safer and more obvious if you just set the upper limit
> as KFD_CONTEXT_ID_PRIMARY - 1. Then you don't need
> KFD_CONTEXT_ID_WIDTH at all. 

will fix in V6, thanks!

>
> Regards,
>   Felix
>
>
>> +    if (ret < 0)
>> +        goto out;
>> +
>> +    process->context_id = ret;
>> +    ret = 0;
>> +
>> +out:
>> +    kfd_unref_process(primary_process);
>> +
>> +    return ret;
>> +}
>> +
>> +static void kfd_process_free_id(struct kfd_process *process)
>> +{
>> +    struct kfd_process *primary_process;
>> +
>> +    if (process->context_id != KFD_CONTEXT_ID_PRIMARY)
>> +        return;
>> +
>> +    primary_process =
>> kfd_lookup_process_by_mm(process->lead_thread->mm);
>> +    if (!primary_process)
>> +        return;
>> +
>> +    ida_free(&primary_process->id_table, process->context_id);
>> +
>> +    kfd_unref_process(primary_process);
>> +}
>> +
>>   struct kfd_process *kfd_create_process(struct task_struct *thread)
>>   {
>>       struct kfd_process *process;
>> @@ -1195,6 +1253,11 @@ static void kfd_process_wq_release(struct
>> work_struct *work)
>>       if (ef)
>>           dma_fence_signal(ef);
>>   +    if (p->context_id != KFD_CONTEXT_ID_PRIMARY)
>> +        kfd_process_free_id(p);
>> +    else
>> +        ida_destroy(&p->id_table);
>> +
>>       kfd_process_remove_sysfs(p);
>>       kfd_debugfs_remove_process(p);
>>   @@ -1601,6 +1664,13 @@ static struct kfd_process
>> *create_process(const struct task_struct *thread, bool
>>               goto err_register_notifier;
>>           }
>>           BUG_ON(mn != &process->mmu_notifier);
>> +        ida_init(&process->id_table);
>> +    }
>> +
>> +    err = kfd_process_alloc_id(process);
>> +    if (err) {
>> +        pr_err("Creating kfd process: failed to alloc an id\n");
>> +        goto err_alloc_id;
>>       }
>>         kfd_unref_process(process);
>> @@ -1610,6 +1680,8 @@ static struct kfd_process *create_process(const
>> struct task_struct *thread, bool
>>         return process;
>>   +err_alloc_id:
>> +    kfd_process_free_id(process);
>>   err_register_notifier:
>>       hash_del_rcu(&process->kfd_processes);
>>       svm_range_list_fini(process);

[-- Attachment #2: Type: text/html, Size: 12403 bytes --]