On Fri, Aug 14, 2026 at 12:32:58AM -0700, Matthew Brost wrote: > panthor_vm_prepare_mapped_bos_resvs() locks every external object mapped > in the VM and then validates the evicted ones. Validation here means > panthor_vm_bo_validate(), which swaps the BO's pages back in and restores > its VMAs. That is slow, and an external object is one which can be shared > with another process, so the whole of it happens while holding dma-resv > locks other processes may be waiting on. > > Nothing is gained by holding those. A resident object needs no swapping > in; only the evicted ones do. Split the locking into the two passes > gpuvm now understands: the early pass takes just the evicted external > objects and swaps them in, and the late pass takes the ones which were > resident and are therefore normally ready to use as they are. Private > objects are covered by the VM resv, which is held from the start, so > evicted ones are still validated in the early pass. > > The split is only worth it when there is something to validate, so > drm_gpuvm_needs_two_pass() decides, and a submit with nothing evicted > keeps doing exactly what it does today in a single pass. > > Both passes run in the same drm_exec transaction, so nothing is unlocked > in between and the late pass only ever adds locks. They take disjoint > sets of objects, so passing slot_count to both still reserves it exactly > once per object. > > The early pass reads the evicted state without the object's dma-resv, > that being the lock it is trying not to take. The race is benign: an > object evicted right after the early pass skipped it is picked up by the > late pass instead, which is why that pass still validates. > > Validation here allocates pages, which can recurse into panthor's own > shrinker, so it is worth being explicit about what the early pass can > evict. There is no deadlock: drm_gem_lru_scan() acquires the resv with > ww_mutex_trylock() and skips what it cannot get. VM-exclusive BOs share > the VM resv, which is held across both passes, so those are always > skipped. External objects are not held by the early pass, though, so > reclaim can evict one while the early pass validates something else. > > That is handled, and is why the late pass validates rather than only > locking: it picks up anything evicted after the early pass looked at it. > The cost is that the swapin for such a BO happens under the full set of > locks, i.e. it degrades to the current behaviour for that one object. > > Xe avoids this by refusing to evict BOs bound to a VM the current task is > validating (xe_bo_eviction_valuable() and xe_vm_is_validating()). Panthor > has no equivalent guard. Adding one would make the split more effective > under memory pressure, but it is not needed for correctness, so it is left > as a follow up. > > 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
Reviewed-by: Liviu Dudau <[email protected]> Best regards, Liviu > --- > drivers/gpu/drm/panthor/panthor_mmu.c | 53 ++++++++++++++++++++++++++- > 1 file changed, 51 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c > b/drivers/gpu/drm/panthor/panthor_mmu.c > index 9f63a048df61..ef7fac18ade3 100644 > --- a/drivers/gpu/drm/panthor/panthor_mmu.c > +++ b/drivers/gpu/drm/panthor/panthor_mmu.c > @@ -3256,6 +3256,26 @@ int panthor_vm_unmap_range(struct panthor_vm *vm, u64 > va, u64 size) > * need to reserve a slot on all BOs mapped to a VM and update this slot with > * the job fence after its submission. > * > + * When something is evicted the locks are taken in two passes; when nothing > + * is, a single pass is used, as before. The early pass only takes the > external > + * objects which actually need validating, i.e. the evicted ones, and swaps > + * them back in. Private objects are covered by the VM resv, which is held > + * from the start, so they are validated here too. The late pass then takes > + * the external objects the early pass left out, which were resident and so > + * normally need no swapping in; it still validates, since one of them may > + * have been evicted in the meantime. > + * > + * The point is that panthor_vm_bo_validate() swaps pages back in, which is > + * slow, and an external object is one which can be shared with another > + * process. Doing that while holding the resv of a resident shared BO would > + * stall whoever else needs it, for no benefit, since a resident object is > + * ready to use as it is. > + * > + * Both passes run in the same drm_exec transaction: nothing is unlocked in > + * between and the late pass only ever adds locks. The passes take disjoint > + * sets of objects, so reserving @slot_count in each still reserves it > + * exactly once per object. > + * > * Return: 0 on success, a negative error code otherwise. > */ > int panthor_vm_prepare_mapped_bos_resvs(struct drm_exec *exec, struct > panthor_vm *vm, > @@ -3268,11 +3288,40 @@ int panthor_vm_prepare_mapped_bos_resvs(struct > drm_exec *exec, struct panthor_vm > if (ret) > return ret; > > - ret = drm_gpuvm_prepare_objects(&vm->base, exec, slot_count); > + /* > + * With nothing evicted there is no validation to keep the resident > + * objects unlocked for, so do not pay for the second walk. > + */ > + if (!drm_gpuvm_needs_two_pass(&vm->base)) { > + ret = drm_gpuvm_prepare_objects(&vm->base, exec, slot_count); > + if (ret) > + return ret; > + > + return drm_gpuvm_validate(&vm->base, exec); > + } > + > + ret = drm_gpuvm_prepare_objects_pass(&vm->base, exec, slot_count, > + DRM_GPUVM_EXEC_PASS_EARLY); > + if (ret) > + return ret; > + > + ret = drm_gpuvm_validate_pass(&vm->base, exec, > + DRM_GPUVM_EXEC_PASS_EARLY); > if (ret) > return ret; > > - return drm_gpuvm_validate(&vm->base, exec); > + ret = drm_gpuvm_prepare_objects_pass(&vm->base, exec, slot_count, > + DRM_GPUVM_EXEC_PASS_LATE); > + if (ret) > + return ret; > + > + /* > + * Objects the early pass skipped were resident then, but another > + * process may have evicted one since. Now that everything is locked, > + * pick up whatever is left. > + */ > + return drm_gpuvm_validate_pass(&vm->base, exec, > + DRM_GPUVM_EXEC_PASS_LATE); > } > > unsigned long > -- > 2.34.1 > -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯
