On 6/10/26 10:26, Philipp Stanner wrote:
> commit c8a5d5ea3ba6 ("nouveau: fix client work fence deletion race")
> fixed a race. To do so, it replaced the automatically locking
> dma_fence_is_signaled() with manual locks plus
> dma_fence_is_signaled_locked().
>
> For someone browsing through the code, this reads very much like a
> cleanup or rework leftover. Future contributors and / or new maintainers
> not familiar with the history might be tempted to remove that bugfix.
>
> Document the bugfix.
>
> Signed-off-by: Philipp Stanner <[email protected]>
Reviewed-by: Christian König <[email protected]>
> ---
> (I did not test this)
> ---
> drivers/gpu/drm/nouveau/nouveau_drm.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
> b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 42a81166f3a9..519a0c164a72 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -159,6 +159,13 @@ nouveau_cli_work_ready(struct dma_fence *fence)
> unsigned long flags;
> bool ret = true;
>
> + /*
> + * This is not a cleanup / rework leftover, but a bugfix to prevent a
> + * race with someone signalling the fence. The locked
> + * dma_fence_is_signaled() cannot be used. The dma_fence implementation
> + * is not fully synchronized with locks, but also uses atomic bits,
> + * which can cause the dma_fence_put() below to be executed too soon.
> + */
> dma_fence_lock_irqsave(fence, flags);
> if (!dma_fence_is_signaled_locked(fence))
> ret = false;
