[PATCH 1/6] drm/amdgpu: move taking mmap_sem into get_user_pages

[Felix Kuehling]

I made one comment on patch 5. Other than that, this series is
Reviewed-by: Felix Kuehling <Felix.Kuehling at amd.com>

This is the second week I'll get very little productive work done,
because I'm busy debugging memory manager changes and patching KFD to
keep it working. This series will require some changes to userptr
handling in KFD. Once KFD upstreaming is done, you'll have to change
amdgpu_amdkfd_gpuvm code when you're messing around with the memory
manager code. Feels like you're trying to get in all your changes as
long as that's still my problem. :P

The more you keep changing the memory manager, the longer this whole
upstreaming exercise will take me. My biggest motivation to finish
upstreaming is, so that I can stop adapting your memory manager patches.
This is not helping! ;)

Cheers,
  Felix


On 2017-09-05 11:37 AM, Christian König wrote:
> From: Christian König <christian.koenig at amd.com>
>
> This didn't helped as intended, just simplify the code.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 12 +-----------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  4 ----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |  4 ++++
>  3 files changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 4ab35db..e6e04d3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -497,18 +497,14 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
>  	struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
>  	struct amdgpu_bo_list_entry *e;
>  	struct list_head duplicates;
> -	bool need_mmap_lock = false;
>  	unsigned i, tries = 10;
>  	int r;
>  
>  	INIT_LIST_HEAD(&p->validated);
>  
>  	p->bo_list = amdgpu_bo_list_get(fpriv, cs->in.bo_list_handle);
> -	if (p->bo_list) {
> -		need_mmap_lock = p->bo_list->first_userptr !=
> -			p->bo_list->num_entries;
> +	if (p->bo_list)
>  		amdgpu_bo_list_get_list(p->bo_list, &p->validated);
> -	}
>  
>  	INIT_LIST_HEAD(&duplicates);
>  	amdgpu_vm_get_pd_bo(&fpriv->vm, &p->validated, &p->vm_pd);
> @@ -516,9 +512,6 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
>  	if (p->uf_entry.robj)
>  		list_add(&p->uf_entry.tv.head, &p->validated);
>  
> -	if (need_mmap_lock)
> -		down_read(&current->mm->mmap_sem);
> -
>  	while (1) {
>  		struct list_head need_pages;
>  		unsigned i;
> @@ -670,9 +663,6 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
>  
>  error_free_pages:
>  
> -	if (need_mmap_lock)
> -		up_read(&current->mm->mmap_sem);
> -
>  	if (p->bo_list) {
>  		for (i = p->bo_list->first_userptr;
>  		     i < p->bo_list->num_entries; ++i) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index f1e61b3..b0d45c8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -318,8 +318,6 @@ int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data,
>  	}
>  
>  	if (args->flags & AMDGPU_GEM_USERPTR_VALIDATE) {
> -		down_read(&current->mm->mmap_sem);
> -
>  		r = amdgpu_ttm_tt_get_user_pages(bo->tbo.ttm,
>  						 bo->tbo.ttm->pages);
>  		if (r)
> @@ -334,8 +332,6 @@ int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data,
>  		amdgpu_bo_unreserve(bo);
>  		if (r)
>  			goto free_pages;
> -
> -		up_read(&current->mm->mmap_sem);
>  	}
>  
>  	r = drm_gem_handle_create(filp, gobj, &handle);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 07c3f11..d2ce1a7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -622,6 +622,8 @@ int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages)
>  	if (!(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY))
>  		flags |= FOLL_WRITE;
>  
> +	down_read(&current->mm->mmap_sem);
> +
>  	if (gtt->userflags & AMDGPU_GEM_USERPTR_ANONONLY) {
>  		/* check that we only use anonymous memory
>  		   to prevent problems with writeback */
> @@ -657,6 +659,8 @@ int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages)
>  
>  	} while (pinned < ttm->num_pages);
>  
> +	up_read(&current->mm->mmap_sem);
> +
>  	return 0;
>  
>  release_pages: