Re: [PATCH 0/3] drm/gpuvm: two pass locking for exec

[Matthew Brost <matthew.brost@intel.com>]

On Tue, Aug 18, 2026 at 01:26:13PM +0200, Thomas Hellström wrote:

+ Adding public lists back, these were unintentionally droppped.

> On Fri, 2026-08-14 at 14:46 -0700, Matthew Brost wrote:
> > On Fri, Aug 14, 2026 at 12:34:01PM +0200, Thomas Hellström wrote:
> > > On Fri, 2026-08-14 at 00:32 -0700, Matthew Brost wrote:
> > > > Two processes share a set of buffers, and each has buffers of its
> > > > own
> > > > which the other never sees. Say process A has mapped
> > > > 
> > > >   S        the shared buffers, also mapped by B
> > > >   P        buffers private to A
> > > > 
> > > > and B has mapped S plus private buffers of its own. The overlap
> > > > is
> > > > exactly S, and the work each process wants to do on its own
> > > > buffers
> > > > is
> > > > independent of the other.
> > > > 
> > > > A submits. Its exec locks the dma-resv of everything it has
> > > > mapped, S
> > > > and P both, then finds something in P has been evicted and
> > > > migrates
> > > > it
> > > > back in. B submits, and blocks on S for as long as that migration
> > > > takes,
> > > > even though the migration is of a buffer belonging to A which B
> > > > has
> > > > never seen.
> > > > 
> > > > So the stall does not come from the overlapping set. The buffers
> > > > in S
> > > > are resident, and neither exec has anything to do to them beyond
> > > > attaching a fence. They are held only because an exec locks
> > > > everything
> > > > it has mapped in one go, and they stay held until the slowest
> > > > unrelated
> > > > thing in that transaction is done.
> > > > 
> > > > Which buffers get evicted is a separate matter, and one which
> > > > already
> > > > has answers: eviction heuristics which leave shared buffers
> > > > alone, or
> > > > one process' allocations outranking another's. This is what is
> > > > left
> > > > once
> > > > those work.
> > > > 
> > > > Where this tends to show up is compositors and presentation,
> > > > which is
> > > > also where userspace has worked hardest to avoid it. Wayland
> > > > explicit
> > > > sync exists so that a compositor is not latched onto its clients'
> > > > rendering, waiting on fences it never asked for. The locking
> > > > above
> > > > reintroduces that coupling anyway, in the kernel, and does it
> > > > under
> > > > memory pressure, which is where a missed frame is least welcome
> > > > and
> > > > the
> > > > cause is hardest to see.
> > > > 
> > > > The fix is to stop coupling "lock the VM" to "validate it".
> > > > Instead
> > > > of
> > > > locking everything and then validating, lock the private buffers
> > > > and
> > > > the
> > > > evicted external ones, validate those, and only then lock the
> > > > rest,
> > > > all
> > > > within the same drm_exec transaction.
> > > 
> > > This sounds like the tradeoff becomes "WW transaction rollbacks
> > > potentially become substantially more expensive": If we validate
> > > before
> > > the full locking transaction completes, the validation work *might*
> > > be
> > > in vain.
> > > 
> > 
> > Yes, indeed, if a WW transaction rolls back and unlocks everything,
> > it is
> > possible that validation from the first pass becomes immediately
> > undone
> > due to memory pressure. It is probably an acceptable tradeoff if we
> > really want to prioritize presentation at all costs.
> 
> Yes, I agree. Also worth noting that when we hit sleeping WW locks
> during eviction we already have the same problem: The whole transaction
> may roll back.
> 

Ah yes, if we get -ENOMEM we'd rollback or if we get TTM eviction to
part of the WW transaction, we'd also could rollback on lock contention
alone. 

> >  We could also use
> > an Xe-side heuristic to always perform a single pass when running at
> > a
> > privileged level in the exec IOCTL.
> >  
> > This would help with SurfaceFlinger compositors, which I know run at
> > the
> > highest privilege level. I'm unsure whether Wayland does this as
> > well,
> > though; I'd have to double-check. But also I think the ultimate goal
> > is
> > never have anything evicted in a compositor VM by ultizing priorities
> > or perhaps a pinning uAPI once we get cgroups. If we get here, then
> > single pass vs two for compositor is a moot point as single pass
> > always
> > taken if nothing is evicted.
> > 
> > I also noticed another potential issue in Xe. xe_vm_is_validating()
> > only elides eviction for the matching task, leaving a hole for kswapd
> > or
> > a foreign process. We may want to consider also eliding eviction from
> > kswapd when vm->validation.validating != NULL and the current context
> > is
> > kswapd, or perhaps simply using a blanket vm->validation.validating
> > !=
> > NULL check.
> 
> IIRC xe_vm_is_validating() is actually task state and an ugly
> workaround I introduced for avoiding passing the drm_exec down the full

I thought it was an ugly workaround so VM bind on particular BO wouldn't
evict any other BO bound in the VM. 'git format-patch -1 9d5558649f68e'.
I think happens to also do what we want here but perhaps we shouldn't
extend this further.

> call chain in the VM code (and the TTM code as well for that matter). I
> don't think we should extend its usage to foreign / peer processes.
> 
> Doesn't kswapd skip on the shrinker bo trylock? So that as soon as a VM
> has locked the vm resv, all its local bos are protected from shrinking?
> 

Yes, the shrinker does look to be trylock so we are good there. 

> >  
> > It may also be worth widening the xe_vm_is_validating() guard and
> > acquiring it immediately after obtaining the VM's private dma-resv
> > lock.
> > We would need some additional support in gpuvm for this, though.
> > 
> > > Accordingly, I think a follow-up to this might be to consider
> > > switching
> > > the dma-resv WW locks over from the Wait-Die algorithm to Wound-
> > > Wait
> > > which is considerably less prone to rollbacks.
> > 
> > I believe you are the expert here, and while I have only done a
> > little                                                               
> >                                                  ┃
> > research, I think this is a good suggestion. The numbers in
> > 08295b3b5bee                                                         
> >                                                      ┃
> > seem to support it, and now that this series moves expensive work
> > to                                                                   
> >                                                ┃
> > earlier parts of the WW transaction, before all locks are
> > fully                                                                
> >                                                        ┃
> > acquired, fewer rollbacks would hopefully prevent that work from
> > being                                                                
> >                                                 ┃
> > redone.  
> >  
> > Is my understanding correct?
> 
> Yes. If a lot of work is needed in a transaction for each locked
> object, then Wound-Wait tends to be the algorithm of choice rather than
> Wait-Die which works best when all locks are taken upfront.
> 

So I think this should discussed in a standalone follow up as this is
global choice for dma-resv.

Matt

> Thanks,
> Thomas
> 
> 
> > 
> > Matt
> > 
> > > 
> > > Thanks,
> > > Thomas
> > > 
> > > 
> > > 
> > > > 
> > > > Patch 1 lets a driver split the locking of an exec that way,
> > > > patches
> > > > 2
> > > > and 3 use it in Xe and Panthor, whose panthor_vm_bo_validate()
> > > > swaps
> > > > pages back in under those same shared locks. It is opt-in, and
> > > > drivers
> > > > which do not ask for it are unaffected.
> > > > 
> > > > Matt
> > > > 
> > > > Cc: Alice Ryhl <aliceryhl@google.com>
> > > > Cc: Boris Brezillon <boris.brezillon@collabora.com>
> > > > Cc: Danilo Krummrich <dakr@kernel.org>
> > > > Cc: David Airlie <airlied@gmail.com>
> > > > Cc: Jonathan Corbet <corbet@lwn.net>
> > > > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > Cc: Maxime Ripard <mripard@kernel.org>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Cc: Shuah Khan <skhan@linuxfoundation.org>
> > > > Cc: Simona Vetter <simona@ffwll.ch>
> > > > Cc: Steven Price <steven.price@arm.com>
> > > > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > > > Assisted-by: GitHub_Copilot:claude-opus-5
> > > > 
> > > > Matthew Brost (3):
> > > >   drm/gpuvm: allow locking external objects in two passes
> > > >   drm/xe: lock the resident BOs of an exec last
> > > >   drm/panthor: lock the resident BOs of a submit last
> > > > 
> > > >  Documentation/gpu/drm-mm.rst          |   6 +
> > > >  drivers/gpu/drm/drm_gpuvm.c           | 480
> > > > +++++++++++++++++++++++++-
> > > >  drivers/gpu/drm/panthor/panthor_mmu.c |  53 ++-
> > > >  drivers/gpu/drm/xe/xe_exec.c          |  23 +-
> > > >  drivers/gpu/drm/xe/xe_vm.c            |  43 ++-
> > > >  drivers/gpu/drm/xe/xe_vm.h            |   3 +-
> > > >  include/drm/drm_gpuvm.h               | 133 ++++++-
> > > >  7 files changed, 709 insertions(+), 32 deletions(-)