[Intel-gfx] [PATCH 4/4] drm/i915: Only free the oldest stale context before allocating

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-07-05 16:18:23)
> 
> On 05/07/2017 15:26, Chris Wilson wrote:
> > Currently, we move all unreferenced contexts to an RCU free list and
> > then onto a worker for eventual reaping. To compensate against this
> > growing into a long list with frequent allocations starving the system
> > of available memory, before we allocate a new context we reap all the
> > stale contexts. This puts all the cost of destroying the context into
> > the next allocator, which is presumably more sensitive to syscall
> > latency and unfair. We can limit the number of contexts being freed by
> > the new allocator to both keep the list trimmed and to allow the
> > allocator to be reasonably fast.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_gem_context.c | 19 +++++++++++++++++--
> >   1 file changed, 17 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> > index 4f6773ea7d85..1a87d04e7937 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > @@ -201,6 +201,21 @@ static void contexts_free(struct drm_i915_private *i915)
> >               i915_gem_context_free(ctx);
> >   }
> >   
> > +static void contexts_free_first(struct drm_i915_private *i915)
> > +{
> > +     struct i915_gem_context *ctx;
> > +     struct llist_node *freed;
> > +
> > +     lockdep_assert_held(&i915->drm.struct_mutex);
> > +
> > +     freed = llist_del_first(&i915->contexts.free_list);
> > +     if (!freed)
> > +             return;
> > +
> > +     ctx = container_of(freed, typeof(*ctx), free_link);
> > +     i915_gem_context_free(ctx);
> > +}
> > +
> >   static void contexts_free_worker(struct work_struct *work)
> >   {
> >       struct drm_i915_private *i915 =
> > @@ -383,8 +398,8 @@ i915_gem_create_context(struct drm_i915_private *dev_priv,
> >   
> >       lockdep_assert_held(&dev_priv->drm.struct_mutex);
> >   
> > -     /* Reap stale contexts */
> > -     contexts_free(dev_priv);
> > +     /* Reap the most stale context */
> > +     contexts_free_first(dev_priv);
> >   
> >       ctx = __create_hw_context(dev_priv, file_priv);
> >       if (IS_ERR(ctx))
> > 
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Pushed this set as after looking at the report from many-magics, it
should not be related to context recreation.

Thanks for the review and suggestions.
-Chris