Re: [PATCH 03/13] drm/xe: Drop queued page faults when device I/O is blocked

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Aug 27, 2026 at 03:47:51PM +0530, Arvind Yadav wrote:
> Page-fault work may still be queued when PCI error recovery starts or
> the device becomes permanently wedged. Servicing these faults can migrate
> memory or update page tables after device I/O has been blocked.
> 
> Check the device state before and after fault servicing to cover a reset
> racing with the worker. Drop the active fault and its chained faults
> without sending a hardware response, and invalidate the cached fault
> state.
> 
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pagefault.c | 40 +++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pagefault.c b/drivers/gpu/drm/xe/xe_pagefault.c
> index 2e415995f067..f486165f2323 100644
> --- a/drivers/gpu/drm/xe/xe_pagefault.c
> +++ b/drivers/gpu/drm/xe/xe_pagefault.c
> @@ -592,6 +592,38 @@ static void xe_pagefault_save_to_vm(struct xe_device *xe, struct xe_pagefault *p
>  	xe_vm_put(vm);
>  }
>  
> +static bool
> +xe_pagefault_drop_if_blocked(struct xe_pagefault_queue *pf_queue,
> +			     struct xe_pagefault_work *pf_work,
> +			     struct xe_pagefault *pf,
> +			     u64 *cache_start)
> +{
> +	struct xe_pagefault *next;
> +
> +	if (!xe_device_io_blocked(pf_work->xe))
> +		return false;
> +
> +	/*
> +	 * cache_start is private to this worker invocation. pf_work->cache is
> +	 * shared with fault producers and must be invalidated under the queue
> +	 * lock.
> +	 */
> +	xe_pagefault_cache_start_invalidate(*cache_start);
> +
> +	guard(spinlock_irq)(&pf_queue->lock);
> +
> +	xe_pagefault_cache_invalidate(pf_queue, pf_work);
> +
> +	while (pf) {
> +		next = pf->consumer.next;
> +		pf->consumer.next = NULL;
> +		pf->consumer.alloc_state = XE_PAGEFAULT_ALLOC_STATE_FREE;
> +		pf = next;

is there some helper we could re-use? I know we cannot use the pop directly,
because it would deadlock, but perhaps we can unify some code here...

> +	}
> +
> +	return true;
> +}
> +
>  static void xe_pagefault_queue_work(struct work_struct *w)
>  {
>  	struct xe_pagefault_work *pf_work =
> @@ -615,6 +647,10 @@ static void xe_pagefault_queue_work(struct work_struct *w)
>  		int err = 0;
>  		bool invalidated = false;
>  
> +		if (xe_pagefault_drop_if_blocked(pf_queue, pf_work, pf,
> +						 &cache_start))
> +			continue;

do we really need to loop inside the loop or only check and let this main loop goes?

> +
>  		/* Last fault same address, ack immediately */
>  		if (xe_pagefault_match(pf, cache_start, cache_end, cache_asid)) {
>  			xe_gt_stats_incr(gt, XE_GT_STATS_ID_LAST_PAGEFAULT_COUNT, 1);
> @@ -623,6 +659,10 @@ static void xe_pagefault_queue_work(struct work_struct *w)
>  
>  		err = xe_pagefault_service(pf);
>  
> +		if (xe_pagefault_drop_if_blocked(pf_queue, pf_work, pf,
> +						 &cache_start))

do we really need to check after?

> +			continue;
> +
>  		if (err) {

now this err far from where it is collected is pretty bad...

>  			if (!(pf->consumer.access_type & XE_PAGEFAULT_ACCESS_PREFETCH)) {
>  				xe_pagefault_save_to_vm(gt_to_xe(gt), pf);
> -- 
> 2.43.0
>