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 <[email protected]>
> Cc: Thomas Hellström <[email protected]>
> Cc: Himal Prasad Ghimiray <[email protected]>
> Cc: Rodrigo Vivi <[email protected]>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Arvind Yadav <[email protected]>
> ---
> 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
>
