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

[Boris Brezillon]

On Wed, 27 Sep 2017 15:24:16 +0100
Chris Wilson <chris at chris-wilson.co.uk> wrote:

> Quoting Boris Brezillon (2017-09-27 15:06:53)
> > On Wed, 27 Sep 2017 14:50:10 +0100
> > Chris Wilson <chris at chris-wilson.co.uk> wrote:
> >   
> > > Quoting Boris Brezillon (2017-09-27 14:45:17)  
> > > >  static struct vc4_bo *
> > > >  vc4_bo_from_cache(struct vc4_screen *screen, uint32_t size, const char *name)
> > > >  {
> > > > @@ -111,6 +121,11 @@ vc4_bo_from_cache(struct vc4_screen *screen, uint32_t size, const char *name)
> > > >                          return NULL;
> > > >                  }
> > > >  
> > > > +                if (vc4_bo_purgeable(bo, false)) {
> > > > +                        mtx_unlock(&cache->lock);
> > > > +                        return NULL;    
> > > 
> > > So this would just mean that the bo was purged in the meantime. Why not
> > > just try to use the next one in the cache or allocate afresh?  
> > 
> > No, this means the BO was purged and the kernel failed to allocate the
> > memory back. We don't care about the retained status here, because we
> > don't need to restore BO's content, that's why we're not checking
> > arg.retained in vc4_bo_purgeable(). Allocating a fresh BO is likely to
> > fail with the same ENOMEM error because both path use the CMA mem.  
> 
> Hmm, you don't treat purging as permanent. But you do track the lose of
> contents, so retained is false?

vc4_bo_purgeable() is not reporting the retained status, it just
reports whether the BO can be used or not. I can change
vc4_bo_purgeable() semantic to return 1 if the BO content was retained,
0 if it was purged and -1 if you the ioctl returned an error (ENOMEM)
if you prefer, but in the end, all I'll check here is
'vc4_bo_purgeable() >= 0' because I don't don't care about the retained
status in this specific use case, all I care about is whether the BO can
be re-used or not (IOW, is there a valid CMA region attached to the BO).

> 
> I took a harder line, and said that userspace should recreate the object
> from scratch after it was purged. I thought that would be easier
> overall. But maybe not.:)

Well, maybe I'm wrong in how I implemented this
DRM_IOCTL_VC4_GEM_MADVISE ioctl, but right now, when the BO has been
purged and someone marks it back as unpurgeable I'm trying to
re-allocate BO's buffer in the ioctl path, and if the CMA allocation
fails I return -ENOMEM. I could move the allocation in the fault
handler, but this would result in pretty much the same behavior except
it would require an extra page-fault to realize the memory is not
available or force us to check the retained status and decide to
release the BO object from the BO cache.

Note that I tried to stay as close as possible to the existing
CMA-based-BO logic where everything is allocated at creation time and
not based on an on-demand allocation, hence the decision to allocate
the CMA region in the ioctl path and not in the page-fault handler.

Regards,

Boris