On Fri, Mar 28, 2025 at 06:10:36PM +0000, Matthew Auld wrote:
> We now use the same notifier lock for SVM and userptr, with that we can
> combine xe_pt_userptr_pre_commit and xe_pt_svm_pre_commit.
>
> Suggested-by: Matthew Brost <matthew.br...@intel.com>
> Signed-off-by: Matthew Auld <matthew.a...@intel.com>
> Cc: Himal Prasad Ghimiray <himal.prasad.ghimi...@intel.com>
> Cc: Thomas Hellström <thomas.hellst...@linux.intel.com>
> ---
> drivers/gpu/drm/xe/xe_pt.c | 95 +++++++++++++-------------------------
> 1 file changed, 33 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index b097c91e2e02..947b82aa19a6 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -1396,7 +1396,7 @@ static int op_check_userptr(struct xe_vm *vm, struct
> xe_vma_op *op,
> return err;
> }
>
> -static int xe_pt_userptr_pre_commit(struct xe_migrate_pt_update *pt_update)
> +static int xe_pt_svm_userptr_pre_commit(struct xe_migrate_pt_update
> *pt_update)
> {
> struct xe_vm *vm = pt_update->vops->vm;
> struct xe_vma_ops *vops = pt_update->vops;
> @@ -1409,55 +1409,40 @@ static int xe_pt_userptr_pre_commit(struct
> xe_migrate_pt_update *pt_update)
> if (err)
> return err;
>
> - down_read(&vm->svm.gpusvm.notifier_lock);
> + drm_gpusvm_notifier_lock(&vm->svm.gpusvm);
Also any reason not use xe_svm_notifier_lock / xe_svm_notifier_unlock wrappers?
Matt
>
> list_for_each_entry(op, &vops->list, link) {
> - err = op_check_userptr(vm, op, pt_update_ops);
> - if (err) {
> - up_read(&vm->svm.gpusvm.notifier_lock);
> - break;
> + if (pt_update_ops->needs_svm_lock) {
> +#if IS_ENABLED(CONFIG_DRM_XE_GPUSVM)
> + struct xe_svm_range *range = op->map_range.range;
> +
> + if (op->subop == XE_VMA_SUBOP_UNMAP_RANGE)
> + continue;
> +
> + xe_svm_range_debug(range, "PRE-COMMIT");
> +
> + xe_assert(vm->xe,
> + xe_vma_is_cpu_addr_mirror(op->map_range.vma));
> + xe_assert(vm->xe, op->subop == XE_VMA_SUBOP_MAP_RANGE);
> +
> + if (!xe_svm_range_pages_valid(range)) {
> + xe_svm_range_debug(range, "PRE-COMMIT - RETRY");
> + drm_gpusvm_notifier_unlock(&vm->svm.gpusvm);
> + return -EAGAIN;
> + }
> +#endif
> + } else {
> + err = op_check_userptr(vm, op, pt_update_ops);
> + if (err) {
> + drm_gpusvm_notifier_unlock(&vm->svm.gpusvm);
> + break;
> + }
> }
> }
>
> return err;
> }
>
> -#if IS_ENABLED(CONFIG_DRM_XE_GPUSVM)
> -static int xe_pt_svm_pre_commit(struct xe_migrate_pt_update *pt_update)
> -{
> - struct xe_vm *vm = pt_update->vops->vm;
> - struct xe_vma_ops *vops = pt_update->vops;
> - struct xe_vma_op *op;
> - int err;
> -
> - err = xe_pt_pre_commit(pt_update);
> - if (err)
> - return err;
> -
> - xe_svm_notifier_lock(vm);
> -
> - list_for_each_entry(op, &vops->list, link) {
> - struct xe_svm_range *range = op->map_range.range;
> -
> - if (op->subop == XE_VMA_SUBOP_UNMAP_RANGE)
> - continue;
> -
> - xe_svm_range_debug(range, "PRE-COMMIT");
> -
> - xe_assert(vm->xe, xe_vma_is_cpu_addr_mirror(op->map_range.vma));
> - xe_assert(vm->xe, op->subop == XE_VMA_SUBOP_MAP_RANGE);
> -
> - if (!xe_svm_range_pages_valid(range)) {
> - xe_svm_range_debug(range, "PRE-COMMIT - RETRY");
> - xe_svm_notifier_unlock(vm);
> - return -EAGAIN;
> - }
> - }
> -
> - return 0;
> -}
> -#endif
> -
> struct invalidation_fence {
> struct xe_gt_tlb_invalidation_fence base;
> struct xe_gt *gt;
> @@ -2255,22 +2240,12 @@ static const struct xe_migrate_pt_update_ops
> migrate_ops = {
> .pre_commit = xe_pt_pre_commit,
> };
>
> -static const struct xe_migrate_pt_update_ops userptr_migrate_ops = {
> +static const struct xe_migrate_pt_update_ops svm_userptr_migrate_ops = {
> .populate = xe_vm_populate_pgtable,
> .clear = xe_migrate_clear_pgtable_callback,
> - .pre_commit = xe_pt_userptr_pre_commit,
> + .pre_commit = xe_pt_svm_userptr_pre_commit,
> };
>
> -#if IS_ENABLED(CONFIG_DRM_XE_GPUSVM)
> -static const struct xe_migrate_pt_update_ops svm_migrate_ops = {
> - .populate = xe_vm_populate_pgtable,
> - .clear = xe_migrate_clear_pgtable_callback,
> - .pre_commit = xe_pt_svm_pre_commit,
> -};
> -#else
> -static const struct xe_migrate_pt_update_ops svm_migrate_ops;
> -#endif
> -
> /**
> * xe_pt_update_ops_run() - Run PT update operations
> * @tile: Tile of PT update operations
> @@ -2296,10 +2271,8 @@ xe_pt_update_ops_run(struct xe_tile *tile, struct
> xe_vma_ops *vops)
> struct xe_vma_op *op;
> int err = 0, i;
> struct xe_migrate_pt_update update = {
> - .ops = pt_update_ops->needs_svm_lock ?
> - &svm_migrate_ops :
> - pt_update_ops->needs_userptr_lock ?
> - &userptr_migrate_ops :
> + .ops = pt_update_ops->needs_svm_lock ||
> pt_update_ops->needs_userptr_lock ?
> + &svm_userptr_migrate_ops :
> &migrate_ops,
> .vops = vops,
> .tile_id = tile->id,
> @@ -2419,10 +2392,8 @@ xe_pt_update_ops_run(struct xe_tile *tile, struct
> xe_vma_ops *vops)
> &ifence->base.base, &mfence->base.base);
> }
>
> - if (pt_update_ops->needs_svm_lock)
> - xe_svm_notifier_unlock(vm);
> - if (pt_update_ops->needs_userptr_lock)
> - up_read(&vm->svm.gpusvm.notifier_lock);
> + if (pt_update_ops->needs_svm_lock || pt_update_ops->needs_userptr_lock)
> + drm_gpusvm_notifier_unlock(&vm->svm.gpusvm);
>
> return fence;
>
> --
> 2.48.1
>
