[PATCH 2/2] drm/amdkfd: Release an acquired process vm

[Christian König]

Am 27.08.2018 um 22:45 schrieb Oak Zeng:
> For compute vm acquired from amdgpu, vm.pasid is managed
> by kfd. Decouple pasid from such vm on process destroy
> to avoid duplicate pasid release.
>
> Signed-off-by: Oak Zeng <Oak.Zeng at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h        |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 26 +++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c          |  4 +++-
>   drivers/gpu/drm/amd/include/kgd_kfd_interface.h   |  1 +
>   7 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index e1f6454..b414889 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -165,6 +165,7 @@ int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
>   void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
>   				struct amdgpu_vm *vm);
>   void amdgpu_amdkfd_gpuvm_destroy_process_vm(struct kgd_dev *kgd, void *vm);
> +void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *vm);
>   uint32_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *vm);
>   int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>   		struct kgd_dev *kgd, uint64_t va, uint64_t size,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
> index ea79908..03a83d3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
> @@ -204,6 +204,7 @@ static const struct kfd2kgd_calls kfd2kgd = {
>   	.create_process_vm = amdgpu_amdkfd_gpuvm_create_process_vm,
>   	.acquire_process_vm = amdgpu_amdkfd_gpuvm_acquire_process_vm,
>   	.destroy_process_vm = amdgpu_amdkfd_gpuvm_destroy_process_vm,
> +	.release_process_vm = amdgpu_amdkfd_gpuvm_release_process_vm,
>   	.get_process_page_dir = amdgpu_amdkfd_gpuvm_get_process_page_dir,
>   	.set_vm_context_page_table_base = set_vm_context_page_table_base,
>   	.alloc_memory_of_gpu = amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> index 19dd665..b2b9c5f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> @@ -164,6 +164,7 @@ static const struct kfd2kgd_calls kfd2kgd = {
>   	.create_process_vm = amdgpu_amdkfd_gpuvm_create_process_vm,
>   	.acquire_process_vm = amdgpu_amdkfd_gpuvm_acquire_process_vm,
>   	.destroy_process_vm = amdgpu_amdkfd_gpuvm_destroy_process_vm,
> +	.release_process_vm = amdgpu_amdkfd_gpuvm_release_process_vm,
>   	.get_process_page_dir = amdgpu_amdkfd_gpuvm_get_process_page_dir,
>   	.set_vm_context_page_table_base = set_vm_context_page_table_base,
>   	.alloc_memory_of_gpu = amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> index 1db60aa..3722bbd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> @@ -201,6 +201,7 @@ static const struct kfd2kgd_calls kfd2kgd = {
>   	.create_process_vm = amdgpu_amdkfd_gpuvm_create_process_vm,
>   	.acquire_process_vm = amdgpu_amdkfd_gpuvm_acquire_process_vm,
>   	.destroy_process_vm = amdgpu_amdkfd_gpuvm_destroy_process_vm,
> +	.release_process_vm = amdgpu_amdkfd_gpuvm_release_process_vm,
>   	.get_process_page_dir = amdgpu_amdkfd_gpuvm_get_process_page_dir,
>   	.set_vm_context_page_table_base = set_vm_context_page_table_base,
>   	.alloc_memory_of_gpu = amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 0980a1f..6fd839c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1117,6 +1117,32 @@ void amdgpu_amdkfd_gpuvm_destroy_process_vm(struct kgd_dev *kgd, void *vm)
>   	kfree(vm);
>   }
>   
> +void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *vm)
> +{
> +	struct amdgpu_device *adev = get_amdgpu_device(kgd);
> +        struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
> +
> +        if (WARN_ON(!vm))
> +                return;
> +
> +        pr_debug("Releasing process vm %p\n", vm);
> +
> +        /* The original pasid of amdgpu vm has already been
> +         * released during making a amdgpu vm to a compute vm
> +         * The current pasid is managed by kfd and will be
> +         * released on kfd process destroy. Set amdgpu pasid
> +         * to 0 to avoid duplicate release.
> +         */
> +	if (avm->pasid) {
> +		unsigned long flags;
> +
> +		spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags);
> +		idr_remove(&adev->vm_manager.pasid_idr, avm->pasid);
> +		spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags);
> +	}
> +	avm->pasid = 0;

Can we factor this out into a helper inside amdgpu_vm.c?

Would be nice to not manipulate VM internals here.

Christian.

> +}
> +
>   uint32_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *vm)
>   {
>   	struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 5f559c5..7afeaa1 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -320,8 +320,10 @@ static void kfd_process_destroy_pdds(struct kfd_process *p)
>   		pr_debug("Releasing pdd (topology id %d) for process (pasid %d)\n",
>   				pdd->dev->id, p->pasid);
>   
> -		if (pdd->drm_file)
> +		if (pdd->drm_file) {
> +			pdd->dev->kfd2kgd->release_process_vm(pdd->dev->kgd, pdd->vm);
>   			fput(pdd->drm_file);
> +		}
>   		else if (pdd->vm)
>   			pdd->dev->kfd2kgd->destroy_process_vm(
>   				pdd->dev->kgd, pdd->vm);
> diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> index 1632869..8fdd032 100644
> --- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> +++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> @@ -352,6 +352,7 @@ struct kfd2kgd_calls {
>   			unsigned int pasid, void **vm, void **process_info,
>   			struct dma_fence **ef);
>   	void (*destroy_process_vm)(struct kgd_dev *kgd, void *vm);
> +	void (*release_process_vm)(struct kgd_dev *kgd, void *vm);
>   	uint32_t (*get_process_page_dir)(void *vm);
>   	void (*set_vm_context_page_table_base)(struct kgd_dev *kgd,
>   			uint32_t vmid, uint32_t page_table_base);