* [PATCH] drm/nouveau: idle the channel before destroying an engine object via abi16
@ 2026-10-23 14:42 ` Alex
0 siblings, 0 replies; 7+ messages in thread
From: Alex @ 2026-10-23 14:42 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich
Cc: Karol Herbst, nouveau, dri-devel, linux-kernel, xnderv
Cc: dri-devel@lists.freedesktop.org, nouveau@lists.freedesktop.org
nouveau_abi16_ioctl_del() destroys an engine object (ENGOBJ) created by
nouveau_abi16_ioctl_new() without waiting for the owning channel to go
idle. The legacy path, nouveau_abi16_ioctl_gpuobj_free(), does call
nouveau_channel_idle() first ("synchronize with the user channel and
destroy the gpu object"), the new ioctl does not.
Chromium-based applications (Chrome, Yandex Browser, Electron apps)
create and immediately delete probe 3D objects at startup. If the GPU
is busy with other work at that moment (compositor animation while the
app is launched from the GNOME dash, cursor movement), the engine
context is unbound while the channel still references it, and the GPU
faults:
nouveau 0000:01:00.0: fifo: fault 00 [VIRT_READ] at 0000000000000000
engine 00 [GR] client 0f [HUB/CTXCTL] reason 00 [PDE] on channel 5
nouveau 0000:01:00.0: fifo: ... PAGE_NOT_PRESENT ... channel 5 killed!
after which the desktop freezes. Observed on GF114 (GeForce GTX 560 Ti)
with Linux 7.0.x, reproducible almost every time by launching a Chromium
app from the dash; not seen in 10 consecutive launches with this patch.
Remember the channel an engine object was created on, drop the reference
when the channel is destroyed first, and idle the channel before
nvif_object_dtor() like the gpuobj_free path does.
Signed-off-by: Alex <xnderv@yandex.com>
---
drivers/gpu/drm/nouveau/nouveau_abi16.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
@@ -89,6 +89,7 @@
ENGOBJ,
} type;
u64 object;
+ struct nouveau_abi16_chan *chan;
struct nvif_object engobj;
@@ -173,6 +174,13 @@
struct nouveau_abi16_chan *chan)
{
struct nouveau_abi16_ntfy *ntfy, *temp;
+ struct nouveau_abi16_obj *obj;
+
+ /* Forget references to this channel held by engine objects. */
+ list_for_each_entry(obj, &abi16->objects, head) {
+ if (obj->chan == chan)
+ obj->chan = NULL;
+ }
/* Cancel all jobs from the entity's queue. */
if (chan->sched)
@@ -749,8 +757,16 @@
obj = nouveau_abi16_obj_find(abi16, ioctl->object);
if (obj) {
- if (obj->type == ENGOBJ)
+ if (obj->type == ENGOBJ) {
+ /* Synchronize with the user channel before destroying the
+ * engine object, as the legacy GPUOBJ_FREE path does. The
+ * channel may still have work queued that needs the engine
+ * context which is about to be unbound.
+ */
+ if (obj->chan && obj->chan->chan)
+ nouveau_channel_idle(obj->chan->chan);
nvif_object_dtor(&obj->engobj);
+ }
nouveau_abi16_obj_del(obj);
}
@@ -791,6 +807,7 @@
obj = nouveau_abi16_obj_new(abi16, ENGOBJ, args->object);
if (IS_ERR(obj))
return PTR_ERR(obj);
+ obj->chan = chan;
ret = nvif_object_ctor(&chan->chan->user, "abi16EngObj", args->handle, args->oclass,
NULL, 0, &obj->engobj);
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] drm/nouveau: idle the channel before destroying an engine object via abi16
@ 2026-10-23 14:42 ` Alex
0 siblings, 0 replies; 7+ messages in thread
From: Alex @ 2026-10-23 14:42 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich; +Cc: nouveau, dri-devel, linux-kernel, xnderv
Cc: dri-devel@lists.freedesktop.org, nouveau@lists.freedesktop.org
nouveau_abi16_ioctl_del() destroys an engine object (ENGOBJ) created by
nouveau_abi16_ioctl_new() without waiting for the owning channel to go
idle. The legacy path, nouveau_abi16_ioctl_gpuobj_free(), does call
nouveau_channel_idle() first ("synchronize with the user channel and
destroy the gpu object"), the new ioctl does not.
Chromium-based applications (Chrome, Yandex Browser, Electron apps)
create and immediately delete probe 3D objects at startup. If the GPU
is busy with other work at that moment (compositor animation while the
app is launched from the GNOME dash, cursor movement), the engine
context is unbound while the channel still references it, and the GPU
faults:
nouveau 0000:01:00.0: fifo: fault 00 [VIRT_READ] at 0000000000000000
engine 00 [GR] client 0f [HUB/CTXCTL] reason 00 [PDE] on channel 5
nouveau 0000:01:00.0: fifo: ... PAGE_NOT_PRESENT ... channel 5 killed!
after which the desktop freezes. Observed on GF114 (GeForce GTX 560 Ti)
with Linux 7.0.x, reproducible almost every time by launching a Chromium
app from the dash; not seen in 10 consecutive launches with this patch.
Remember the channel an engine object was created on, drop the reference
when the channel is destroyed first, and idle the channel before
nvif_object_dtor() like the gpuobj_free path does.
Signed-off-by: Alex <xnderv@yandex.com>
---
drivers/gpu/drm/nouveau/nouveau_abi16.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
@@ -89,6 +89,7 @@
ENGOBJ,
} type;
u64 object;
+ struct nouveau_abi16_chan *chan;
struct nvif_object engobj;
@@ -173,6 +174,13 @@
struct nouveau_abi16_chan *chan)
{
struct nouveau_abi16_ntfy *ntfy, *temp;
+ struct nouveau_abi16_obj *obj;
+
+ /* Forget references to this channel held by engine objects. */
+ list_for_each_entry(obj, &abi16->objects, head) {
+ if (obj->chan == chan)
+ obj->chan = NULL;
+ }
/* Cancel all jobs from the entity's queue. */
if (chan->sched)
@@ -749,8 +757,16 @@
obj = nouveau_abi16_obj_find(abi16, ioctl->object);
if (obj) {
- if (obj->type == ENGOBJ)
+ if (obj->type == ENGOBJ) {
+ /* Synchronize with the user channel before destroying the
+ * engine object, as the legacy GPUOBJ_FREE path does. The
+ * channel may still have work queued that needs the engine
+ * context which is about to be unbound.
+ */
+ if (obj->chan && obj->chan->chan)
+ nouveau_channel_idle(obj->chan->chan);
nvif_object_dtor(&obj->engobj);
+ }
nouveau_abi16_obj_del(obj);
}
@@ -791,6 +807,7 @@
obj = nouveau_abi16_obj_new(abi16, ENGOBJ, args->object);
if (IS_ERR(obj))
return PTR_ERR(obj);
+ obj->chan = chan;
ret = nvif_object_ctor(&chan->chan->user, "abi16EngObj", args->handle, args->oclass,
NULL, 0, &obj->engobj);
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/nouveau: idle the channel before destroying an engine object via abi16
2026-10-23 14:42 ` Alex
(?)
@ 2026-10-23 15:00 ` sashiko-bot
-1 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-23 15:00 UTC (permalink / raw)
To: Alex; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/nouveau: race between nouveau_abi16_ioctl_del() and chan->sched jobs
--
commit 4bc27237b92dfdc3db9914229dd5c55e2c5a4d5a
Author: Alex <xnderv@yandex.com>
drm/nouveau: idle the channel before destroying an engine object via abi16
This commit records the owning channel on engine objects created via
nouveau_abi16_ioctl_new() and idles that channel in nouveau_abi16_ioctl_del()
before destroying the engine object. When a channel is destroyed first in
nouveau_abi16_chan_fini(), any channel references held by engine objects are
cleared.
> diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c b/drivers/gpu/drm/nouveau/nouveau_abi16.c
> index edf867c1fdcb0..f130a127d8bb2 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
[ ... ]
> @@ -788,8 +796,16 @@ nouveau_abi16_ioctl_del(struct nouveau_abi16 *abi16, struct nvif_ioctl_v0 *ioctl
>
> obj = nouveau_abi16_obj_find(abi16, ioctl->object);
> if (obj) {
> - if (obj->type == ENGOBJ)
> + if (obj->type == ENGOBJ) {
> + /* Synchronize with the user channel before destroying the
> + * engine object, as the legacy GPUOBJ_FREE path does. The
> + * channel may still have work queued that needs the engine
> + * context which is about to be unbound.
> + */
> + if (obj->chan && obj->chan->chan)
> + nouveau_channel_idle(obj->chan->chan);
> nvif_object_dtor(&obj->engobj);
> + }
[Severity: High]
Can calling nouveau_channel_idle(obj->chan->chan) here race with jobs running
on obj->chan->sched when VM_BIND is enabled?
When a client initializes VM_BIND via DRM_IOCTL_NOUVEAU_VM_INIT,
nouveau_abi16_ioctl_channel_alloc() creates a per-channel scheduler in
chan->sched, and DRM_IOCTL_NOUVEAU_EXEC submits jobs to chan->sched->entity.
Those jobs execute nouveau_exec_job_run() asynchronously on drm->sched_wq
without holding cli->mutex:
drivers/gpu/drm/nouveau/nouveau_exec.c:nouveau_exec_job_run() {
...
for (i = 0; i < exec_job->push.count; i++) {
struct drm_nouveau_exec_push *p = &exec_job->push.s[i];
bool no_prefetch = p->flags & DRM_NOUVEAU_EXEC_PUSH_NO_PREFETCH;
nvif_chan_gpfifo_push(&chan->chan, p->va, p->va_len, no_prefetch);
}
nvif_chan_gpfifo_post(&chan->chan);
ret = nouveau_fence_emit(fence);
...
}
Meanwhile, nouveau_channel_idle() emits a fence directly onto the channel via
nouveau_fence_new() -> nouveau_fence_emit():
drivers/gpu/drm/nouveau/nouveau_fence.c:nouveau_fence_emit() {
...
if (priv->uevent)
dma_fence_init(&fence->base, &nouveau_fence_ops_uevent,
&fctx->lock, fctx->context, ++fctx->sequence);
else
dma_fence_init(&fence->base, &nouveau_fence_ops_legacy,
&fctx->lock, fctx->context, ++fctx->sequence);
kref_get(&fctx->fence_ref);
ret = fctx->emit(fence);
...
}
Unlike nouveau_abi16_chan_fini(), which stops chan->sched->entity before
idling the channel, calling nouveau_channel_idle() in
nouveau_abi16_ioctl_del() without synchronizing with obj->chan->sched allows
nouveau_fence_emit() and fctx->emit() to run concurrently with
nouveau_exec_job_run(), modifying fctx->sequence, chan->chan.push, and
chan->chan.gpfifo without synchronization.
In addition, if DRM_NOUVEAU_EXEC jobs are still queued in
obj->chan->sched->entity (for example, waiting on in_sync dependencies), they
will not have been pushed to the hardware ring yet when nouveau_channel_idle()
waits on its fence, so nvif_object_dtor(&obj->engobj) can still unbind the
engine context before those queued jobs run on the GPU.
Should nouveau_abi16_ioctl_del() drain or synchronize with obj->chan->sched
before calling nouveau_channel_idle(obj->chan->chan)?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261023144219.46478-1-xnderv@yandex.com?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/nouveau: idle the channel before destroying an engine object via abi16
2026-10-23 14:42 ` Alex
@ 2026-10-23 15:23 ` Alex
-1 siblings, 0 replies; 7+ messages in thread
From: Alex @ 2026-10-23 15:23 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich
Cc: Karol Herbst, nouveau, dri-devel, linux-kernel, xnderv
On the Sashiko AI review of this patch:
> Can calling nouveau_channel_idle(obj->chan->chan) here race with jobs
> running on obj->chan->sched when VM_BIND is enabled?
Thanks, the race is real.
Legacy PUSHBUF submissions take cli->mutex (nouveau_abi16_get()), so they
are serialized with nouveau_abi16_ioctl_del(), which is why the Chromium
case was fixed by the plain nouveau_channel_idle(). With VM_BIND the
EXEC jobs are pushed from the scheduler workqueue without cli->mutex, so
emitting a fence from the ioctl could indeed race with
nouveau_exec_job_run(), and jobs still queued in the entity would not be
covered by the fence at all.
v2 (sent as a reply to this thread) waits for the channel's scheduler to
drain first -- the same job-list wait nouveau_sched_fini() already does,
factored out as nouveau_sched_wait_idle() -- and then idles the channel.
New EXEC submissions cannot sneak in meanwhile because the EXEC ioctl
also holds cli->mutex.
Alex
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/nouveau: idle the channel before destroying an engine object via abi16
@ 2026-10-23 15:23 ` Alex
0 siblings, 0 replies; 7+ messages in thread
From: Alex @ 2026-10-23 15:23 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich; +Cc: nouveau, dri-devel, linux-kernel, xnderv
On the Sashiko AI review of this patch:
> Can calling nouveau_channel_idle(obj->chan->chan) here race with jobs
> running on obj->chan->sched when VM_BIND is enabled?
Thanks, the race is real.
Legacy PUSHBUF submissions take cli->mutex (nouveau_abi16_get()), so they
are serialized with nouveau_abi16_ioctl_del(), which is why the Chromium
case was fixed by the plain nouveau_channel_idle(). With VM_BIND the
EXEC jobs are pushed from the scheduler workqueue without cli->mutex, so
emitting a fence from the ioctl could indeed race with
nouveau_exec_job_run(), and jobs still queued in the entity would not be
covered by the fence at all.
v2 (sent as a reply to this thread) waits for the channel's scheduler to
drain first -- the same job-list wait nouveau_sched_fini() already does,
factored out as nouveau_sched_wait_idle() -- and then idles the channel.
New EXEC submissions cannot sneak in meanwhile because the EXEC ioctl
also holds cli->mutex.
Alex
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] drm/nouveau: idle the channel before destroying an engine object via abi16
2026-10-23 15:23 ` Alex
@ 2026-10-23 15:23 ` Alex
-1 siblings, 0 replies; 7+ messages in thread
From: Alex @ 2026-10-23 15:23 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich
Cc: Karol Herbst, nouveau, dri-devel, linux-kernel, xnderv
Cc: dri-devel@lists.freedesktop.org, nouveau@lists.freedesktop.org
nouveau_abi16_ioctl_del() destroys an engine object (ENGOBJ) created by
nouveau_abi16_ioctl_new() without waiting for the owning channel to go
idle. The legacy path, nouveau_abi16_ioctl_gpuobj_free(), does call
nouveau_channel_idle() first ("synchronize with the user channel and
destroy the gpu object"), the new ioctl does not.
Chromium-based applications (Chrome, Yandex Browser, Electron apps)
create and immediately delete probe 3D objects at startup. If the GPU
is busy with other work at that moment (compositor animation while the
app is launched from the GNOME dash, cursor movement), the engine
context is unbound while the channel still references it, and the GPU
faults:
nouveau 0000:01:00.0: fifo: fault 00 [VIRT_READ] at 0000000000000000
engine 00 [GR] client 0f [HUB/CTXCTL] reason 00 [PDE] on channel 5
nouveau 0000:01:00.0: fifo: ... PAGE_NOT_PRESENT ... channel 5 killed!
after which the desktop freezes. Observed on GF114 (GeForce GTX 560 Ti)
with Linux 7.0.x, reproducible almost every time by launching a Chromium
app from the dash; not seen in 10 consecutive launches with this patch.
Remember the channel an engine object was created on, drop the reference
when the channel is destroyed first, and idle the channel before
nvif_object_dtor() like the gpuobj_free path does.
Signed-off-by: Alex <xnderv@yandex.com>
---
v2: wait for the channel's scheduler to drain before idling the channel
when VM_BIND is in use. Legacy PUSHBUF submissions are serialized
with the ioctl by cli->mutex, but EXEC jobs are pushed to the channel
asynchronously from the scheduler workqueue, so emitting a fence from
nouveau_abi16_ioctl_del() could race with nouveau_exec_job_run(), and
jobs still queued in the entity would not have been covered by the
fence at all. Reuse the existing job-list wait from
nouveau_sched_fini() through a small helper, nouveau_sched_wait_idle().
(Race pointed out by the Sashiko AI review of v1.)
drivers/gpu/drm/nouveau/nouveau_abi16.c | 27 +++++++++++++++++++++++-
drivers/gpu/drm/nouveau/nouveau_sched.c | 9 +++++++-
drivers/gpu/drm/nouveau/nouveau_sched.h | 1 +
3 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c b/drivers/gpu/drm/nouveau/nouveau_abi16.c
index b776030..d979cd8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
@@ -89,6 +89,7 @@ struct nouveau_abi16_obj {
ENGOBJ,
} type;
u64 object;
+ struct nouveau_abi16_chan *chan;
struct nvif_object engobj;
@@ -173,6 +174,13 @@ nouveau_abi16_chan_fini(struct nouveau_abi16 *abi16,
struct nouveau_abi16_chan *chan)
{
struct nouveau_abi16_ntfy *ntfy, *temp;
+ struct nouveau_abi16_obj *obj;
+
+ /* Forget references to this channel held by engine objects. */
+ list_for_each_entry(obj, &abi16->objects, head) {
+ if (obj->chan == chan)
+ obj->chan = NULL;
+ }
/* Cancel all jobs from the entity's queue. */
if (chan->sched)
@@ -784,8 +792,24 @@ nouveau_abi16_ioctl_del(struct nouveau_abi16 *abi16, struct nvif_ioctl_v0 *ioctl
obj = nouveau_abi16_obj_find(abi16, ioctl->object);
if (obj) {
- if (obj->type == ENGOBJ)
+ if (obj->type == ENGOBJ) {
+ /* Synchronize with the user channel before destroying the
+ * engine object, as the legacy GPUOBJ_FREE path does. The
+ * channel may still have work queued that needs the engine
+ * context which is about to be unbound.
+ *
+ * Legacy PUSHBUF submissions are serialized with this ioctl
+ * by cli->mutex. With VM_BIND, EXEC jobs are pushed to the
+ * channel asynchronously by the scheduler, so wait for every
+ * submitted job to finish before touching the channel.
+ */
+ if (obj->chan && obj->chan->chan) {
+ if (obj->chan->sched)
+ nouveau_sched_wait_idle(obj->chan->sched);
+ nouveau_channel_idle(obj->chan->chan);
+ }
nvif_object_dtor(&obj->engobj);
+ }
nouveau_abi16_obj_del(obj);
}
@@ -826,6 +850,7 @@ nouveau_abi16_ioctl_new(struct nouveau_abi16 *abi16, struct nvif_ioctl_v0 *ioctl
obj = nouveau_abi16_obj_new(abi16, ENGOBJ, args->object);
if (IS_ERR(obj))
return PTR_ERR(obj);
+ obj->chan = chan;
ret = nvif_object_ctor(&chan->chan->user, "abi16EngObj", args->handle, args->oclass,
NULL, 0, &obj->engobj);
diff --git a/drivers/gpu/drm/nouveau/nouveau_sched.c b/drivers/gpu/drm/nouveau/nouveau_sched.c
index b3f02c4..956ca73 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sched.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sched.c
@@ -493,13 +493,20 @@ nouveau_sched_job_list_empty(struct nouveau_sched *sched)
return empty;
}
+/* Wait until every job submitted to the scheduler has finished. */
+void
+nouveau_sched_wait_idle(struct nouveau_sched *sched)
+{
+ wait_event(sched->job.wq, nouveau_sched_job_list_empty(sched));
+}
+
static void
nouveau_sched_fini(struct nouveau_sched *sched)
{
struct drm_gpu_scheduler *drm_sched = &sched->base;
struct drm_sched_entity *entity = &sched->entity;
- wait_event(sched->job.wq, nouveau_sched_job_list_empty(sched));
+ nouveau_sched_wait_idle(sched);
drm_sched_entity_fini(entity);
drm_sched_fini(drm_sched);
diff --git a/drivers/gpu/drm/nouveau/nouveau_sched.h b/drivers/gpu/drm/nouveau/nouveau_sched.h
index 51ce8dc..0562af6 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sched.h
+++ b/drivers/gpu/drm/nouveau/nouveau_sched.h
@@ -115,5 +115,6 @@ struct nouveau_sched {
int nouveau_sched_create(struct nouveau_sched **psched, struct nouveau_drm *drm,
struct workqueue_struct *wq, u32 credit_limit);
void nouveau_sched_destroy(struct nouveau_sched **psched);
+void nouveau_sched_wait_idle(struct nouveau_sched *sched);
#endif
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2] drm/nouveau: idle the channel before destroying an engine object via abi16
@ 2026-10-23 15:23 ` Alex
0 siblings, 0 replies; 7+ messages in thread
From: Alex @ 2026-10-23 15:23 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich; +Cc: nouveau, dri-devel, linux-kernel, xnderv
Cc: dri-devel@lists.freedesktop.org, nouveau@lists.freedesktop.org
nouveau_abi16_ioctl_del() destroys an engine object (ENGOBJ) created by
nouveau_abi16_ioctl_new() without waiting for the owning channel to go
idle. The legacy path, nouveau_abi16_ioctl_gpuobj_free(), does call
nouveau_channel_idle() first ("synchronize with the user channel and
destroy the gpu object"), the new ioctl does not.
Chromium-based applications (Chrome, Yandex Browser, Electron apps)
create and immediately delete probe 3D objects at startup. If the GPU
is busy with other work at that moment (compositor animation while the
app is launched from the GNOME dash, cursor movement), the engine
context is unbound while the channel still references it, and the GPU
faults:
nouveau 0000:01:00.0: fifo: fault 00 [VIRT_READ] at 0000000000000000
engine 00 [GR] client 0f [HUB/CTXCTL] reason 00 [PDE] on channel 5
nouveau 0000:01:00.0: fifo: ... PAGE_NOT_PRESENT ... channel 5 killed!
after which the desktop freezes. Observed on GF114 (GeForce GTX 560 Ti)
with Linux 7.0.x, reproducible almost every time by launching a Chromium
app from the dash; not seen in 10 consecutive launches with this patch.
Remember the channel an engine object was created on, drop the reference
when the channel is destroyed first, and idle the channel before
nvif_object_dtor() like the gpuobj_free path does.
Signed-off-by: Alex <xnderv@yandex.com>
---
v2: wait for the channel's scheduler to drain before idling the channel
when VM_BIND is in use. Legacy PUSHBUF submissions are serialized
with the ioctl by cli->mutex, but EXEC jobs are pushed to the channel
asynchronously from the scheduler workqueue, so emitting a fence from
nouveau_abi16_ioctl_del() could race with nouveau_exec_job_run(), and
jobs still queued in the entity would not have been covered by the
fence at all. Reuse the existing job-list wait from
nouveau_sched_fini() through a small helper, nouveau_sched_wait_idle().
(Race pointed out by the Sashiko AI review of v1.)
drivers/gpu/drm/nouveau/nouveau_abi16.c | 27 +++++++++++++++++++++++-
drivers/gpu/drm/nouveau/nouveau_sched.c | 9 +++++++-
drivers/gpu/drm/nouveau/nouveau_sched.h | 1 +
3 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c b/drivers/gpu/drm/nouveau/nouveau_abi16.c
index b776030..d979cd8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
@@ -89,6 +89,7 @@ struct nouveau_abi16_obj {
ENGOBJ,
} type;
u64 object;
+ struct nouveau_abi16_chan *chan;
struct nvif_object engobj;
@@ -173,6 +174,13 @@ nouveau_abi16_chan_fini(struct nouveau_abi16 *abi16,
struct nouveau_abi16_chan *chan)
{
struct nouveau_abi16_ntfy *ntfy, *temp;
+ struct nouveau_abi16_obj *obj;
+
+ /* Forget references to this channel held by engine objects. */
+ list_for_each_entry(obj, &abi16->objects, head) {
+ if (obj->chan == chan)
+ obj->chan = NULL;
+ }
/* Cancel all jobs from the entity's queue. */
if (chan->sched)
@@ -784,8 +792,24 @@ nouveau_abi16_ioctl_del(struct nouveau_abi16 *abi16, struct nvif_ioctl_v0 *ioctl
obj = nouveau_abi16_obj_find(abi16, ioctl->object);
if (obj) {
- if (obj->type == ENGOBJ)
+ if (obj->type == ENGOBJ) {
+ /* Synchronize with the user channel before destroying the
+ * engine object, as the legacy GPUOBJ_FREE path does. The
+ * channel may still have work queued that needs the engine
+ * context which is about to be unbound.
+ *
+ * Legacy PUSHBUF submissions are serialized with this ioctl
+ * by cli->mutex. With VM_BIND, EXEC jobs are pushed to the
+ * channel asynchronously by the scheduler, so wait for every
+ * submitted job to finish before touching the channel.
+ */
+ if (obj->chan && obj->chan->chan) {
+ if (obj->chan->sched)
+ nouveau_sched_wait_idle(obj->chan->sched);
+ nouveau_channel_idle(obj->chan->chan);
+ }
nvif_object_dtor(&obj->engobj);
+ }
nouveau_abi16_obj_del(obj);
}
@@ -826,6 +850,7 @@ nouveau_abi16_ioctl_new(struct nouveau_abi16 *abi16, struct nvif_ioctl_v0 *ioctl
obj = nouveau_abi16_obj_new(abi16, ENGOBJ, args->object);
if (IS_ERR(obj))
return PTR_ERR(obj);
+ obj->chan = chan;
ret = nvif_object_ctor(&chan->chan->user, "abi16EngObj", args->handle, args->oclass,
NULL, 0, &obj->engobj);
diff --git a/drivers/gpu/drm/nouveau/nouveau_sched.c b/drivers/gpu/drm/nouveau/nouveau_sched.c
index b3f02c4..956ca73 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sched.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sched.c
@@ -493,13 +493,20 @@ nouveau_sched_job_list_empty(struct nouveau_sched *sched)
return empty;
}
+/* Wait until every job submitted to the scheduler has finished. */
+void
+nouveau_sched_wait_idle(struct nouveau_sched *sched)
+{
+ wait_event(sched->job.wq, nouveau_sched_job_list_empty(sched));
+}
+
static void
nouveau_sched_fini(struct nouveau_sched *sched)
{
struct drm_gpu_scheduler *drm_sched = &sched->base;
struct drm_sched_entity *entity = &sched->entity;
- wait_event(sched->job.wq, nouveau_sched_job_list_empty(sched));
+ nouveau_sched_wait_idle(sched);
drm_sched_entity_fini(entity);
drm_sched_fini(drm_sched);
diff --git a/drivers/gpu/drm/nouveau/nouveau_sched.h b/drivers/gpu/drm/nouveau/nouveau_sched.h
index 51ce8dc..0562af6 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sched.h
+++ b/drivers/gpu/drm/nouveau/nouveau_sched.h
@@ -115,5 +115,6 @@ struct nouveau_sched {
int nouveau_sched_create(struct nouveau_sched **psched, struct nouveau_drm *drm,
struct workqueue_struct *wq, u32 credit_limit);
void nouveau_sched_destroy(struct nouveau_sched **psched);
+void nouveau_sched_wait_idle(struct nouveau_sched *sched);
#endif
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-10-24 14:35 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-23 14:42 [PATCH] drm/nouveau: idle the channel before destroying an engine object via abi16 Alex
2026-10-23 14:42 ` Alex
2026-10-23 15:00 ` sashiko-bot
2026-10-23 15:23 ` Alex
2026-10-23 15:23 ` Alex
2026-10-23 15:23 ` [PATCH v2] " Alex
2026-10-23 15:23 ` Alex
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.