Re: [PATCH 2/2] drm/xe: Remove redundant full pagefault queue check

On Tue, Aug 11, 2026 at 10:09:05PM +0200, Michal Wajdeczko wrote:
> There is no need to check the queue inside the pagefault handler
> as it is immediately checked later by the xe_pagefault_queue_add()
> function and we can just look for the NULL lpf entry. There will
> be also already a full WARN triggered, so no need keep drm_warn().
> 
> Signed-off-by: Michal Wajdeczko <[email protected]>
> Cc: Matthew Brost <[email protected]>

Reviewed-by: Matthew Brost <[email protected]>

> ---
>  drivers/gpu/drm/xe/xe_pagefault.c | 45 +++++++++++++------------------
>  1 file changed, 18 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pagefault.c b/drivers/gpu/drm/xe/xe_pagefault.c
> index cfb65621baec..7e829c61387f 100644
> --- a/drivers/gpu/drm/xe/xe_pagefault.c
> +++ b/drivers/gpu/drm/xe/xe_pagefault.c
> @@ -864,41 +864,32 @@ static int xe_pagefault_work_index(struct xe_device *xe)
>  int xe_pagefault_handler(struct xe_device *xe, struct xe_pagefault *pf)
>  {
>  	struct xe_pagefault_queue *pf_queue = &xe->usm.pf_queue;
> -	bool full;
> +	struct xe_pagefault *lpf;
> +	bool empty;
>  
>  	guard(spinlock_irqsave)(&pf_queue->lock);
> -	full = xe_pagefault_queue_full(pf_queue);
> -	if (!full) {
> -		struct xe_pagefault *lpf;
> -		bool empty = xe_pagefault_queue_empty(pf_queue);
>  
> -		lpf = xe_pagefault_queue_add(pf_queue, pf);
> -		if (lpf) {
> -			lpf->consumer.next = NULL;
> +	empty = xe_pagefault_queue_empty(pf_queue);
> +	lpf = xe_pagefault_queue_add(pf_queue, pf);
> +	if (!lpf)
> +		return -ENOSPC;
>  
> -			if (xe_pagefault_try_chain(pf_queue, lpf)) {
> -				xe_gt_stats_incr(pf->gt,
> -						 XE_GT_STATS_ID_CHAIN_IRQ_PAGEFAULT_COUNT,
> -						 1);
> -				if (empty) {
> -					xe_gt_stats_incr(pf->gt,
> -							 XE_GT_STATS_ID_CHAIN_DRAIN_IRQ_PAGEFAULT_COUNT,
> -							 1);
> -					xe_pagefault_queue_advance(pf_queue);
> -				}
> -			} else {
> -				int work_index = xe_pagefault_work_index(xe);
> -
> -				queue_work(xe->usm.pagefault_wq,
> -					   &xe->usm.pf_workers[work_index].work);
> -			}
> +	lpf->consumer.next = NULL;
> +	if (xe_pagefault_try_chain(pf_queue, lpf)) {
> +		xe_gt_stats_incr(pf->gt, XE_GT_STATS_ID_CHAIN_IRQ_PAGEFAULT_COUNT, 1);
> +		if (empty) {
> +			xe_gt_stats_incr(pf->gt,
> +					 XE_GT_STATS_ID_CHAIN_DRAIN_IRQ_PAGEFAULT_COUNT, 1);
> +			xe_pagefault_queue_advance(pf_queue);
>  		}
>  	} else {
> -		drm_warn(&xe->drm,
> -			 "PageFault Queue full, shouldn't be possible\n");
> +		int work_index = xe_pagefault_work_index(xe);
> +
> +		queue_work(xe->usm.pagefault_wq,
> +			   &xe->usm.pf_workers[work_index].work);
>  	}
>  
> -	return full ? -ENOSPC : 0;
> +	return 0;
>  }
>  
>  /**
> -- 
> 2.47.1
>