[Intel-gfx] [PATCH] drm/i915: Prefer to reschedule the free_object worker rather than block

[Chris Wilson]

On Wed, Mar 29, 2017 at 11:31:28AM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > Avoid blocking the kworker by putting back the freed object list if we
> > cannot immediately take the mutex. We will try again shortly, and flush
> > the work when desperate.
> >
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=100434
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index ab77e38ec264..c2e5cb529b0f 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -4200,7 +4200,19 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
> >  {
> >  	struct drm_i915_gem_object *obj, *on;
> >  
> > -	mutex_lock(&i915->drm.struct_mutex);
> > +	if (!mutex_trylock(&i915->drm.struct_mutex)) {
> > +		/* If we fail to acquire the struct_mutex, put back the
> > +		 * freed list and we will try again in the future. By
> > +		 * rescheduling the task we prevent us from blocking
> > +		 * the worker indefinitely on a prolonged wait for
> > +		 * struct_mutex.
> 
> I don't understand the last part of the comment. If we don't
> want a prolonged block due to mutex, should we limit the amount
> of work we do here, inside the mutex. By limiting how much we
> free per aquiring the lock?

Possibly, but if we spend 20s inside here we get NMI lockup warnings
from this function. On the other hand, we regulary bug out and hold the
struct_mutex indefinitely -- if we wait on the stuck mutex here, we tie
up the kworker.

The worst part here is that we requeue the work for immediate execute.
If the worker is otherwise idle, it will spin. That's not great. (It
works for our other workers as they are on a delay, the delay here is
that we wait for an rcu tick before queuing the work in the first place.)

I like the idea of letting the kworker get on with something else
instead of blocking, but not yet happy.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre