[Mesa-dev] [PATCH v2] vc4: Mark BOs as purgeable when they enter the BO cache

[Boris Brezillon]

On Thu, 05 Oct 2017 11:25:46 -0700
Eric Anholt <eric at anholt.net> wrote:

> Boris Brezillon <boris.brezillon at free-electrons.com> writes:
> 
> > This patch makes use of the DRM_IOCTL_VC4_GEM_MADVISE ioctl to mark all
> > BOs placed in the mesa BO cache as purgeable so that the system can
> > reclaim this memory under memory pressure.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon at free-electrons.com>
> > ---
> > Hello,
> >
> > Note that this series depends on kernel code that has not been accepted
> > yet and is just provided to show reviewers how the ioctl can be used
> > and what to expect from it.
> >
> > Please do not consider this for inclusion in MESA until the kernel part
> > has been accepted.
> >
> > Thanks,
> >
> > Boris
> > ---  
> 
> 
> >  static struct vc4_bo *
> >  vc4_bo_from_cache(struct vc4_screen *screen, uint32_t size, const char *name)
> >  {
> >          struct vc4_bo_cache *cache = &screen->bo_cache;
> >          uint32_t page_index = size / 4096 - 1;
> > +        struct vc4_bo *iter, *tmp, *bo = NULL;
> >  
> >          if (cache->size_list_size <= page_index)
> >                  return NULL;
> >  
> > -        struct vc4_bo *bo = NULL;
> >          mtx_lock(&cache->lock);
> > -        if (!list_empty(&cache->size_list[page_index])) {
> > -                bo = LIST_ENTRY(struct vc4_bo, cache->size_list[page_index].next,
> > -                                size_list);
> > -
> > -                /* Check that the BO has gone idle.  If not, then we want to
> > -                 * allocate something new instead, since we assume that the
> > -                 * user will proceed to CPU map it and fill it with stuff.
> > +	LIST_FOR_EACH_ENTRY_SAFE(iter, tmp, &cache->size_list[page_index],
> > +				 size_list) {
> > +                /* Check that the BO has gone idle. If not, then we try the
> > +                 * next one in the list, and if none of them are idle then
> > +                 * we want to allocate something new instead, since we assume
> > +                 * that the user will proceed to CPU map it and fill it with
> > +                 * stuff.
> >                   */
> > -                if (!vc4_bo_wait(bo, 0, NULL)) {
> > -                        mtx_unlock(&cache->lock);
> > -                        return NULL;
> > -                }
> > -
> > +                if (!vc4_bo_wait(iter, 0, NULL))
> > +                        continue;  
> 
> Since things get pushed onto the list in the order they will become
> available, we can just break when we get a busy one.

Makes sense.

> 
> Other than that, and needing a re-import of vc4_drm.h in
> include/drm-uapi (see README), this patch is:

It's already done [1], I just didn't post the patch to avoid polluting
the ML with something that is not definitive yet (the kernel header
might change after your review ;-)).

> 
> Reviewed-by: Eric Anholt <eric at anholt.net>
> 
> I don't think I'll get to review of the kernel side today -- it'll take
> a bit more concentration than I have right now.
> 
> > +
> > +                if (!vc4_bo_unpurgeable(iter)) {
> > +                        /* The BO has been purged. Free it and try to find
> > +                         * another one in the cache.
> > +                         */
> > +                        vc4_bo_remove_from_cache(cache, iter);
> > +                        vc4_bo_free(iter);
> > +                        continue;
> > +		}
> > +
> > +                bo = iter;
> >                  pipe_reference_init(&bo->reference, 1);
> >                  vc4_bo_remove_from_cache(cache, bo);
> >  
> >                  bo->name = name;
> > +                break;
> >          }
> >          mtx_unlock(&cache->lock);
> >          return bo;  

[1]https://github.com/bbrezillon/mesa/commit/e48157273ab90d34fdeb4b1324077d63067e94bd