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 --- 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
