Hello Felix, Yes I agree the user space may be buggy, and the kernel should be robust.
Even though it is named locking, it is actually a reservation, not recursive locking. The kfd_process that successfully invokes the ioctl reserves the device as a profiler. So when a user space locks the device multiple times, it does not increase any reference counters, and doesn't need to unlock multiple times. Multiple times lockings from the same kfd_process just keeps the reservation, it still reserves the device successfully, so IMHO kfd should return 0 other than -EALREADY, which is a negative error code. By the way, the function is declared to return uint32_t, which is definitely wrong, because the function may return errors, for example -EBUSY. Thanks Lingshan On 8/13/2026 12:27 AM, Kuehling, Felix wrote: > I suspect this won't work correctly. If the profiler locks the device > multiple times, it probably also unlocks it multiple times. But I see > no reference counting being done, so this may result in a situation > where user mode locks twice, and unlocks once. Now it's lost the lock > but it was still expecting to have it from the first lock operation. > > I suspect that this is a user mode bug, and KFD is working as expected. > > Regards, > Felix > > On 2026-08-12 03:14, Zhu, Lingshan wrote: >> AMD General >> >> Gentle Ping. >> >> Thanks >> Lingshan >> >> -----Original Message----- >> From: Zhu, Lingshan <[email protected]> >> Sent: Thursday, August 6, 2026 4:43 PM >> To: Deucher, Alexander <[email protected]>; Koenig, Christian >> <[email protected]>; Kuehling, Felix <[email protected]> >> Cc: Huang, Ray <[email protected]>; [email protected]; >> Zhu, Lingshan <[email protected]> >> Subject: [PATCH] amdkfd: allow profiler process reenter >> >> profile_lock_device() currently returns -EALREADY when the current >> profiler process requests to lock the device again. However any >> negtive values here are treated as failure, so this causes rejecting >> a profiler operation even though the caller still owns the device >> locking. >> >> This commit fixes this problem by returning 0 when the current >> profiler process locking the device again. >> >> This commit also changes profile_lock_device() to return an int value >> other than uint32_t, because it returns negative number when fail. >> >> Signed-off-by: Zhu Lingshan <[email protected]> >> --- >> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c >> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c >> index 309510e23315..9acd5c91e51c 100644 >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c >> @@ -3329,8 +3329,8 @@ static int kfd_ioctl_create_process(struct file >> *filep, struct kfd_process *p, v >> return 0; >> } >> >> -static inline uint32_t profile_lock_device(struct kfd_process *p, >> - uint32_t gpu_id, uint32_t op) >> +static inline int profile_lock_device(struct kfd_process *p, >> + uint32_t gpu_id, uint32_t op) >> { >> struct kfd_process_device *pdd; >> struct kfd_dev *kfd; >> @@ -3365,7 +3365,7 @@ static inline uint32_t >> profile_lock_device(struct kfd_process *p, >> } >> return status; >> } else if (kfd->profiler_process == p) { >> - status = -EALREADY; >> + status = 0; >> } else { >> status = -EBUSY; >> } >> -- >> 2.53.0 >>
