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 <[email protected]> > > > > Cc: Boris Brezillon <[email protected]> > > > > Cc: Danilo Krummrich <[email protected]> > > > > Cc: David Airlie <[email protected]> > > > > Cc: Jonathan Corbet <[email protected]> > > > > Cc: Liviu Dudau <[email protected]> > > > > Cc: Maarten Lankhorst <[email protected]> > > > > Cc: Maxime Ripard <[email protected]> > > > > Cc: Rodrigo Vivi <[email protected]> > > > > Cc: Shuah Khan <[email protected]> > > > > Cc: Simona Vetter <[email protected]> > > > > Cc: Steven Price <[email protected]> > > > > Cc: Thomas Hellström <[email protected]> > > > > Cc: Thomas Zimmermann <[email protected]> > > > > Signed-off-by: Matthew Brost <[email protected]> > > > > 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(-)
