AMD General Interesting, wonder how this was dropped. This has been present and been used by customers for a while (and this is an issue they would absolutely hit).
There was some weirdness in the migration if I remember correctly from DKMS -> Public when a migration for the PTL work took place and I suspect that maybe the change just wasn’t picked up or was dropped as part of that migration. What two branches do you need to check here? Ben From: Kuehling, Felix <[email protected]> Sent: Tuesday, August 4, 2026 3:41 PM To: Zhu, Lingshan <[email protected]>; Deucher, Alexander <[email protected]>; Koenig, Christian <[email protected]>; Welton, Benjamin <[email protected]> Cc: Huang, Ray <[email protected]>; [email protected] Subject: Re: [PATCH] amdkfd: fix deadlock in set_perfcount [+Benjamin] On 2026-07-31 05:51, Zhu Lingshan wrote: set_perfcount holds the DQM lock (dqm_lock(dqm) while walking queues, but pqm_update_mqd calls update_queue, which acquires the same lock by dqm_lock(dqm), causing deadlocks. This commit introduces update_queue_locked which assumes the dqm lock is already held, and update_queue calls it with proper lockings. set_perfcount calls update_queue_locked instead of pqm_update_mqd, therefore the deadlock is fixed. I think the patch looks fine to me. I'm just wondering how this bug was not noticed in developer testing of this feature. I see a very similar patch already exists on the DKMS branch but somehow it's missing in amd-staging-drm-next: commit 5cd0710732a45527b6398f8b522289f9adca708b Author: Benjamin Welton <[email protected]><mailto:[email protected]> AuthorDate: Mon Jan 6 10:30:01 2025 -0800 Commit: Chengjun Yao <[email protected]><mailto:[email protected]> CommitDate: Mon Jun 22 12:51:06 2026 +0800 amd/amdkfd: fix double lock aquisition in set_perfcount Seperates out locking from update_queue to allow updating of queues by code already holding the mqd lock. Fixes a hang in set_perfcount. This change was in the original mailing list commit for set_perfcount but was not included in gerrit. Fixes: b58289f0abf7 ("Add kfd_ioctl_profiler to contain profiler kernel driver changes") Signed-off-by: Benjamin Welton <[email protected]><mailto:[email protected]> Acked-by: Kent Russell <[email protected]><mailto:[email protected]> Benjamin, do you know what's going on here? I think the above patch just needs to be ported to amd-staging-drm-next. But then I'm wondering what else is missing. Or maybe the whole feature was submitted to amd-staging-drm-next by mistake. Regards, Felix Signed-off-by: Zhu Lingshan <[email protected]><mailto:[email protected]> --- .../drm/amd/amdkfd/kfd_device_queue_manager.c | 44 ++++++++++++------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c index 51ee9c39104b..c34c5f6a5541 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c @@ -77,6 +77,9 @@ static struct queue *find_queue_by_doorbell_offset(struct device_queue_manager * static void set_queue_as_reset(struct device_queue_manager *dqm, struct queue *q, struct qcm_process_device *qpd); static int reset_queues_mes(struct device_queue_manager *dqm, struct queue *q); +static int update_queue_locked(struct device_queue_manager *dqm, + struct queue *q, + struct mqd_update_info *minfo); static inline enum KFD_MQD_TYPE get_mqd_type_from_queue_type(enum kfd_queue_type type) @@ -341,8 +344,7 @@ static void set_perfcount(struct device_queue_manager *dqm, int enable) list_for_each_entry(cur, &dqm->queues, list) { qpd = cur->qpd; list_for_each_entry(q, &qpd->queues_list, list) { - pqm_update_mqd(qpd->pqm, q->properties.queue_id, - &minfo); + update_queue_locked(dqm, q, &minfo); } } dqm_unlock(dqm); @@ -1059,8 +1061,9 @@ static int destroy_queue_nocpsch(struct device_queue_manager *dqm, return retval; } -static int update_queue(struct device_queue_manager *dqm, struct queue *q, - struct mqd_update_info *minfo) +static int update_queue_locked(struct device_queue_manager *dqm, + struct queue *q, + struct mqd_update_info *minfo) { int retval = 0; struct device *dev = dqm->dev->adev->dev; @@ -1068,12 +1071,11 @@ static int update_queue(struct device_queue_manager *dqm, struct queue *q, struct kfd_process_device *pdd; bool prev_active = false; - dqm_lock(dqm); + lockdep_assert_held(&dqm->lock_hidden); + pdd = kfd_get_process_device_data(q->device, q->process); - if (!pdd) { - retval = -ENODEV; - goto out_unlock; - } + if (!pdd) + return -ENODEV; mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type( q->properties.type)]; @@ -1089,14 +1091,12 @@ static int update_queue(struct device_queue_manager *dqm, struct queue *q, retval = remove_queue_mes(dqm, q, &pdd->qpd); /* queue is reset so inaccessable */ - if (pdd->has_reset_queue) { - retval = -EACCES; - goto out_unlock; - } + if (pdd->has_reset_queue) + return -EACCES; if (retval) { dev_err(dev, "unmap queue failed\n"); - goto out_unlock; + return retval; } } else if (prev_active && (q->properties.type == KFD_QUEUE_TYPE_COMPUTE || @@ -1105,7 +1105,7 @@ static int update_queue(struct device_queue_manager *dqm, struct queue *q, if (!dqm->sched_running) { WARN_ONCE(1, "Update non-HWS queue while stopped\n"); - goto out_unlock; + return -EIO; } retval = mqd_mgr->destroy_mqd(mqd_mgr, q->mqd, @@ -1115,7 +1115,7 @@ static int update_queue(struct device_queue_manager *dqm, struct queue *q, KFD_UNMAP_LATENCY_MS, q->pipe, q->queue); if (retval) { dev_err(dev, "destroy mqd failed\n"); - goto out_unlock; + return retval; } } @@ -1163,8 +1163,18 @@ static int update_queue(struct device_queue_manager *dqm, struct queue *q, &q->properties, current->mm); } -out_unlock: + return retval; +} + +static int update_queue(struct device_queue_manager *dqm, struct queue *q, + struct mqd_update_info *minfo) +{ + int retval; + + dqm_lock(dqm); + retval = update_queue_locked(dqm, q, minfo); dqm_unlock(dqm); + return retval; }
