Commit fec8fdb5 authored by Christian König's avatar Christian König Committed by Alex Deucher
Browse files

drm/amdgpu: fix userptr HMM range handling v2 



The basic problem here is that it's not allowed to page fault while
holding the reservation lock.

So it can happen that multiple processes try to validate an userptr
at the same time.

Work around that by putting the HMM range object into the mutex
protected bo list for now.

v2: make sure range is set to NULL in case of an error

Signed-off-by: default avatarChristian König <christian.koenig@amd.com>
Reviewed-by: default avatarAlex Deucher <alexander.deucher@amd.com>
Reviewed-by: default avatarFelix Kuehling <Felix.Kuehling@amd.com>
CC: stable@vger.kernel.org
Signed-off-by: default avatarAlex Deucher <alexander.deucher@amd.com>
parent 631945e0

drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c

+8 −4
Original line number Diff line number Diff line
@@ -938,6 +938,7 @@ static int init_user_pages(struct kgd_mem *mem, uint64_t user_addr, 
	struct amdkfd_process_info *process_info = mem->process_info;

	struct amdgpu_bo *bo = mem->bo;

	struct ttm_operation_ctx ctx = { true, false };

	struct hmm_range *range;

	int ret = 0;



	mutex_lock(&process_info->lock);
@@ -967,7 +968,7 @@ static int init_user_pages(struct kgd_mem *mem, uint64_t user_addr, 
		return 0;

	}



	ret = amdgpu_ttm_tt_get_user_pages(bo, bo->tbo.ttm->pages);

	ret = amdgpu_ttm_tt_get_user_pages(bo, bo->tbo.ttm->pages, &range);

	if (ret) {

		pr_err("%s: Failed to get user pages: %d\n", __func__, ret);

		goto unregister_out;
@@ -985,7 +986,7 @@ static int init_user_pages(struct kgd_mem *mem, uint64_t user_addr, 
	amdgpu_bo_unreserve(bo);



release_out:

	amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm);

	amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm, range);

unregister_out:

	if (ret)

		amdgpu_mn_unregister(bo);
@@ -2317,6 +2318,8 @@ static int update_invalid_user_pages(struct amdkfd_process_info *process_info, 
	/* Go through userptr_inval_list and update any invalid user_pages */

	list_for_each_entry(mem, &process_info->userptr_inval_list,

			    validate_list.head) {

		struct hmm_range *range;



		invalid = atomic_read(&mem->invalid);

		if (!invalid)

			/* BO hasn't been invalidated since the last
@@ -2327,7 +2330,8 @@ static int update_invalid_user_pages(struct amdkfd_process_info *process_info, 
		bo = mem->bo;



		/* Get updated user pages */

		ret = amdgpu_ttm_tt_get_user_pages(bo, bo->tbo.ttm->pages);

		ret = amdgpu_ttm_tt_get_user_pages(bo, bo->tbo.ttm->pages,

						   &range);

		if (ret) {

			pr_debug("Failed %d to get user pages\n", ret);
@@ -2346,7 +2350,7 @@ static int update_invalid_user_pages(struct amdkfd_process_info *process_info, 
			 * FIXME: Cannot ignore the return code, must hold

			 * notifier_lock

			 */

			amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm);

			amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm, range);

		}



		/* Mark the BO as valid unless it was invalidated

drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c

+1 −0
Original line number Diff line number Diff line
@@ -209,6 +209,7 @@ void amdgpu_bo_list_get_list(struct amdgpu_bo_list *list, 
			list_add_tail(&e->tv.head, &bucket[priority]);



		e->user_pages = NULL;

		e->range = NULL;

	}



	/* Connect the sorted buckets in the output list. */

drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h

+3 −0
Original line number Diff line number Diff line
@@ -26,6 +26,8 @@ 
#include <drm/ttm/ttm_execbuf_util.h>

#include <drm/amdgpu_drm.h>



struct hmm_range;



struct amdgpu_device;

struct amdgpu_bo;

struct amdgpu_bo_va;
@@ -36,6 +38,7 @@ struct amdgpu_bo_list_entry { 
	struct amdgpu_bo_va		*bo_va;

	uint32_t			priority;

	struct page			**user_pages;

	struct hmm_range		*range;

	bool				user_invalidated;

};

drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c

+5 −3
Original line number Diff line number Diff line
@@ -912,7 +912,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, 
			goto out_free_user_pages;

		}



		r = amdgpu_ttm_tt_get_user_pages(bo, e->user_pages);

		r = amdgpu_ttm_tt_get_user_pages(bo, e->user_pages, &e->range);

		if (r) {

			kvfree(e->user_pages);

			e->user_pages = NULL;
@@ -990,9 +990,10 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, 


		if (!e->user_pages)

			continue;

		amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm);

		amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm, e->range);

		kvfree(e->user_pages);

		e->user_pages = NULL;

		e->range = NULL;

	}

	mutex_unlock(&p->bo_list->bo_list_mutex);

	return r;
@@ -1267,7 +1268,8 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, 
	amdgpu_bo_list_for_each_userptr_entry(e, p->bo_list) {

		struct amdgpu_bo *bo = ttm_to_amdgpu_bo(e->tv.bo);



		r |= !amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm);

		r |= !amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm, e->range);

		e->range = NULL;

	}

	if (r) {

		r = -EAGAIN;

drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c

+4 −2
Original line number Diff line number Diff line
@@ -378,6 +378,7 @@ int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data, 
	struct amdgpu_device *adev = drm_to_adev(dev);

	struct drm_amdgpu_gem_userptr *args = data;

	struct drm_gem_object *gobj;

	struct hmm_range *range;

	struct amdgpu_bo *bo;

	uint32_t handle;

	int r;
@@ -418,7 +419,8 @@ int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data, 
		goto release_object;



	if (args->flags & AMDGPU_GEM_USERPTR_VALIDATE) {

		r = amdgpu_ttm_tt_get_user_pages(bo, bo->tbo.ttm->pages);

		r = amdgpu_ttm_tt_get_user_pages(bo, bo->tbo.ttm->pages,

						 &range);

		if (r)

			goto release_object;
@@ -441,7 +443,7 @@ int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data, 


user_pages_done:

	if (args->flags & AMDGPU_GEM_USERPTR_VALIDATE)

		amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm);

		amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm, range);



release_object:

	drm_gem_object_put(gobj);