Re: [PATCH V5 18/18] amdkfd: introduce new ioctl AMDKFD_IOC_CREATE_PROCESS

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

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

On 10/18/2025 7:55 AM, Felix Kuehling wrote:
> On 2025-10-17 04:42, Zhu Lingshan wrote:
>> This commit implemetns a new ioctl AMDKFD_IOC_CREATE_PROCESS
>> that creates a new secondary kfd_progress on the FD.
>>
>> To keep backward compatibility, userspace programs need to invoke
>> this ioctl explicitly on a FD to create a secondary
>> kfd_process which replacing its primary kfd_process.
>>
>> This commit bumps ioctl minor version.
>>
>> Signed-off-by: Zhu Lingshan<lingshan.zhu@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 41 ++++++++++++++++++++++++
>>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  1 +
>>   drivers/gpu/drm/amd/amdkfd/kfd_process.c |  3 +-
>>   include/uapi/linux/kfd_ioctl.h           |  8 +++--
>>   4 files changed, 49 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>> index 7c02e8473622..8fe58526bc3a 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>> @@ -3140,6 +3140,44 @@ static int kfd_ioctl_set_debug_trap(struct
>> file *filep, struct kfd_process *p, v
>>       return r;
>>   }
>>   +/* userspace programs need to invoke this ioctl explicitly on a FD to
>> + * create a secondary kfd_process which replacing its primary
>> kfd_process
>> + */
>> +static int kfd_ioctl_create_process(struct file *filep, struct
>> kfd_process *p, void *data)
>> +{
>> +    struct kfd_process *process;
>> +    int ret;
>> +
>> +    if (!filep->private_data || !p)
>> +        return -EINVAL;
>> +
>> +    if (p != filep->private_data)
>> +        return -EINVAL;
>> +
>> +    /* Each FD owns only one kfd_process */
>> +    if (p->context_id != KFD_CONTEXT_ID_PRIMARY)
>> +        return -EINVAL;
>> +
>> +    mutex_lock(&kfd_processes_mutex);
>> +    process = create_process(current, false);
>> +    mutex_unlock(&kfd_processes_mutex);
>
> There could be race conditions between multiple threads calling this
> ioctl on the same primary fd concurrently. We could use the
> kfd_processes_mutex to protect against this. You'd need to do some of
> the above checks and the assignment to filep->private_data under the
> lock to be safe, to allow only one thread to successfully create the
> secondary context. I think this would work:
>
>     /* Atomically create the secondary context and assign it to
> filep->private_data */
>     mutex_lock(&kfd_processes_mutex);
>     if (p != filep->private_data) {
>         /* Another thread already created a secondary context */
>         mutex_unlock(&kfd_processes_mutex);
>         return -EINVAL;
>     }
>     secondary = create_process(current, false);
>     if (!IS_ERR(process))
>         filep->private_data = secondary;
>     mutex_unlock(&kfd_processes_mutex);
>
>     if (IS_ERR(secondary))
>         return PTR_ERR(secondary);
>
>     /* Release the process reference that was held by
> filep->private_data */
>     kfd_unref_process(p);
>
>     ... 
>
will fix in V6, thanks!

> Regards,
>   Felix
>
>
>> +
>> +    if (IS_ERR(process))
>> +        return PTR_ERR(process);
>> +
>> +    /* Each open() increases kref of the primary kfd_process,
>> +     * so we need to reduce it here before we create a new secondary
>> process replacing it
>> +     */
>> +    kfd_unref_process(p);
>> +
>> +    filep->private_data = process;
>> +    ret = kfd_create_process_sysfs(process);
>> +    if (ret)
>> +        pr_warn("Failed to create sysfs entry for the kfd_process");
>> +
>> +    return 0;
>> +}
>> +
>>   #define AMDKFD_IOCTL_DEF(ioctl, _func, _flags) \
>>       [_IOC_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags =
>> _flags, \
>>                   .cmd_drv = 0, .name = #ioctl}
>> @@ -3258,6 +3296,9 @@ static const struct amdkfd_ioctl_desc
>> amdkfd_ioctls[] = {
>>         AMDKFD_IOCTL_DEF(AMDKFD_IOC_DBG_TRAP,
>>               kfd_ioctl_set_debug_trap, 0),
>> +
>> +    AMDKFD_IOCTL_DEF(AMDKFD_IOC_CREATE_PROCESS,
>> +            kfd_ioctl_create_process, 0),
>>   };
>>     #define AMDKFD_CORE_IOCTL_COUNT    ARRAY_SIZE(amdkfd_ioctls)
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> index 4237c859050d..578f9f217e16 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> @@ -1055,6 +1055,7 @@ struct amdkfd_ioctl_desc {
>>   };
>>   bool kfd_dev_is_large_bar(struct kfd_node *dev);
>>   +struct kfd_process *create_process(const struct task_struct
>> *thread, bool primary);
>>   int kfd_process_create_wq(void);
>>   void kfd_process_destroy_wq(void);
>>   void kfd_cleanup_processes(void);
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> index 629a706e2a13..bbb72a77bed5 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> @@ -68,7 +68,6 @@ static struct workqueue_struct *kfd_restore_wq;
>>   static struct kfd_process *find_process(const struct task_struct
>> *thread,
>>                       bool ref);
>>   static void kfd_process_ref_release(struct kref *ref);
>> -static struct kfd_process *create_process(const struct task_struct
>> *thread, bool primary);
>>     static void evict_process_worker(struct work_struct *work);
>>   static void restore_process_worker(struct work_struct *work);
>> @@ -1578,7 +1577,7 @@ void kfd_process_set_trap_debug_flag(struct
>> qcm_process_device *qpd,
>>    * On return the kfd_process is fully operational and will be freed
>> when the
>>    * mm is released
>>    */
>> -static struct kfd_process *create_process(const struct task_struct
>> *thread, bool primary)
>> +struct kfd_process *create_process(const struct task_struct *thread,
>> bool primary)
>>   {
>>       struct kfd_process *process;
>>       struct mmu_notifier *mn;
>> diff --git a/include/uapi/linux/kfd_ioctl.h
>> b/include/uapi/linux/kfd_ioctl.h
>> index 5d1727a6d040..84aa24c02715 100644
>> --- a/include/uapi/linux/kfd_ioctl.h
>> +++ b/include/uapi/linux/kfd_ioctl.h
>> @@ -44,9 +44,10 @@
>>    * - 1.16 - Add contiguous VRAM allocation flag
>>    * - 1.17 - Add SDMA queue creation with target SDMA engine ID
>>    * - 1.18 - Rename pad in set_memory_policy_args to misc_process_flag
>> + * - 1.19 - Add a new ioctl to craete secondary kfd processes
>>    */
>>   #define KFD_IOCTL_MAJOR_VERSION 1
>> -#define KFD_IOCTL_MINOR_VERSION 18
>> +#define KFD_IOCTL_MINOR_VERSION 19
>>     struct kfd_ioctl_get_version_args {
>>       __u32 major_version;    /* from KFD */
>> @@ -1671,7 +1672,10 @@ struct kfd_ioctl_dbg_trap_args {
>>   #define AMDKFD_IOC_DBG_TRAP            \
>>           AMDKFD_IOWR(0x26, struct kfd_ioctl_dbg_trap_args)
>>   +#define AMDKFD_IOC_CREATE_PROCESS        \
>> +        AMDKFD_IO(0x27)
>> +
>>   #define AMDKFD_COMMAND_START        0x01
>> -#define AMDKFD_COMMAND_END        0x27
>> +#define AMDKFD_COMMAND_END        0x28
>>     #endif

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