[Intel-gfx] [PATCH v2 1/5] drm/i915: Implement a framework for batch buffer pools

[Chris Wilson]

On Wed, Jul 09, 2014 at 08:10:47AM -0700, Volkin, Bradley D wrote:
> On Wed, Jul 09, 2014 at 12:37:08AM -0700, Chris Wilson wrote:
> > On Tue, Jul 08, 2014 at 03:26:36PM -0700, bradley.d.volkin at intel.com wrote:
> > > From: Brad Volkin <bradley.d.volkin at intel.com>
> > > 
> > > This adds a small module for managing a pool of batch buffers.
> > > The only current use case is for the command parser, as described
> > > in the kerneldoc in the patch. The code is simple, but separating
> > > it out makes it easier to change the underlying algorithms and to
> > > extend to future use cases should they arise.
> > > 
> > > The interface is simple: init to create an empty pool, fini to
> > > clean it up; get to obtain a new buffer, put to return it to the
> > > pool. Note that all buffers must be returned to the pool before
> > > freeing it.
> > > 
> > > Buffers are purgeable while in the pool, but not explicitly
> > > truncated in order to avoid overhead during execbuf.
> > > 
> > > Locking is currently based on the caller holding the struct_mutex.
> > > We already do that in the places where we will use the batch pool
> > > for the command parser.
> > > 
> > > v2:
> > > - s/BUG_ON/WARN_ON/ for locking assertions
> > > - Remove the cap on pool size
> > > - Switch from alloc/free to init/fini
> > 
> > You are missing the madvise mechanics.
> 
> _get() and _put() set obj->madv, which I thought was enough for what I've
> described (i.e. purgeable while in the pool but not explicitly truncated).
> Is that not enough for the shrinker to truncate as needed? Or do you want
> them explicitly truncated while in the pool? I was worried about the extra
> cost of getting the backing storage back during execbuf.

You need to check that the shrinker hasn't truncated your backing
storage whilst it was marked DONTNEED. It just amounts to checking for
obj->madv == __I915_MADV_PURGED in pool_get() and discarding the cache
entry if it has been truncated.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre