Re: [CI 4/7] drm/xe: Move vma rebinding to the drm_exec locking loop

[Matthew Brost <matthew.brost@intel.com>]

On Tue, Mar 12, 2024 at 03:04:44PM +0100, Thomas Hellström wrote:
> Rebinding might allocate page-table bos, causing evictions.
> To support blocking locking during these evictions,
> perform the rebinding in the drm_exec locking loop.
> 
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> ---
>  drivers/gpu/drm/xe/xe_exec.c | 13 ++++++++++---
>  drivers/gpu/drm/xe/xe_vm.c   | 16 +++++++++-------
>  2 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_exec.c b/drivers/gpu/drm/xe/xe_exec.c
> index 13d1fba2c9d7..f36fb22a7861 100644
> --- a/drivers/gpu/drm/xe/xe_exec.c
> +++ b/drivers/gpu/drm/xe/xe_exec.c
> @@ -96,13 +96,20 @@
>  
>  static int xe_exec_fn(struct drm_gpuvm_exec *vm_exec)
>  {
> +	struct xe_vm *vm = container_of(vm_exec->vm, struct xe_vm, gpuvm);
>  	struct drm_gem_object *obj;
>  	unsigned long index;
>  	int ret;
>  
> -	ret = drm_gpuvm_validate(vm_exec->vm, &vm_exec->exec);
> -	if (ret)
> -		return ret;
> +	do {
> +		ret = drm_gpuvm_validate(vm_exec->vm, &vm_exec->exec);
> +		if (ret)
> +			return ret;
> +
> +		ret = xe_vm_rebind(vm, false);
> +		if (ret)
> +			return ret;
> +	} while (!list_empty(&vm->gpuvm.evict.list));
>  

You need to delete xe_vm_rebind from xe_exec_ioctl too.

>  	drm_exec_for_each_locked_object(&vm_exec->exec, index, obj) {
>  		ret = dma_resv_reserve_fences(obj->resv, 1);
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index faad7f52b3ca..fad5b1ae7f2d 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -513,9 +513,15 @@ static int xe_preempt_work_begin(struct drm_exec *exec, struct xe_vm *vm,
>  	if (err)
>  		return err;
>  
> -	err = drm_gpuvm_validate(&vm->gpuvm, exec);
> -	if (err)
> -		return err;
> +	do {
> +		err = drm_gpuvm_validate(&vm->gpuvm, exec);
> +		if (err)
> +			return err;
> +
> +		err = xe_vm_rebind(vm, true);
> +		if (err)
> +			return err;
> +	} while (!list_empty(&vm->gpuvm.evict.list));

Same code pattern twice here? Opportunity for a helper?

Matt

>  
>  	drm_exec_for_each_locked_object(exec, index, obj) {
>  		err = dma_resv_reserve_fences(obj->resv, vm->preempt.num_exec_queues);
> @@ -575,10 +581,6 @@ static void preempt_rebind_work_func(struct work_struct *w)
>  	if (err)
>  		goto out_unlock;
>  
> -	err = xe_vm_rebind(vm, true);
> -	if (err)
> -		goto out_unlock;
> -
>  	if (vm->rebind_fence) {
>  		dma_fence_wait(vm->rebind_fence, false);
>  		dma_fence_put(vm->rebind_fence);
> -- 
> 2.44.0
>