* [PATCH V1] drm/amdgpu/userq: unblock signal ioctl from userq_mutex
@ 2026-04-08 8:30 Sunil Khatri
2026-04-08 8:32 ` Christian König
0 siblings, 1 reply; 3+ messages in thread
From: Sunil Khatri @ 2026-04-08 8:30 UTC (permalink / raw)
To: Alex Deucher, Christian König; +Cc: amd-gfx, Sunil Khatri
Signal ioctl does not depend on eviction fence and neither it
needs to hold userq_mutex as we just wait for userq fences
to be signalled before we go to suspend state in hardware
and we prempt and unmap the queues.
This unblocks other thread which hold userq_mutex to go on
without breaking the code.
Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
---
.../drm/amd/amdgpu/amdgpu_eviction_fence.c | 38 +++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 18 ---------
2 files changed, 38 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
index 5ae477c49a53..53f0bf6b3ca0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
@@ -54,6 +54,41 @@ static const struct dma_fence_ops amdgpu_eviction_fence_ops = {
.enable_signaling = amdgpu_eviction_fence_enable_signaling,
};
+static void
+amdgpu_userq_wait_for_signal(struct amdgpu_userq_mgr *uq_mgr)
+{
+ struct amdgpu_usermode_queue *queue;
+ unsigned long queue_id = 0;
+ struct dma_fence *f;
+
+ for (;;) {
+ xa_lock(&uq_mgr->userq_xa);
+ queue = xa_find(&uq_mgr->userq_xa, &queue_id, ULONG_MAX,
+ XA_PRESENT);
+ if (!queue) {
+ xa_unlock(&uq_mgr->userq_xa);
+ break;
+ }
+
+ kref_get(&queue->refcount);
+ xa_unlock(&uq_mgr->userq_xa);
+
+ mutex_lock(&uq_mgr->userq_mutex);
+ f = dma_fence_get(queue->last_fence);
+ mutex_unlock(&uq_mgr->userq_mutex);
+ if (!f) {
+ amdgpu_userq_put(queue);
+ queue_id++;
+ continue;
+ }
+
+ dma_fence_wait(f, false);
+ dma_fence_put(f);
+ amdgpu_userq_put(queue);
+ queue_id++;
+ }
+}
+
static void
amdgpu_eviction_fence_suspend_worker(struct work_struct *work)
{
@@ -66,6 +101,9 @@ amdgpu_eviction_fence_suspend_worker(struct work_struct *work)
struct dma_fence *ev_fence;
bool cookie;
+ /* Wait for any pending userqueue fence work to finish */
+ amdgpu_userq_wait_for_signal(uq_mgr);
+
mutex_lock(&uq_mgr->userq_mutex);
/*
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index c4e92113b557..1e8c08b63f65 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -1269,27 +1269,9 @@ void amdgpu_userq_reset_work(struct work_struct *work)
amdgpu_device_gpu_recover(adev, NULL, &reset_context);
}
-static void
-amdgpu_userq_wait_for_signal(struct amdgpu_userq_mgr *uq_mgr)
-{
- struct amdgpu_usermode_queue *queue;
- unsigned long queue_id;
-
- xa_for_each(&uq_mgr->userq_xa, queue_id, queue) {
- struct dma_fence *f = queue->last_fence;
-
- if (!f)
- continue;
-
- dma_fence_wait(f, false);
- }
-}
-
void
amdgpu_userq_evict(struct amdgpu_userq_mgr *uq_mgr)
{
- /* Wait for any pending userqueue fence work to finish */
- amdgpu_userq_wait_for_signal(uq_mgr);
amdgpu_userq_evict_all(uq_mgr);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH V1] drm/amdgpu/userq: unblock signal ioctl from userq_mutex
2026-04-08 8:30 [PATCH V1] drm/amdgpu/userq: unblock signal ioctl from userq_mutex Sunil Khatri
@ 2026-04-08 8:32 ` Christian König
2026-04-08 8:38 ` Khatri, Sunil
0 siblings, 1 reply; 3+ messages in thread
From: Christian König @ 2026-04-08 8:32 UTC (permalink / raw)
To: Sunil Khatri, Alex Deucher; +Cc: amd-gfx
On 4/8/26 10:30, Sunil Khatri wrote:
> Signal ioctl does not depend on eviction fence and neither it
> needs to hold userq_mutex as we just wait for userq fences
> to be signalled before we go to suspend state in hardware
> and we prempt and unmap the queues.
>
> This unblocks other thread which hold userq_mutex to go on
> without breaking the code.
>
> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
> ---
> .../drm/amd/amdgpu/amdgpu_eviction_fence.c | 38 +++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 18 ---------
> 2 files changed, 38 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
> index 5ae477c49a53..53f0bf6b3ca0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
> @@ -54,6 +54,41 @@ static const struct dma_fence_ops amdgpu_eviction_fence_ops = {
> .enable_signaling = amdgpu_eviction_fence_enable_signaling,
> };
>
> +static void
> +amdgpu_userq_wait_for_signal(struct amdgpu_userq_mgr *uq_mgr)
> +{
> + struct amdgpu_usermode_queue *queue;
> + unsigned long queue_id = 0;
> + struct dma_fence *f;
> +
> + for (;;) {
> + xa_lock(&uq_mgr->userq_xa);
> + queue = xa_find(&uq_mgr->userq_xa, &queue_id, ULONG_MAX,
> + XA_PRESENT);
> + if (!queue) {
> + xa_unlock(&uq_mgr->userq_xa);
> + break;
> + }
> +
> + kref_get(&queue->refcount);
> + xa_unlock(&uq_mgr->userq_xa);
> +
> + mutex_lock(&uq_mgr->userq_mutex);
> + f = dma_fence_get(queue->last_fence);
> + mutex_unlock(&uq_mgr->userq_mutex);
> + if (!f) {
> + amdgpu_userq_put(queue);
> + queue_id++;
> + continue;
> + }
> +
> + dma_fence_wait(f, false);
That doesn't looks correct to me. We need to hold the userq_mutex while waiting for this fence to make sure that userspace doesn't installs a new one.
I've already pointed that out on teams.
Regards,
Christian.
> + dma_fence_put(f);
> + amdgpu_userq_put(queue);
> + queue_id++;
> + }
> +}
> +
> static void
> amdgpu_eviction_fence_suspend_worker(struct work_struct *work)
> {
> @@ -66,6 +101,9 @@ amdgpu_eviction_fence_suspend_worker(struct work_struct *work)
> struct dma_fence *ev_fence;
> bool cookie;
>
> + /* Wait for any pending userqueue fence work to finish */
> + amdgpu_userq_wait_for_signal(uq_mgr);
> +
> mutex_lock(&uq_mgr->userq_mutex);
>
> /*
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index c4e92113b557..1e8c08b63f65 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -1269,27 +1269,9 @@ void amdgpu_userq_reset_work(struct work_struct *work)
> amdgpu_device_gpu_recover(adev, NULL, &reset_context);
> }
>
> -static void
> -amdgpu_userq_wait_for_signal(struct amdgpu_userq_mgr *uq_mgr)
> -{
> - struct amdgpu_usermode_queue *queue;
> - unsigned long queue_id;
> -
> - xa_for_each(&uq_mgr->userq_xa, queue_id, queue) {
> - struct dma_fence *f = queue->last_fence;
> -
> - if (!f)
> - continue;
> -
> - dma_fence_wait(f, false);
> - }
> -}
> -
> void
> amdgpu_userq_evict(struct amdgpu_userq_mgr *uq_mgr)
> {
> - /* Wait for any pending userqueue fence work to finish */
> - amdgpu_userq_wait_for_signal(uq_mgr);
> amdgpu_userq_evict_all(uq_mgr);
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH V1] drm/amdgpu/userq: unblock signal ioctl from userq_mutex
2026-04-08 8:32 ` Christian König
@ 2026-04-08 8:38 ` Khatri, Sunil
0 siblings, 0 replies; 3+ messages in thread
From: Khatri, Sunil @ 2026-04-08 8:38 UTC (permalink / raw)
To: Christian König, Sunil Khatri, Alex Deucher; +Cc: amd-gfx
On 08-04-2026 02:02 pm, Christian König wrote:
> On 4/8/26 10:30, Sunil Khatri wrote:
>> Signal ioctl does not depend on eviction fence and neither it
>> needs to hold userq_mutex as we just wait for userq fences
>> to be signalled before we go to suspend state in hardware
>> and we prempt and unmap the queues.
>>
>> This unblocks other thread which hold userq_mutex to go on
>> without breaking the code.
>>
>> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
>> ---
>> .../drm/amd/amdgpu/amdgpu_eviction_fence.c | 38 +++++++++++++++++++
>> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 18 ---------
>> 2 files changed, 38 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>> index 5ae477c49a53..53f0bf6b3ca0 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>> @@ -54,6 +54,41 @@ static const struct dma_fence_ops amdgpu_eviction_fence_ops = {
>> .enable_signaling = amdgpu_eviction_fence_enable_signaling,
>> };
>>
>> +static void
>> +amdgpu_userq_wait_for_signal(struct amdgpu_userq_mgr *uq_mgr)
>> +{
>> + struct amdgpu_usermode_queue *queue;
>> + unsigned long queue_id = 0;
>> + struct dma_fence *f;
>> +
>> + for (;;) {
>> + xa_lock(&uq_mgr->userq_xa);
>> + queue = xa_find(&uq_mgr->userq_xa, &queue_id, ULONG_MAX,
>> + XA_PRESENT);
>> + if (!queue) {
>> + xa_unlock(&uq_mgr->userq_xa);
>> + break;
>> + }
>> +
>> + kref_get(&queue->refcount);
>> + xa_unlock(&uq_mgr->userq_xa);
>> +
>> + mutex_lock(&uq_mgr->userq_mutex);
>> + f = dma_fence_get(queue->last_fence);
>> + mutex_unlock(&uq_mgr->userq_mutex);
>> + if (!f) {
>> + amdgpu_userq_put(queue);
>> + queue_id++;
>> + continue;
>> + }
>> +
>> + dma_fence_wait(f, false);
> That doesn't looks correct to me. We need to hold the userq_mutex while waiting for this fence to make sure that userspace doesn't installs a new one.
>
> I've already pointed that out on teams.
Ok in that case dma_fence_wait will remain with userq_mutex. Got it.
Thanks
Sunil Khatri
>
> Regards,
> Christian.
>
>> + dma_fence_put(f);
>> + amdgpu_userq_put(queue);
>> + queue_id++;
>> + }
>> +}
>> +
>> static void
>> amdgpu_eviction_fence_suspend_worker(struct work_struct *work)
>> {
>> @@ -66,6 +101,9 @@ amdgpu_eviction_fence_suspend_worker(struct work_struct *work)
>> struct dma_fence *ev_fence;
>> bool cookie;
>>
>> + /* Wait for any pending userqueue fence work to finish */
>> + amdgpu_userq_wait_for_signal(uq_mgr);
>> +
>> mutex_lock(&uq_mgr->userq_mutex);
>>
>> /*
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> index c4e92113b557..1e8c08b63f65 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> @@ -1269,27 +1269,9 @@ void amdgpu_userq_reset_work(struct work_struct *work)
>> amdgpu_device_gpu_recover(adev, NULL, &reset_context);
>> }
>>
>> -static void
>> -amdgpu_userq_wait_for_signal(struct amdgpu_userq_mgr *uq_mgr)
>> -{
>> - struct amdgpu_usermode_queue *queue;
>> - unsigned long queue_id;
>> -
>> - xa_for_each(&uq_mgr->userq_xa, queue_id, queue) {
>> - struct dma_fence *f = queue->last_fence;
>> -
>> - if (!f)
>> - continue;
>> -
>> - dma_fence_wait(f, false);
>> - }
>> -}
>> -
>> void
>> amdgpu_userq_evict(struct amdgpu_userq_mgr *uq_mgr)
>> {
>> - /* Wait for any pending userqueue fence work to finish */
>> - amdgpu_userq_wait_for_signal(uq_mgr);
>> amdgpu_userq_evict_all(uq_mgr);
>> }
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-08 8:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-08 8:30 [PATCH V1] drm/amdgpu/userq: unblock signal ioctl from userq_mutex Sunil Khatri
2026-04-08 8:32 ` Christian König
2026-04-08 8:38 ` Khatri, Sunil
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.