A VM_BIND submit locks the resv of every BO mapped in the VM, then validates the evicted ones, which means getting their pages and mapping them again. That is slow, and external objects can be shared with other processes, so all of it happens while holding resv locks other processes may be waiting on, for BOs which needed no work at all.
Use the two pass locking gpuvm now provides. The early pass locks only the evicted external objects and validates them, along with the evicted private ones, which the VM resv held from the start covers. The late pass locks the external objects which were resident, and still validates in case one of them was evicted meanwhile. When nothing is evicted, drm_gpuvm_exec_pass_needs_split() says so and the submit keeps using a single pass. Both passes run in the same drm_exec transaction, nothing is unlocked in between, and they take disjoint sets of objects, so reserving one fence slot in each still reserves it exactly once per object. This moves validation from after drm_sched_job_arm() and fence attachment into the locking loop, ahead of everything else, which is also where a failure is easiest to unwind. The order does not matter to the shrinker: it skips any BO mapped in a VM whose resv it cannot trylock, and the submit holds the VM resv throughout, so a BO mapped in this VM cannot be evicted while it is locked, whether or not a fence is attached to it yet. The same means the early pass can never evict a BO the late pass is about to lock, the property Xe gets from xe_vm_set_validating(). Cc: Abhinav Kumar <[email protected]> Cc: Alice Ryhl <[email protected]> Cc: Anna Maniscalco <[email protected]> Cc: Antonino Maniscalco <[email protected]> Cc: Boris Brezillon <[email protected]> Cc: Danilo Krummrich <[email protected]> Cc: David Airlie <[email protected]> Cc: Dmitry Baryshkov <[email protected]> Cc: Jessica Zhang <[email protected]> Cc: Jonathan Corbet <[email protected]> Cc: Liviu Dudau <[email protected]> Cc: Lyude Paul <[email protected]> Cc: Maarten Lankhorst <[email protected]> Cc: Marijn Suijten <[email protected]> Cc: Maxime Ripard <[email protected]> Cc: Randy Dunlap <[email protected]> Cc: Rob Clark <[email protected]> Cc: Rodrigo Vivi <[email protected]> Cc: Sean Paul <[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: LLM --- v3: - Follow the drm_gpuvm_exec_pass_ function renames (Danilo) --- drivers/gpu/drm/msm/msm_gem_submit.c | 84 ++++++++++++++++++++++------ 1 file changed, 66 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index 1215b388cb40..4e454345567a 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -266,6 +266,71 @@ static int submit_lookup_cmds(struct msm_gem_submit *submit, return ret; } +/* + * Lock and validate every BO mapped in a VM_BIND VM. Unlike the legacy path, + * where submit_pin_objects() only validates the BOs userspace attached to the + * submit, userspace does not tell us which BOs a VM_BIND submit uses, so the + * entire VM has to be validated. + * + * When something is evicted, the locks are taken in two passes. The early + * pass locks only the external objects which need validating, i.e. the + * evicted ones, and validates them along with the evicted private objects, + * which the VM resv held from the start already covers. The late pass then + * locks the external objects which were resident. Validation means getting + * pages and mapping them, which is slow, and an external object can be shared + * with another process, so there is no point in stalling that process on the + * resv of a resident BO for the duration of it. The late pass still + * validates, in case one of those BOs got evicted meanwhile. + * + * Both passes run in the same drm_exec transaction, nothing is unlocked in + * between, and they take disjoint sets of objects, so reserving one fence + * slot in each reserves it exactly once per object. + * + * The shrinker cannot evict a BO the early pass is about to validate, nor one + * it has validated already: it only evicts a BO after trylocking the resv of + * every VM the BO is mapped in, and the VM resv is held throughout. + */ +static int submit_prepare_vm_objects(struct msm_gem_submit *submit) +{ + struct drm_gpuvm *vm = submit->vm; + struct drm_exec *exec = &submit->exec; + int ret; + + ret = drm_gpuvm_prepare_vm(vm, exec, 1); + if (ret) + return ret; + + /* + * 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_exec_pass_needs_split(vm)) { + ret = drm_gpuvm_prepare_objects(vm, exec, 1); + if (ret) + return ret; + + return drm_gpuvm_validate(vm, exec); + } + + ret = drm_gpuvm_exec_pass_prepare_objects(vm, exec, 1, + DRM_GPUVM_EXEC_PASS_EARLY); + if (ret) + return ret; + + ret = drm_gpuvm_exec_pass_validate(vm, exec, + DRM_GPUVM_EXEC_PASS_EARLY); + if (ret) + return ret; + + ret = drm_gpuvm_exec_pass_prepare_objects(vm, exec, 1, + DRM_GPUVM_EXEC_PASS_LATE); + if (ret) + return ret; + + return drm_gpuvm_exec_pass_validate(vm, exec, + DRM_GPUVM_EXEC_PASS_LATE); +} + static int submit_lock_objects_vmbind(struct msm_gem_submit *submit) { unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT | DRM_EXEC_IGNORE_DUPLICATES; @@ -276,12 +341,7 @@ static int submit_lock_objects_vmbind(struct msm_gem_submit *submit) submit->has_exec = true; drm_exec_until_all_locked (&submit->exec) { - ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1); - drm_exec_retry_on_contention(exec); - if (ret) - break; - - ret = drm_gpuvm_prepare_objects(submit->vm, exec, 1); + ret = submit_prepare_vm_objects(submit); drm_exec_retry_on_contention(exec); if (ret) break; @@ -790,18 +850,6 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data, submit_attach_object_fences(submit); - if (msm_context_is_vmbind(ctx)) { - /* - * If we are not using VM_BIND, submit_pin_vmas() will validate - * just the BOs attached to the submit. In that case we don't - * need to validate the _entire_ vm, because userspace tracked - * what BOs are associated with the submit. - */ - ret = drm_gpuvm_validate(submit->vm, &submit->exec); - if (ret) - goto out; - } - /* The scheduler owns a ref now: */ msm_gem_submit_get(submit); -- 2.34.1
