[PATCH 3/3] drm/ttm: check if free mem space is under the lower limit

[Christian König]

Am 09.02.2018 um 11:00 schrieb Roger He:
> the free mem space and the lower limit both include two parts:
> system memory and swap space.
>
> For the OOM triggered by TTM, that is the case as below:
> first swap space is full of swapped out pages and soon
> system memory also is filled up with ttm pages. and then
> any memory allocation request will run into OOM.
>
> to cover two cases:
> a. if no swap disk at all or free swap space is under swap mem
>     limit but available system mem is bigger than sys mem limit,
>     allow TTM allocation;
>
> b. if the available system mem is less than sys mem limit but
>     free swap space is bigger than swap mem limit, allow TTM
>     allocation.
>
> v2: merge two memory limit(swap and system) into one
> v3: keep original behavior except ttm_opt_ctx->flags with
>      TTM_OPT_FLAG_FORCE_ALLOC
> v4: always set force_alloc as tx->flags & TTM_OPT_FLAG_FORCE_ALLOC
>
> Signed-off-by: Roger He <Hongbo.He at amd.com>
> ---
>   drivers/gpu/drm/ttm/ttm_memory.c         | 35 ++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/ttm/ttm_page_alloc.c     |  4 ++++
>   drivers/gpu/drm/ttm/ttm_page_alloc_dma.c |  4 ++++
>   include/drm/ttm/ttm_memory.h             |  5 +++++
>   4 files changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
> index aa0c381..0d41a99 100644
> --- a/drivers/gpu/drm/ttm/ttm_memory.c
> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
> @@ -36,6 +36,7 @@
>   #include <linux/mm.h>
>   #include <linux/module.h>
>   #include <linux/slab.h>
> +#include <linux/swap.h>
>   
>   #define TTM_MEMORY_ALLOC_RETRIES 4
>   
> @@ -375,6 +376,11 @@ int ttm_mem_global_init(struct ttm_mem_global *glob)
>   
>   	si_meminfo(&si);
>   
> +	/* lower limit of swap space and 256MB is enough */
> +	glob->lower_mem_limit = 256 << 8;
> +	/* lower limit of ram and keep consistent with each zone->emer_mem */
> +	glob->lower_mem_limit += si.totalram >> 2;

That looks really good to me, but we need to make sure that this is 
configurable.

E.g. we need to add an attribute to ttm_mem_glob_kobj_type and implement 
the show/store callbacks.

Apart from that one more comment below.

> +
>   	ret = ttm_mem_init_kernel_zone(glob, &si);
>   	if (unlikely(ret != 0))
>   		goto out_no_zone;
> @@ -469,6 +475,35 @@ void ttm_mem_global_free(struct ttm_mem_global *glob,
>   }
>   EXPORT_SYMBOL(ttm_mem_global_free);
>   
> +/*
> + * check if the available mem is under lower memory limit
> + *
> + * a. if no swap disk at all or free swap space is under swap_mem_limit
> + * but available system mem is bigger than sys_mem_limit, allow TTM
> + * allocation;
> + *
> + * b. if the available system mem is less than sys_mem_limit but free
> + * swap disk is bigger than swap_mem_limit, allow TTM allocation.
> + */
> +bool
> +ttm_check_under_lowerlimit(struct ttm_mem_global *glob,
> +		uint64_t num_pages, bool force_alloc)

I would pass in the whole ttm_operation_ctx here.

Regards,
Christian.

> +{
> +	bool ret = false;
> +	int64_t available;
> +
> +	if (force_alloc)
> +		return false;
> +
> +	available = get_nr_swap_pages() + si_mem_available();
> +	available -= num_pages;
> +	if (available < glob->lower_mem_limit)
> +		ret = true;
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(ttm_check_under_lowerlimit);
> +
>   static int ttm_mem_global_reserve(struct ttm_mem_global *glob,
>   				  struct ttm_mem_zone *single_zone,
>   				  uint64_t amount, bool reserve)
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c
> index 5edcd89..bd312e6 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
> @@ -1100,6 +1100,10 @@ int ttm_pool_populate(struct ttm_tt *ttm, struct ttm_operation_ctx *ctx)
>   	if (ttm->state != tt_unpopulated)
>   		return 0;
>   
> +	if (ttm_check_under_lowerlimit(mem_glob, ttm->num_pages,
> +				       ctx->flags & TTM_OPT_FLAG_FORCE_ALLOC))
> +		return -ENOMEM;
> +
>   	ret = ttm_get_pages(ttm->pages, ttm->num_pages, ttm->page_flags,
>   			    ttm->caching_state);
>   	if (unlikely(ret != 0)) {
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> index b122f6e..fc32096 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> @@ -940,6 +940,10 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev,
>   	if (ttm->state != tt_unpopulated)
>   		return 0;
>   
> +	if (ttm_check_under_lowerlimit(mem_glob, num_pages,
> +				       ctx->flags & TTM_OPT_FLAG_FORCE_ALLOC))
> +		return -ENOMEM;
> +
>   	INIT_LIST_HEAD(&ttm_dma->pages_list);
>   	i = 0;
>   
> diff --git a/include/drm/ttm/ttm_memory.h b/include/drm/ttm/ttm_memory.h
> index 8936285..5ada921 100644
> --- a/include/drm/ttm/ttm_memory.h
> +++ b/include/drm/ttm/ttm_memory.h
> @@ -49,6 +49,8 @@
>    * @work: The workqueue callback for the shrink queue.
>    * @lock: Lock to protect the @shrink - and the memory accounting members,
>    * that is, essentially the whole structure with some exceptions.
> + * @lower_mem_limit: include lower limit of swap space and lower limit of
> + * system memory.
>    * @zones: Array of pointers to accounting zones.
>    * @num_zones: Number of populated entries in the @zones array.
>    * @zone_kernel: Pointer to the kernel zone.
> @@ -67,6 +69,7 @@ struct ttm_mem_global {
>   	struct workqueue_struct *swap_queue;
>   	struct work_struct work;
>   	spinlock_t lock;
> +	uint64_t lower_mem_limit;
>   	struct ttm_mem_zone *zones[TTM_MEM_MAX_ZONES];
>   	unsigned int num_zones;
>   	struct ttm_mem_zone *zone_kernel;
> @@ -90,4 +93,6 @@ extern void ttm_mem_global_free_page(struct ttm_mem_global *glob,
>   				     struct page *page, uint64_t size);
>   extern size_t ttm_round_pot(size_t size);
>   extern uint64_t ttm_get_kernel_zone_memory_size(struct ttm_mem_global *glob);
> +extern bool ttm_check_under_lowerlimit(struct ttm_mem_global *glob,
> +				uint64_t num_pages, bool force_alloc);
>   #endif