On 02-09-2026 11:03, Matthew Brost wrote:
On Tue, Sep 01, 2026 at 10:30:41PM -0700, Matthew Brost wrote:
On Wed, Sep 02, 2026 at 10:19:22AM +0530, Yadav, Arvind wrote:
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...
Why would it deadlock... xe_device_io_blocked is just an couple of
atomics, right?
So why wouldn't this work to squash all faults.
You are right, this does not deadlock as per you suggestion I will move
the check after the QUEUED state check.
diff --git a/drivers/gpu/drm/xe/xe_pagefault.c
b/drivers/gpu/drm/xe/xe_pagefault.c
index 2e415995f067..1c9e5539b90d 100644
--- a/drivers/gpu/drm/xe/xe_pagefault.c
+++ b/drivers/gpu/drm/xe/xe_pagefault.c
@@ -477,6 +477,12 @@ static bool xe_pagefault_queue_pop(struct
xe_pagefault_queue *pf_queue,
lpf = xe_pagefault_queue_tail_fault(pf_queue);
xe_pagefault_queue_advance(pf_queue);
+ if (xe_device_io_blocked(xe)) {
+ lpf->consumer.alloc_state =
+ XE_PAGEFAULT_ALLOC_STATE_FREE;
+ continue;
+ }
+
^^^ This should actually go after the XE_PAGEFAULT_ALLOC_STATE_QUEUED check.
Noted,
Arvind
Matt
if (lpf->consumer.alloc_state !=
XE_PAGEFAULT_ALLOC_STATE_QUEUED)
continue;
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.
But this is still toctou - really everything in patch is as immediately
after either check the state can change. I can't say I'm a fan of any
weak checks in this code...
But just for thought: after this point all we do is CT which should be
down and we'd drop it there before touching the hardware.
Matt
+ 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
