NAK/Cmnt: [PATCH 1/1] Revert "drm/qxl: simplify qxl_fence_wait"

[John Cabaj]

On 7/15/25 12:12 PM, ghadi.rahme at canonical.com wrote:
> From: Alex Constantino <dreaming.about.electric.sheep at gmail.com>

Need a BugLink on the patch itself as well as the cover letter.

Also, the BugLink from the cover letter suggests both Jammy and Noble 
have a fix, but the subject only indicates Noble.

> 
> This reverts commit 5a838e5d5825c85556011478abde708251cc0776.
> 
> Changes from commit 5a838e5d5825 ("drm/qxl: simplify qxl_fence_wait") would
> result in a '[TTM] Buffer eviction failed' exception whenever it reached a
> timeout.
> Due to a dependency to DMA_FENCE_WARN this also restores some code deleted
> by commit d72277b6c37d ("dma-buf: nuke DMA_FENCE_TRACE macros v2").
> 
> Fixes: 5a838e5d5825 ("drm/qxl: simplify qxl_fence_wait")
> Link: https://lore.kernel.org/regressions/ZTgydqRlK6WX_b29@eldamar.lan/
> Reported-by: Timo Lindfors <timo.lindfors at iki.fi>
> Closes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1054514
> Signed-off-by: Alex Constantino <dreaming.about.electric.sheep at gmail.com>
> Signed-off-by: Maxime Ripard <mripard at kernel.org>
> Link: https://patchwork.freedesktop.org/patch/msgid/20240404181448.1643-2-dreaming.about.electric.sheep@gmail.com

Since this revert was actually upstream, we need the commit SHA included 
(`git cherry-pick -x` option).

> Signed-off-by: Ghadi Elie Rahme <ghadi.rahme at canonical.com>
> ---
>   drivers/gpu/drm/qxl/qxl_release.c | 50 +++++++++++++++++++++++++++----
>   include/linux/dma-fence.h         |  7 +++++
>   2 files changed, 52 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
> index 368d26da0d6a..9febc8b73f09 100644
> --- a/drivers/gpu/drm/qxl/qxl_release.c
> +++ b/drivers/gpu/drm/qxl/qxl_release.c
> @@ -58,16 +58,56 @@ static long qxl_fence_wait(struct dma_fence *fence, bool intr,
>   			   signed long timeout)
>   {
>   	struct qxl_device *qdev;
> +	struct qxl_release *release;
> +	int count = 0, sc = 0;
> +	bool have_drawable_releases;
>   	unsigned long cur, end = jiffies + timeout;
>   
>   	qdev = container_of(fence->lock, struct qxl_device, release_lock);
> +	release = container_of(fence, struct qxl_release, base);
> +	have_drawable_releases = release->type == QXL_RELEASE_DRAWABLE;
>   
> -	if (!wait_event_timeout(qdev->release_event,
> -				(dma_fence_is_signaled(fence) ||
> -				 (qxl_io_notify_oom(qdev), 0)),
> -				timeout))
> -		return 0;
> +retry:
> +	sc++;
> +
> +	if (dma_fence_is_signaled(fence))
> +		goto signaled;
> +
> +	qxl_io_notify_oom(qdev);
> +
> +	for (count = 0; count < 11; count++) {
> +		if (!qxl_queue_garbage_collect(qdev, true))
> +			break;
> +
> +		if (dma_fence_is_signaled(fence))
> +			goto signaled;
> +	}
> +
> +	if (dma_fence_is_signaled(fence))
> +		goto signaled;
> +
> +	if (have_drawable_releases || sc < 4) {
> +		if (sc > 2)
> +			/* back off */
> +			usleep_range(500, 1000);
> +
> +		if (time_after(jiffies, end))
> +			return 0;
> +
> +		if (have_drawable_releases && sc > 300) {
> +			DMA_FENCE_WARN(fence,
> +				       "failed to wait on release %llu after spincount %d\n",
> +				       fence->context & ~0xf0000000, sc);
> +			goto signaled;
> +		}
> +		goto retry;
> +	}
> +	/*
> +	 * yeah, original sync_obj_wait gave up after 3 spins when
> +	 * have_drawable_releases is not set.
> +	 */
>   
> +signaled:
>   	cur = jiffies;
>   	if (time_after(cur, end))
>   		return 0;
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index e06bad467f55..c3f9bb6602ba 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -682,4 +682,11 @@ static inline bool dma_fence_is_container(struct dma_fence *fence)
>   	return dma_fence_is_array(fence) || dma_fence_is_chain(fence);
>   }
>   
> +#define DMA_FENCE_WARN(f, fmt, args...) \
> +	do {								\
> +		struct dma_fence *__ff = (f);				\
> +		pr_warn("f %llu#%llu: " fmt, __ff->context, __ff->seqno,\
> +			 ##args);					\
> +	} while (0)
> +
>   #endif /* __LINUX_DMA_FENCE_H */