From: Karim Sabar <[email protected]>

Big games die after every s2idle resume on flat-CCS iGPUs. The exec
ioctl keeps returning -ENOMEM even with 11-16 GB of RAM free.

On suspend xe_bo_evict_all_user() evicts every user BO in XE_PL_TT on
these parts. On resume the first exec rebinds every VMA in one job and
xe_vma_ops_alloc() kmallocs one 384-byte op per VMA as a single array.
Final Fantasy VII Rebirth has about 33,500 VMAs which means a 12.9 MB
array. It's over the 4 MB KMALLOC_MAX_SIZE and can never succeed. A
bind that big can also fail later with -ENOBUFS when it doesn't fit in
the migrate layer's vm_update_sa.

When xe_vm_rebind() fails with -ENOMEM or -ENOBUFS, fall back to
xe_vm_rebind_iterative(). It rebinds each VMA on the rebind list in its
own job, the same thing Mesa does from userspace when a bind returns
-ENOBUFS. Each VMA comes off the list once its job is queued which
means a failure part way through leaves the rest for the next attempt.

I tested this on a OneXPlayer 3 (Panther Lake) running SteamOS's 7.2.7
kernel. A small Vulkan program fills a VM with 20,000 or 35,000 64 KiB
buffers, suspends with rtcwake and submits again after resume. Without
the fix the first exec after resume returns -ENOMEM over and over and
Mesa reports the device as lost. With the same fallback loaded into
xe_vm_validate_rebind() through a test module, the bulk rebind failed
with -ENOMEM on all four resumes and the fallback rebound 20,024 VMAs
three times and 35,027 VMAs once with no errors. The program kept
submitting after every resume and the kernel log stayed clean. The
patch itself is build tested on drm-xe-fixes with W=1.

I tracked this down and tested it on my own device. I used an AI
assistant to help with the debugging and with writing the fallback. I
ran every test and reviewed the change myself.

Fixes: 2e524668c440 ("drm/xe: Add xe_vm_pgtable_update_op to xe_vma_ops")
Suggested-by: Matthew Brost <[email protected]>
Cc: [email protected] # v6.11+
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Karim Sabar <[email protected]>
---
Hey Matt,

Sorry for the slow turnaround on this one. I said I'd write it and then
it sat for a week. Here's the iterative rebind you suggested. When
xe_vm_rebind() fails with -ENOMEM or -ENOBUFS, xe_vm_validate_rebind()
now falls back to rebinding one VMA per job.

I only hooked it into xe_vm_validate_rebind() like you said. The
preempt rebind worker for LR VMs still calls xe_vm_rebind() directly. I
left that alone since it's not the path I can reproduce but I'm happy to
add it there too if you want it covered.

Thanks for offering to take the error injection and IGT test on your
side. Also I'm not on the Intel CI allowlist which means CI won't run on
this one until someone hits retest. Would you mind kicking it off?

Changes in v2:
- Fall back to per-VMA rebinds in xe_vm_validate_rebind() instead of
  switching the op array to kvmalloc (Matt)
- Link to v1: 
https://lore.kernel.org/r/[email protected]

 drivers/gpu/drm/xe/xe_vm.c | 49 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 99e42bd29..3cbb305b8 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -370,6 +370,8 @@ static int xe_gpuvm_validate(struct drm_gpuvm_bo *vm_bo, 
struct drm_exec *exec)
        return 0;
 }
 
+static int xe_vm_rebind_iterative(struct xe_vm *vm);
+
 /**
  * xe_vm_validate_rebind() - Validate buffer objects and rebind vmas
  * @vm: The vm for which we are rebinding.
@@ -397,6 +399,8 @@ int xe_vm_validate_rebind(struct xe_vm *vm, struct drm_exec 
*exec,
                        return ret;
 
                ret = xe_vm_rebind(vm, false);
+               if (ret == -ENOBUFS || ret == -ENOMEM)
+                       ret = xe_vm_rebind_iterative(vm);
                if (ret)
                        return ret;
        } while (!list_empty(&vm->gpuvm.evict.list));
@@ -856,6 +860,51 @@ int xe_vm_rebind(struct xe_vm *vm, bool rebind_worker)
        return err;
 }
 
+static int xe_vm_rebind_iterative(struct xe_vm *vm)
+{
+       struct dma_fence *fence;
+       struct xe_vma *vma, *next;
+       struct xe_vma_ops vops;
+       struct xe_vma_op *op, *next_op;
+       int err = 0, i;
+
+       lockdep_assert_held(&vm->lock);
+       xe_vm_assert_held(vm);
+
+       list_for_each_entry_safe(vma, next, &vm->rebind_list,
+                                combined_links.rebind) {
+               xe_vma_ops_init(&vops, vm, NULL, NULL, 0);
+               for (i = 0; i < XE_MAX_TILES_PER_DEVICE; ++i)
+                       vops.pt_update_ops[i].wait_vm_bookkeep = true;
+
+               trace_xe_vma_rebind_exec(vma);
+
+               err = xe_vm_ops_add_rebind(&vops, vma, vma->tile_present);
+               if (!err)
+                       err = xe_vma_ops_alloc(&vops, false);
+               if (!err) {
+                       fence = ops_execute(vm, &vops);
+                       if (IS_ERR(fence)) {
+                               err = PTR_ERR(fence);
+                       } else {
+                               dma_fence_put(fence);
+                               list_del_init(&vma->combined_links.rebind);
+                       }
+               }
+
+               list_for_each_entry_safe(op, next_op, &vops.list, link) {
+                       list_del(&op->link);
+                       kfree(op);
+               }
+               xe_vma_ops_fini(&vops);
+
+               if (err)
+                       break;
+       }
+
+       return err;
+}
+
 struct dma_fence *xe_vma_rebind(struct xe_vm *vm, struct xe_vma *vma, u8 
tile_mask)
 {
        struct dma_fence *fence = NULL;

base-commit: d2ce8fcd778ee6d92748c92d23a56d4416c29d86

Reply via email to