[PATCH 3/3] drm/ttm: under memory pressure minimize the size of memory pool
Daniel Vetter
daniel at ffwll.ch
Wed Aug 13 03:42:46 PDT 2014
On Wed, Aug 13, 2014 at 11:06:25AM +0200, Thomas Hellstrom wrote:
>
> 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.
Yeah, artificially limiting your cache from growing when your shrinker
gets called will just break the equal-memory pressure the core mm uses to
rebalance between all caches when workload changes. In i915 we let
everything grow without artificial bounds and only rely upon the shrinker
callbacks to ensure we don't consume more than our fair share of available
memory overall.
-Daniel
>
> /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);
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the dri-devel
mailing list