[PATCH 1/3] drm/ttm: add bit flag TTM_OPT_FLAG_FORCE_ALLOC

[Christian König]

Am 09.02.2018 um 08:30 schrieb Roger He:
> set TTM_OPT_FLAG_FORCE_ALLOC when we are servicing for page
> fault routine.
>
> for ttm_mem_global_reserve if in page fault routine, allow the gtt
> pages reservation always. because page fault routing already grabbed
> system memory and the allowance of this exception is harmless.
> Otherwise, it will trigger OOM killer.
>
> v2: keep original behavior except ttm bo with flag no_retry
> v3: forward the ttm_operation_ctx to ttm_mem_global_reserve
>
> Signed-off-by: Roger He <Hongbo.He at amd.com>
> ---
>   drivers/gpu/drm/ttm/ttm_bo_vm.c          |  6 ++++--
>   drivers/gpu/drm/ttm/ttm_memory.c         | 18 ++++++++++++++----
>   drivers/gpu/drm/ttm/ttm_page_alloc_dma.c |  1 -
>   include/drm/ttm/ttm_bo_api.h             |  4 +++-
>   4 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index 716e724..313398a 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -224,7 +224,7 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
>   		cvma.vm_page_prot = ttm_io_prot(bo->mem.placement,
>   						cvma.vm_page_prot);
>   	} else {
> -		struct ttm_operation_ctx ctx = {
> +		struct ttm_operation_ctx ttm_opt_ctx = {

Drop all those variable renames, if we really want to do this we should 
have a separate patch.

>   			.interruptible = false,
>   			.no_wait_gpu = false
>   		};
> @@ -233,8 +233,10 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
>   		cvma.vm_page_prot = ttm_io_prot(bo->mem.placement,
>   						cvma.vm_page_prot);
>   
> +		if (ttm->page_flags & TTM_PAGE_FLAG_NO_RETRY)
> +			ttm_opt_ctx.flags = TTM_OPT_FLAG_FORCE_ALLOC;

Just set that flag unconditionally for the page fault context.

>   		/* Allocate all page at once, most common usage */
> -		if (ttm->bdev->driver->ttm_tt_populate(ttm, &ctx)) {
> +		if (ttm->bdev->driver->ttm_tt_populate(ttm, &ttm_opt_ctx)) {
>   			ret = VM_FAULT_OOM;
>   			goto out_io_unlock;
>   		}
> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
> index aa0c381..154719b 100644
> --- a/drivers/gpu/drm/ttm/ttm_memory.c
> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
> @@ -471,7 +471,8 @@ EXPORT_SYMBOL(ttm_mem_global_free);
>   
>   static int ttm_mem_global_reserve(struct ttm_mem_global *glob,
>   				  struct ttm_mem_zone *single_zone,
> -				  uint64_t amount, bool reserve)
> +				  uint64_t amount, bool reserve,
> +				  struct ttm_operation_ctx *ctx)
>   {
>   	uint64_t limit;
>   	int ret = -ENOMEM;
> @@ -479,6 +480,15 @@ static int ttm_mem_global_reserve(struct ttm_mem_global *glob,
>   	struct ttm_mem_zone *zone;
>   
>   	spin_lock(&glob->lock);
> +	/*
> +	 * to cover two special cases:
> +	 * a. if serving page_fault allow reservation anyway since
> +	 * it already allocated system pages. Otherwise it will trigger OOM.
> +	 * b. if serving suspend, allow reservation anyway as well.
> +	 */
> +	if (ctx->flags & TTM_OPT_FLAG_FORCE_ALLOC)
> +		goto force_reserve;
> +

Oh, you want to skip the zone check as well? Not sure if that is a good 
idea or not.

Please drop that hunk for now and only skip the new limit when setting 
the ctx flag.

>   	for (i = 0; i < glob->num_zones; ++i) {
>   		zone = glob->zones[i];
>   		if (single_zone && zone != single_zone)
> @@ -491,6 +501,7 @@ static int ttm_mem_global_reserve(struct ttm_mem_global *glob,
>   			goto out_unlock;
>   	}
>   
> +force_reserve:
>   	if (reserve) {
>   		for (i = 0; i < glob->num_zones; ++i) {
>   			zone = glob->zones[i];
> @@ -516,9 +527,8 @@ static int ttm_mem_global_alloc_zone(struct ttm_mem_global *glob,
>   {
>   	int count = TTM_MEMORY_ALLOC_RETRIES;
>   
> -	while (unlikely(ttm_mem_global_reserve(glob,
> -					       single_zone,
> -					       memory, true)
> +	while (unlikely(ttm_mem_global_reserve(glob, single_zone,
> +					       memory, true, ctx)
>   			!= 0)) {
>   		if (ctx->no_wait_gpu)
>   			return -ENOMEM;
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> index b122f6e..354e0e1 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.cttm_mem_global_reserve
> @@ -944,7 +944,6 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev,
>   	i = 0;
>   
>   	type = ttm_to_type(ttm->page_flags, ttm->caching_state);
> -

Unrelated whitespace change please drop that.

Regards,
Christian.

>   #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>   	if (ttm->page_flags & TTM_PAGE_FLAG_DMA32)
>   		goto skip_huge;
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index 872ff6c..2142639 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -278,7 +278,9 @@ struct ttm_operation_ctx {
>   };
>   
>   /* Allow eviction of reserved BOs */
> -#define TTM_OPT_FLAG_ALLOW_RES_EVICT	0x1
> +#define TTM_OPT_FLAG_ALLOW_RES_EVICT		0x1
> +/* when serving page fault or suspend, allow alloc anyway */
> +#define TTM_OPT_FLAG_FORCE_ALLOC		0x2
>   
>   /**
>    * ttm_bo_reference - reference a struct ttm_buffer_object