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

["Hellstrom, Thomas" <thomas.hellstrom@intel.com>]

On Wed, 2024-03-20 at 22:29 -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 understand the plan that for the 1:1 approach of
vma <-> pt_state, this would mark the pt_state invalid, but the xe_vma
remaining, possibly merge it with neightboring invalid xe_vmas?

For 1:N this would only invalidate certain pt_state chunks.

> 
> I believe this is also the correct behavior for userptr.
> This patch is experimental for CI and open to discuss

With the recent "invalid userptr" discussion we had with compute UMD,
they requested that if a new cpu_mm vma was added, the userptr would be
functional again. Nobody actually tested that that's the case with the
current code, but if so, this would break that behaviour. IMO the
current behavior should be kept. xe_vma remains but is marked invalid.


> 
> 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);
>  	}
>  }
>