On 01-09-2026 02:13, Rodrigo Vivi wrote:
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...
Noted,
+ }
+
+ 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?
Yes, The inner walk is needed. Chained faults have already been removed
from the queue and linked through consumer.next. The outer pop loop will
not see them.
+
/* 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?
Yes. The device can enter reset or become wedged while
xe_pagefault_service() is running. The first check alone does not cover
that race.
+ continue;
+
if (err) {
now this err far from where it is collected is pretty bad...
Agreed. i will keeps err next to its use.
Thanks,
Arvind
if (!(pf->consumer.access_type &
XE_PAGEFAULT_ACCESS_PREFETCH)) {
xe_pagefault_save_to_vm(gt_to_xe(gt), pf);
--
2.43.0
