Re: [PATCH] drm/nouveau: Simplify nouveau_cli_work

[Tvrtko Ursulin]

On 12/06/2026 18:14, Philipp Stanner wrote:
On Fri, 2026-06-12 at 17:54 +0100, Tvrtko Ursulin wrote:
I don't claim this fixes anything but the code certainly was a bit
roundabout and this at least cleans it up.

The only semantic difference is releasing the fence reference after
the work callback is called. Which kind of make sense anyway.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@igalia.com>
Cc: Philipp Stanner <pha...@kernel.org>
Cc: Lyude Paul <ly...@redhat.com>
Cc: Danilo Krummrich <d...@kernel.org>
Cc: nouv...@lists.freedesktop.org
---
  drivers/gpu/drm/nouveau/nouveau_drm.c | 19 ++-----------------
  1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 42a81166f3a9..7de174537fc7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -153,22 +153,6 @@ nouveau_name(struct drm_device *dev)
                return nouveau_platform_name(to_platform_device(dev->dev));
  }
  
-static inline bool
-nouveau_cli_work_ready(struct dma_fence *fence)
-{
-       unsigned long flags;
-       bool ret = true;
-
-       dma_fence_lock_irqsave(fence, flags);
-       if (!dma_fence_is_signaled_locked(fence))
-               ret = false;
-       dma_fence_unlock_irqrestore(fence, flags);
-
-       if (ret == true)
-               dma_fence_put(fence);
-       return ret;
-}
-
  static void
  nouveau_cli_work(struct work_struct *w)
  {
@@ -176,9 +160,10 @@ nouveau_cli_work(struct work_struct *w)
        struct nouveau_cli_work *work, *wtmp;
        mutex_lock(&cli->lock);
        list_for_each_entry_safe(work, wtmp, &cli->worker, head) {
-               if (!work->fence || nouveau_cli_work_ready(work->fence)) {
+               if (!work->fence || dma_fence_is_signaled(work->fence)) {
                        list_del(&work->head);
                        work->func(work);
+                       dma_fence_put(work->fence);

Dave had addressed a UAF with that patch.

So if this would fix it, then the claim would be that work->func(work)
was the user of that fence, and not some other party elsewhere.

I don't know whether that's the case.

Me neither. I say in the commit I don't claim to fix anything. But it is 
more logical and avoids the silly games with ret and all.

What I missed though is that work->func(work) frees the work.

So patch needs to cache the fence... v2 incoming..

Regards,

Tvrtko



P.

                }
        }
        mutex_unlock(&cli->lock);