From: Marek Czernohous <[email protected]>
nouveau_fence_context_del() cancels the uevent work first and only tears
the event down afterwards:
cancel_work_sync(&fctx->uevent_work);
nouveau_fence_context_kill(fctx, 0);
nvif_event_dtor(&fctx->event);
Between the cancel and the dtor the event is still armed, and
nouveau_fence_wait_uevent_handler() queues the work unconditionally:
schedule_work(&fctx->uevent_work);
return NVIF_EVENT_KEEP;
So a non-stall interrupt arriving in that window re-arms the work that
was just cancelled. The callers free the context immediately afterwards,
for example nv84_fence_context_del():
nouveau_fence_context_del(&fctx->base);
chan->fence = NULL;
nouveau_fence_context_free(&fctx->base);
nouveau_fence_uevent_work() then runs against freed memory, taking
fctx->lock and walking fctx->pending.
Only chips from G84 on can reach this at all: nouveau_fence_context_new()
returns before nvif_event_ctor() when priv->uevent is clear, and
nv84_fence_create() is the only place that sets it.
nv84_fence_context_del() is the context_del for all of those, because
nvc0_fence_create() and gv100_fence_create() build on nv84_fence_create()
and override only context_new.
Drop the event first, so no further work can be queued, and only then
drain what is already queued.
nouveau_fence_context_kill() keeps its place after the drain. It can
still touch the event: nouveau_fence_signal() returns true when a fence
that had enable_signaling() called on it is signalled and
fctx->notify_ref drops to zero, and the loop then calls
nvif_event_block() once. That call runs in the same thread just after
the dtor, where nvif_event_constructed() is false and it is a no-op.
Blocking an event that no longer exists would be pointless anyway.
The reordering does open one window, so it is worth saying what closes
it. A fence holder that reaches nouveau_fence_enable_signaling()
between the dtor and the kill gets a silent no-op from
nvif_event_allow(), so that fence will not be woken by a non-stall
interrupt any more. It does not have to be: the kill runs immediately
afterwards, signals every fence on fctx->pending under fctx->lock and
sets fctx->killed, after which nouveau_fence_emit() refuses further
work with -ENODEV.
Reported-by: sashiko-bot <[email protected]>
Link:
https://sashiko.dev/#/patchset/[email protected]?part=1
Fixes: 39126abc5e20 ("nouveau: offload fence uevents work to workqueue")
Cc: [email protected]
Assisted-by: Claude:claude-opus-5
Signed-off-by: Marek Czernohous <[email protected]>
---
drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
b/drivers/gpu/drm/nouveau/nouveau_fence.c
index edbe9e08ba0f..4a3698dc2cd1 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
@@ -96,9 +96,9 @@ nouveau_fence_context_kill(struct nouveau_fence_chan *fctx,
int error)
void
nouveau_fence_context_del(struct nouveau_fence_chan *fctx)
{
+ nvif_event_dtor(&fctx->event);
cancel_work_sync(&fctx->uevent_work);
nouveau_fence_context_kill(fctx, 0);
- nvif_event_dtor(&fctx->event);
fctx->dead = 1;
/*
--
2.54.0
