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

[Eric Anholt]

Boris Brezillon <boris.brezillon at free-electrons.com> writes:

> 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.

Hmm.  The downside I see to this plan is if we eventually decide to have
the purge operation not clear all the BOs, then we would probably rather
have userspace freeing objects that had been purged until it finds one
in the cache that hadn't been purged, rather than forcing reallocation
of this BO now (and possibly then purging something from elsewhere in
the cache).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170927/fc11600b/attachment-0001.sig>