From: Marek Czernohous <[email protected]> This is v3 of the nv04 FIFO series, cut down to the four patches that are ready. v2 is here:
https://lore.kernel.org/nouveau/[email protected]/ The Sashiko review bot flagged three issues on that posting. It was right on all three. Two are fixed by the new patches at the head of this series; the third was in the Tesla recovery patch, which I have dropped from this posting. More on that below, because the reason is not the bot's finding. 1/4 The channel-kill subscription outlives the fence context in nouveau_channel_del(): the context is freed first and the event is only dropped later, among the nvif object teardown. A kill delivered in that window walks a freed fence context. 2/4 The subscription is armed before the fence context exists. nouveau_channel_init() subscribes right after mapping userd and creates the context at the end of the same function, and the backends publish chan->fence from kzalloc() before nouveau_fence_context_new() initialises its lock and pending list. The NULL check in nouveau_channel_kill() does not cover that: the pointer is non-NULL and unusable. Both are pre-existing. Both are unreachable below Fermi today, and on Fermi and newer the windows are narrow, but they are real, so 1/4 and 2/4 carry Fixes: and Cc: stable and stand on their own. I should have carried them from the start. The v2 cover letter even described the teardown ordering and dismissed it as "most of that window is harmless". That judgement was wrong, and I appreciate the catch. 3/4 and 4/4 are unchanged in code apart from the rebase and one joined format string. Both commit messages have been corrected: 3/4 no longer implies a chip gate it does not have and no longer claims a before and after I cannot show, and 4/4 no longer says the fences time out, because they cannot. 3/4 downgrades one specific CACHE_ERROR to debug level. Mesa's NV50 bind probe writes a known method and data pattern that the hardware rejects, repeatedly over the life of a session, and nouveau has been reporting it at error level ever since. It changes no behaviour beyond the log level and is unrelated to the two fixes above; it has simply travelled with them since v2. 4/4 lowers the channel-kill subscription from FERMI_CHANNEL_GPFIFO to NV50_CHANNEL_GPFIFO. On its own it changes no observable behaviour, because nothing kills a channel on Tesla today: the only nvkm_chan_error() callers are the Fermi and newer recovery paths. It is worth having anyway. Leaving the ERRORED event delivered into an empty notifier list is a latent dma-fence contract violation, and it is exactly the trap the recovery work would fall into. Removing it before anything can fall in is the right order. What is not in this posting, and why v2 carried a third patch, the recovery path for Tesla cache_error and dma_pusher faults, and the five-patch v3 I had prepared carried it as 5/5. It is not in this posting, and the bot's third finding is not the reason: that one was a real bug of mine and is fixed. I ran an adversarial review over the whole series before sending, and it found two defects in that patch that the bot had not flagged: - nv04_fifo_recover() has no chip-family guard, but nv04_fifo_intr is .intr for nv04, nv10, nv17, nv40, nv50, g84 and g98. On NV04 to NV40 the escalation would therefore fire while 4/4 deliberately installs no subscriber, producing exactly the dead-letter hang that 4/4 exists to prevent. The series would have contradicted its own reasoning, in the same diff. - The recovery path pulls the drm_device out of drvdata on every fault, while nouveau never clears drvdata and the only cancel_work_sync() for the wedge work runs in nvkm_fifo_dtor(), that is after nouveau_drm_device_del() has already kfree()d the nouveau_drm. A queued work reads freed memory. The first is a one-line guard. The second wants the wedge policy moved into the DRM layer rather than patched where it is, which also removes an nvkm to DRM layering violation I should not have introduced. That is a rework, not a fixup, so it goes into its own posting once it has been rebuilt and soaked. Sending it now would only waste reviewer time. Testing Reference hardware: Apple Mac mini Late 2009, MCP79 / GeForce 9400M (NVAC), Core 2 Duo, Wayland (labwc). Build. Unlike v2, this series is built against the stated base commit: the nouveau module compiles and links with zero warnings and zero errors, with the new format strings and the lowered class gate in place. checkpatch.pl --strict is clean on all four patches. Soak. Code equivalent to 1/4, 3/4 and 4/4 has been running on that machine since 2026-07-25, and equivalent to 2/4 since 2026-08-06. The first spans the kernel bumps 7.1.5, 7.1.6, 7.1.7 and 7.1.8, the second only 7.1.7 and 7.1.8. Since every channel now subscribes to the kill event, the interesting long-run question is channel churn, so the soak is ordinary daily driving with a browser and GL applications rather than a synthetic loop. No regression has shown up in that time. What the soak does not show, stated plainly: that tree carries local patches this series does not, including a cap on the plane-fence wait in the nonblocking commit tail. So the soak says these changes do not misbehave in daily use. It is not an independent demonstration of the failure modes described above. 1/4 and 2/4 are ordering fixes for windows I have not managed to hit deliberately on this hardware; the reasoning is from the source, and the bot's reading of it agrees. I would rather say that plainly than claim a reproduction I do not have. 3/4 filters a benign CACHE_ERROR that Mesa's NV50 bind probe triggers. The logs behind it are from a second, independent MCP79/MCP7A machine, not mine: 99 occurrences across three logs and two kernel versions, under Xorg and Wayland, with kwin and plasmashell named as the faulting clients. They were shared with me by the tester who reported the original NVAC issue. I have not named them and have added no Reported-by or Tested-by: we have not been in touch since then, so I have nobody's consent to point at. On my own machine the filter went in before the persistent logging did, so I cannot show a clean before and after from here. AI assistance Lyude asked on the v1 thread whether these patches were written by a human and pointed at Documentation/process/coding-assistants.rst. The answer, repeated here for the archive: this work is AI assisted. I use Claude (claude-opus-4-7 for v1, claude-opus-5 since) as a coding and analysis assistant. Every patch carries an Assisted-by trailer accordingly, and no Signed-off-by is added by the tool. Nature of the assistance, so you can calibrate your review: the assistant did most of the code archaeology and drafting. I described symptoms, asked for the mechanism to be traced in the source rather than guessed, and asked for each claim to be backed by a file and line. The measurements quoted in earlier postings are from the machine, not model output. The assistant also reviewed its own earlier work adversarially, which is how the two defects in the dropped patch were found, and how several unsupported claims in the v2 cover letter were caught before this posting. I reviewed the result, I understand the code, and I take responsibility for it. Changes since v2 - new 1/4: unsubscribe the channel-kill event before the fence context - new 2/4: subscribe to the channel-kill event after the fence context - both found by the Sashiko review bot on the v2 posting, confirmed against the source before acting on them, and now carrying Fixes: and Cc: stable - dropped the Tesla recovery patch, see above - 3/4 and 4/4 unchanged in code apart from the rebase and one joined format string, both commit messages corrected for accuracy - the series is now built against the base commit Marek Czernohous (4): drm/nouveau: unsubscribe the channel-kill event before the fence context drm/nouveau: subscribe to the channel-kill event after the fence context drm/nouveau/fifo/nv04: filter benign CACHE_ERROR from Mesa NV50 bind probe drm/nouveau: subscribe to channel-kill events on NV50 and newer drivers/gpu/drm/nouveau/nouveau_chan.c | 64 ++++++++++++------- .../gpu/drm/nouveau/nvkm/engine/fifo/nv04.c | 26 ++++++-- 2 files changed, 61 insertions(+), 29 deletions(-) base-commit: c21bb4193868a8de71fc4693fa741e195fdf5d86 -- 2.54.0
