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<[email protected]>
>> ---
>> 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
