Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: <karim.sabar@utexas.edu>
Cc: Thomas Hellstrom <thomas.hellstrom@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
	<intel-xe@lists.freedesktop.org>,
	<dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] drm/xe: Rebind VMAs one at a time when a bulk rebind fails
Date: Wed, 28 Oct 2026 15:43:25 -0700	[thread overview]
Message-ID: <auJ6jZj/P7G+omgH@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <6ae197b3.d47bf0d2.14cb70.fecd@mx.google.com>

On Wed, Oct 28, 2026 at 01:35:29AM -0500, karim.sabar@utexas.edu wrote:
> From: Karim Sabar <karim.sabar@utexas.edu>
> 
> 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

Generally don't use I statements in commit messages, I can adjust your
commit message when I merge this (after our CI comes back).

Thanks for the patch:

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> 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 <matthew.brost@intel.com>
> Cc: stable@vger.kernel.org # v6.11+
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Karim Sabar <karim.sabar@utexas.edu>
> ---
> 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/6ad8027c.7f7fe2b9.10c6e4.4073@mx.google.com
> 
>  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	other threads:[~2026-10-28 22:43 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-21  0:08 [PATCH] drm/xe: Use kvmalloc for the VMA ops page-table update array karim.sabar
2026-10-21 11:43 ` ✗ LGCI.VerificationFailed: failure for " Patchwork
2026-10-21 18:57 ` [PATCH] " Matthew Brost
2026-10-22  2:28   ` karim.sabar
2026-10-22  2:35     ` Matthew Brost
2026-10-28  6:35       ` [PATCH v2] drm/xe: Rebind VMAs one at a time when a bulk rebind fails karim.sabar
2026-10-28 22:43         ` Matthew Brost [this message]
2026-10-28  6:36 ` ✗ LGCI.VerificationFailed: failure for drm/xe: Use kvmalloc for the VMA ops page-table update array (rev2) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=auJ6jZj/P7G+omgH@gsse-cloud1.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=airlied@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=karim.sabar@utexas.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rodrigo.vivi@intel.com \
    --cc=simona@ffwll.ch \
    --cc=thomas.hellstrom@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox