Re: [PATCH 4/4] drm/xe: destroy userptr vma on UNMAP event

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

On Wed, Mar 20, 2024 at 10:29:39PM -0400, Oak Zeng wrote:
> When there is MMU_NOTIFY_UNMAP event happens, the userptr
> is munmapped from CPU. There is no need to keep the xe_vma
> for this userptr from GPU side. So we destroy it.
> 
> But we can't destroy vma directly from the mmu notifier
> callback function, because we need to remove mmu
> notifier during vma destroy. If we remove mmu notifier
> directly from mmu notifier callback, it is a deadlock.
> xe_vma_destroy is modified to destroy vma in a worker
> thread.
> 
> Another reason of this change is, for the future
> hmmptr codes, we destroy vma when hmmptr is unmapped
> from CPU. We want to unify the hmmptr and userptr
> code.
> 
> I believe this is also the correct behavior for userptr.

It is not, the user is still responsible for unmapping. Even if this was
the correct behavior we'd have to unmap from the GPU and update internal
PT state before calling xe_vma_destroy.

The correct behavior in this is case just let the userptr page pin fail
and invalidate GPU mappings. This recently got merge in [1].

Have a pending test for this in [2].

Matt

[1] https://patchwork.freedesktop.org/series/130935/
[2] https://patchwork.freedesktop.org/series/131211/

> This patch is experimental for CI and open to discuss
> 
> Signed-off-by: Oak Zeng <oak.zeng@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_vm.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 11a4bb9d5415..90d1163c1090 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -40,6 +40,8 @@
>  #include "xe_wa.h"
>  #include "xe_hmm.h"
>  
> +static void xe_vma_destroy(struct xe_vma *vma, struct dma_fence *fence);
> +
>  static struct drm_gem_object *xe_vm_obj(struct xe_vm *vm)
>  {
>  	return vm->gpuvm.r_obj;
> @@ -604,6 +606,9 @@ static bool vma_userptr_invalidate(struct mmu_interval_notifier *mni,
>  
>  	trace_xe_vma_userptr_invalidate_complete(vma);
>  
> +	if (range->event == MMU_NOTIFY_UNMAP)
> +		xe_vma_destroy(vma, NULL);
> +
>  	return true;
>  }
>  
> @@ -901,7 +906,8 @@ static void xe_vma_destroy(struct xe_vma *vma, struct dma_fence *fence)
>  			xe_vma_destroy_late(vma);
>  		}
>  	} else {
> -		xe_vma_destroy_late(vma);
> +		INIT_WORK(&vma->destroy_work, vma_destroy_work_func);
> +		queue_work(system_unbound_wq, &vma->destroy_work);
>  	}
>  }
>  
> -- 
> 2.26.3
>