From: Matthew Brost <matthew.brost@intel.com>
To: <karim.sabar@utexas.edu>
Cc: <thomas.hellstrom@linux.intel.com>, <rodrigo.vivi@intel.com>,
<airlied@gmail.com>, <simona@ffwll.ch>,
<intel-xe@lists.freedesktop.org>,
<dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>,
<stable@vger.kernel.org>
Subject: Re: [PATCH] drm/xe: Use kvmalloc for the VMA ops page-table update array
Date: Wed, 21 Oct 2026 19:35:55 -0700 [thread overview]
Message-ID: <atl2i1vJoU2QwCbG@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <6ad974ba.a8122b97.39a7e6.f867@mx.google.com>
On Wed, Oct 21, 2026 at 09:28:08PM -0500, karim.sabar@utexas.edu wrote:
> Hey Matthew!
>
> Sounds good to me, the fallback of trying to bind the vma one at a time sounds good to me. And it is actually a bit broader than my patch since it covers the -ENOBUFS case as well. It should also be easy to get through drm-xe-fixes and then backported the normal way.
>
> I can write it as a v2 & test it on my Panther Lake device, I have a small repro that does about 20,000 mappings and fails after suspend (every time without the fix). If you'd prefer to write it, I can test your patch as well.
>
Since you have a repro, I'd say go ahead and code it up. This is on my
radar, so I can review and pull it quickly. It should be a relatively
simple function, and something like Claude can help you code it with a
concise query if needed.
One follow-up we'd probably want to add is error injection and an IGT
test to ensure long-term functionality, but we (Intel) can take care of
that part as an AR.
Matt
> Thanks,
> Karim
>
> On Wed, 21 Oct 2026 11:57:48 -0700, Matthew Brost wrote:
> > On Tue, Oct 20, 2026 at 07:08:27PM -0500, karim.sabar@utexas.edu wrote:
> >> From: Karim Sabar <karim.sabar@utexas.edu>
> >>
> >
> > Thanks for the patch. I do see the problem in the old code.
> > Unfortunately, we will not be able to pull this into drm-xe-next or
> > drm-xe-fixes, as 7.3 is already out and 0e33330f1238 is included in our
> > 7.4 PR. As a result, this would need to go directly to stable.
> >
> > There is also a risk that predates the series containing 0e33330f1238.
> > If the bind has to run on the GPU, you can still get -ENOBUFS later in
> > the bind pipeline if the bind is too large to fit into the
> > migration-layer BB. If that occurs, the proper fix may actually be in
> > xe_vm_validate_rebind(). If xe_vm_rebind() fails with -ENOBUFS or
> > -ENOMEM, we could call a new function, such as xe_vm_rebind_iterative(),
> > which would run each VMA requiring a rebind in its own job. This is
> > effectively what Mesa does in userspace when a bind operation returns
> > -ENOBUFS.
> >
> > We could probably still pull that approach into drm-xe-fixes + have
> > naturally backport to stable, and it may ultimately be the better
> > solution.
> >
> > Let me know what you think. I can give this an RB, but I'd like to hold
> > off until we've settled on the solution we want to pursue here.
> >
> > Matt
> >
> >> 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 go 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.
> >> __GFP_NOWARN keeps it out of dmesg.
> >>
> >> Data from a Panther Lake handheld (SteamOS kernel 7.2.7), kretprobes
> >> after resume:
> >> xe_vma_ops_alloc -ENOMEM on 63 of 63 attempts
> >> xe_vm_rebind -ENOMEM on 63 of 63 attempts
> >> BO validate, TTM populate, PT prepare never failed
> >>
> >> The array is only used by the CPU and doesn't need to be contiguous.
> >> This switches it to kvmalloc and kvfree.
> >>
> >> With the same change applied as a module on that kernel, the game
> >> survives every sleep I tried with zero ENOMEM. The biggest array was
> >> 30,753 ops (11.8 MB).
> >>
> >> drm-xe-next already does this through commit 0e33330f1238 ("drm/xe: Add
> >> struct xe_pt_job_ops"), but that's part of a big series that won't go
> >> to stable.
> >>
> >> I tracked this down and tested it on my own device. I used an AI
> >> assistant to help with the debugging, i.e the kprobe tracing and
> >> reading through the xe code. I ran every test, picked the fix and
> >> reviewed the change myself.
> >>
> >> What I couldn't do: this is build-tested on drm-xe-fixes but only
> >> run-tested on the SteamOS 7.2.7 kernel through an equivalent module,
> >> not on a mainline kernel.
> >>
> >> Fixes: 2e524668c440 ("drm/xe: Add xe_vm_pgtable_update_op to xe_vma_ops")
> >> Cc: stable@vger.kernel.org # v6.11+
> >> Assisted-by: Claude:claude-opus-5-5
> >> Signed-off-by: Karim Sabar <karim.sabar@utexas.edu>
> >> ---
> >> Downstream SteamOS report with the same data: https://github.com/ValveSoftware/SteamOS/issues/2962
> >> ---
> >> drivers/gpu/drm/xe/xe_vm.c | 8 ++++----
> >> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> >> index 99e42bd29..5caef443b 100644
> >> --- a/drivers/gpu/drm/xe/xe_vm.c
> >> +++ b/drivers/gpu/drm/xe/xe_vm.c
> >> @@ -703,9 +703,9 @@ static int xe_vma_ops_alloc(struct xe_vma_ops *vops, bool array_of_binds)
> >> continue;
> >>
> >> vops->pt_update_ops[i].ops =
> >> - kmalloc_objs(*vops->pt_update_ops[i].ops,
> >> - vops->pt_update_ops[i].num_ops,
> >> - GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
> >> + kvmalloc_objs(*vops->pt_update_ops[i].ops,
> >> + vops->pt_update_ops[i].num_ops,
> >> + GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
> >> if (!vops->pt_update_ops[i].ops)
> >> return array_of_binds ? -ENOBUFS : -ENOMEM;
> >> }
> >> @@ -742,7 +742,7 @@ static void xe_vma_ops_fini(struct xe_vma_ops *vops)
> >> xe_vma_svm_prefetch_ops_fini(vops);
> >>
> >> for (i = 0; i < XE_MAX_TILES_PER_DEVICE; ++i)
> >> - kfree(vops->pt_update_ops[i].ops);
> >> + kvfree(vops->pt_update_ops[i].ops);
> >> }
> >>
> >> static void xe_vma_ops_incr_pt_update_ops(struct xe_vma_ops *vops, u8 tile_mask, int inc_val)
> >>
> >> ---
> >> base-commit: d2ce8fcd778ee6d92748c92d23a56d4416c29d86
> >> change-id: 20261020-xe-kvmalloc-vma-ops-2637ebf81cfd
> >>
> >> Best regards,
> >> --
> >> Karim Sabar <karim.sabar@utexas.edu>
> >>
prev parent reply other threads:[~2026-10-22 2:36 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-21 0:08 karim.sabar
2026-10-21 18:57 ` Matthew Brost
2026-10-22 2:28 ` karim.sabar
2026-10-22 2:35 ` Matthew Brost [this message]
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=atl2i1vJoU2QwCbG@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=stable@vger.kernel.org \
--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
all inboxes | Powered by JetHome®