Re: [PATCH 2/2] drm/xe: Pick correct userptr VMA to repin on REMAP op failure

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

On 2024-02-01 01:48, Matthew Brost wrote:
> A REMAP op is composed of 3 VMA's - unmap, prev map, and next map. When
> op_execute fails with -EAGAIN we need to update the local VMA pointer to
> the current op state and then repin the VMA if it is a userptr.
> 
> Fixes a failure seen in xe_vm.munmap-style-unbind-userptr-one-partial.
> 
> Fixes: b06d47be7c83 ("drm/xe: Port Xe to GPUVA")
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_vm.c | 22 +++++++++++++++++-----
>   1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index e55161136490..2ab863fe7d0a 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -2506,13 +2506,25 @@ static int __xe_vma_op_execute(struct xe_vm *vm, struct xe_vma *vma,
>   	}
>   	drm_exec_fini(&exec);
>   
> -	if (err == -EAGAIN && xe_vma_is_userptr(vma)) {
> +	if (err == -EAGAIN) {
>   		lockdep_assert_held_write(&vm->lock);
> -		err = xe_vma_userptr_pin_pages(vma);
> -		if (!err)
> -			goto retry_userptr;
>   
> -		trace_xe_vma_fail(vma);
> +		if (op->base.op == DRM_GPUVA_OP_REMAP) {
> +			if (!op->remap.unmap_done)
> +				vma = gpuva_to_vma(op->base.remap.unmap->va);
> +			else if (op->remap.prev)
> +				vma = op->remap.prev;
> +			else
> +				vma = op->remap.next;
> +		}
I see this same vma picking in handling of DRM_GPUVA_OP_REMAP.

Could the switch in xe_vma_op_execute() be moved to a separate pick_vma 
function instead, called from this place too?

It might make the code slightly more readable.

Cheers,
~Maarten