On 08/13/2014 05:52 AM, J?r?me Glisse wrote:
> From: J?r?me Glisse <jglisse at redhat.com>
>
> When experiencing memory pressure we want to minimize pool size so that
> memory we just shrinked is not added back again just as the next thing.
>
> This will divide by 2 the maximum pool size for each device each time
> the pool have to shrink. The limit is bumped again is next allocation
> happen after one second since the last shrink. The one second delay is
> obviously an arbitrary choice.
J?r?me,
I don't like this patch. It adds extra complexity and its usefulness is
highly questionable.
There are a number of caches in the system, and if all of them added
some sort of voluntary shrink heuristics like this, we'd end up with
impossible-to-debug unpredictable performance issues.
We should let the memory subsystem decide when to reclaim pages from
caches and what caches to reclaim them from.
/Thomas
>
> Signed-off-by: J?r?me Glisse <jglisse at redhat.com>
> Cc: Mario Kleiner <mario.kleiner.de at gmail.com>
> Cc: Michel D?nzer <michel at daenzer.net>
> Cc: Thomas Hellstrom <thellstrom at vmware.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk at oracle.com>
> ---
> drivers/gpu/drm/ttm/ttm_page_alloc.c | 35
> +++++++++++++++++++++++++-------
> drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 27 ++++++++++++++++++++++--
> 2 files changed, 53 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c
> b/drivers/gpu/drm/ttm/ttm_page_alloc.c
> index 09874d6..ab41adf 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
> @@ -68,6 +68,8 @@
> * @list: Pool of free uc/wc pages for fast reuse.
> * @gfp_flags: Flags to pass for alloc_page.
> * @npages: Number of pages in pool.
> + * @cur_max_size: Current maximum size for the pool.
> + * @shrink_timeout: Timeout for pool maximum size restriction.
> */
> struct ttm_page_pool {
> spinlock_t lock;
> @@ -76,6 +78,8 @@ struct ttm_page_pool {
> gfp_t gfp_flags;
> unsigned npages;
> char *name;
> + unsigned cur_max_size;
> + unsigned long last_shrink;
> unsigned long nfrees;
> unsigned long nrefills;
> };
> @@ -289,6 +293,16 @@ static void ttm_pool_update_free_locked(struct
> ttm_page_pool *pool,
> pool->nfrees += freed_pages;
> }
>
> +static inline void ttm_pool_update_max_size(struct ttm_page_pool *pool)
> +{
> + if (time_before(jiffies, pool->shrink_timeout))
> + return;
> + /* In case we reached zero bounce back to 512 pages. */
> + pool->cur_max_size = max(pool->cur_max_size << 1, 512);
> + pool->cur_max_size = min(pool->cur_max_size,
> + _manager->options.max_size);
> +}
> +
> /**
> * Free pages from pool.
> *
> @@ -407,6 +421,9 @@ ttm_pool_shrink_scan(struct shrinker *shrink, struct
> shrink_control *sc)
> if (shrink_pages == 0)
> break;
> pool = &_manager->pools[(i + pool_offset)%NUM_POOLS];
> + /* No matter what make sure the pool do not grow in next
> second. */
> + pool->cur_max_size = pool->cur_max_size >> 1;
> + pool->shrink_timeout = jiffies + HZ;
> shrink_pages = ttm_page_pool_free(pool, nr_free,
> sc->gfp_mask);
> freed += nr_free - shrink_pages;
> @@ -701,13 +718,12 @@ static void ttm_put_pages(struct page **pages, unsigned
> npages, int flags,
> }
> /* Check that we don't go over the pool limit */
> npages = 0;
> - if (pool->npages > _manager->options.max_size) {
> - npages = pool->npages - _manager->options.max_size;
> - /* free at least NUM_PAGES_TO_ALLOC number of pages
> - * to reduce calls to set_memory_wb */
> - if (npages < NUM_PAGES_TO_ALLOC)
> - npages = NUM_PAGES_TO_ALLOC;
> - }
> + /*
> + * Free at least NUM_PAGES_TO_ALLOC number of pages to reduce calls to
> + * set_memory_wb.
> + */
> + if (pool->npages > (pool->cur_max_size + NUM_PAGES_TO_ALLOC))
> + npages = pool->npages - pool->cur_max_size;
> spin_unlock_irqrestore(&pool->lock, irq_flags);
> if (npages)
> ttm_page_pool_free(pool, npages, GFP_KERNEL);
> @@ -751,6 +767,9 @@ static int ttm_get_pages(struct page **pages, unsigned
> npages, int flags,
> return 0;
> }
>
> + /* Update pool size in case shrinker limited it. */
> + ttm_pool_update_max_size(pool);
> +
> /* combine zero flag to pool flags */
> gfp_flags |= pool->gfp_flags;
>
> @@ -803,6 +822,8 @@ static void ttm_page_pool_init_locked(struct
> ttm_page_pool *pool, gfp_t flags,
> pool->npages = pool->nfrees = 0;
> pool->gfp_flags = flags;
> pool->name = name;
> + pool->cur_max_size = _manager->options.max_size;
> + pool->shrink_timeout = jiffies;
> }
>
> int ttm_page_alloc_init(struct ttm_mem_global *glob, unsigned max_pages)
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> index a076ff3..80b10aa 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> @@ -93,6 +93,8 @@ enum pool_type {
> * @size: Size used during DMA allocation.
> * @npages_free: Count of available pages for re-use.
> * @npages_in_use: Count of pages that are in use.
> + * @cur_max_size: Current maximum size for the pool.
> + * @shrink_timeout: Timeout for pool maximum size restriction.
> * @nfrees: Stats when pool is shrinking.
> * @nrefills: Stats when the pool is grown.
> * @gfp_flags: Flags to pass for alloc_page.
> @@ -110,6 +112,8 @@ struct dma_pool {
> unsigned size;
> unsigned npages_free;
> unsigned npages_in_use;
> + unsigned cur_max_size;
> + unsigned long last_shrink;
> unsigned long nfrees; /* Stats when shrunk. */
> unsigned long nrefills; /* Stats when grown. */
> gfp_t gfp_flags;
> @@ -331,6 +335,17 @@ static void __ttm_dma_free_page(struct dma_pool *pool,
> struct dma_page *d_page)
> kfree(d_page);
> d_page = NULL;
> }
> +
> +static inline void ttm_dma_pool_update_max_size(struct dma_pool *pool)
> +{
> + if (time_before(jiffies, pool->shrink_timeout))
> + return;
> + /* In case we reached zero bounce back to 512 pages. */
> + pool->cur_max_size = max(pool->cur_max_size << 1, 512);
> + pool->cur_max_size = min(pool->cur_max_size,
> + _manager->options.max_size);
> +}
> +
> static struct dma_page *__ttm_dma_alloc_page(struct dma_pool *pool)
> {
> struct dma_page *d_page;
> @@ -606,6 +621,8 @@ static struct dma_pool *ttm_dma_pool_init(struct device
> *dev, gfp_t flags,
> pool->size = PAGE_SIZE;
> pool->type = type;
> pool->nrefills = 0;
> + pool->cur_max_size = _manager->options.max_size;
> + pool->shrink_timeout = jiffies;
> p = pool->name;
> for (i = 0; i < 5; i++) {
> if (type & t[i]) {
> @@ -892,6 +909,9 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct
> device *dev)
> }
> }
>
> + /* Update pool size in case shrinker limited it. */
> + ttm_dma_pool_update_max_size(pool);
> +
> INIT_LIST_HEAD(&ttm_dma->pages_list);
> for (i = 0; i < ttm->num_pages; ++i) {
> ret = ttm_dma_pool_get_pages(pool, ttm_dma, i);
> @@ -953,9 +973,9 @@ void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma,
> struct device *dev)
> } else {
> pool->npages_free += count;
> list_splice(&ttm_dma->pages_list, &pool->free_list);
> - if (pool->npages_free >= (_manager->options.max_size +
> + if (pool->npages_free >= (pool->cur_max_size +
> NUM_PAGES_TO_ALLOC))
> - npages = pool->npages_free - _manager->options.max_size;
> + npages = pool->npages_free - pool->cur_max_size;
> }
> spin_unlock_irqrestore(&pool->lock, irq_flags);
>
> @@ -1024,6 +1044,9 @@ ttm_dma_pool_shrink_scan(struct shrinker *shrink,
> struct shrink_control *sc)
> /* Do it in round-robin fashion. */
> if (++idx < pool_offset)
> continue;
> + /* No matter what make sure the pool do not grow in next
> second. */
> + p->pool->cur_max_size = p->pool->cur_max_size >> 1;
> + p->pool->shrink_timeout = jiffies + HZ;
> nr_free = shrink_pages;
> shrink_pages = ttm_dma_page_pool_free(p->pool, nr_free,
> sc->gfp_mask);
