From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH] drm/i915: Borrow our struct_mutex for the direct reclaim Date: Thu, 8 Nov 2012 13:49:09 +0000 Message-ID: <20121108134909.345d14e3@bwidawsk.net> References: <1349867250-22808-1-git-send-email-chris@chris-wilson.co.uk> <1349881739-32277-1-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from shiva.chad-versace.us (209-20-75-48.static.cloud-ips.com [209.20.75.48]) by gabe.freedesktop.org (Postfix) with ESMTP id 1F33FA0A04 for ; Thu, 8 Nov 2012 05:48:50 -0800 (PST) In-Reply-To: <1349881739-32277-1-git-send-email-chris@chris-wilson.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, 10 Oct 2012 16:08:59 +0100 Chris Wilson wrote: > If we have hit oom whilst holding our struct_mutex, then currently we > cannot reap our own GPU buffers which likely pin most of memory, making > an outright OOM more likely. So if we are running in direct reclaim and > already hold the mutex, attempt to free buffers knowing that the > original function can not continue until we return. > > Signed-off-by: Chris Wilson I will try to review this a bit further on the plane ride home. It's quite a scary patch, but it fixes OOM destroying IGT runs, so it's pretty necessary IMO. Meanwhile: Tested-by: Ben Widawsky > --- > drivers/gpu/drm/i915/i915_gem.c | 24 +++++++++++++++++++++--- > 1 file changed, 21 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 1d0cbfb..bed4084 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4589,6 +4589,18 @@ void i915_gem_release(struct drm_device *dev, struct drm_file *file) > spin_unlock(&file_priv->mm.lock); > } > > +static bool mutex_is_locked_by(struct mutex *mutex, struct task_struct *task) > +{ > + if (!mutex_is_locked(mutex)) > + return false; > + > +#if defined(CONFIG_DEBUG_MUTEXES) || defined(CONFIG_SMP) > + return mutex->owner == task; > +#else > + return false; > +#endif > +} > + > static int > i915_gem_inactive_shrink(struct shrinker *shrinker, struct shrink_control *sc) > { > @@ -4599,10 +4611,15 @@ i915_gem_inactive_shrink(struct shrinker *shrinker, struct shrink_control *sc) > struct drm_device *dev = dev_priv->dev; > struct drm_i915_gem_object *obj; > int nr_to_scan = sc->nr_to_scan; > + bool unlock = true; > int cnt; > > - if (!mutex_trylock(&dev->struct_mutex)) > - return 0; > + if (!mutex_trylock(&dev->struct_mutex)) { > + if (mutex_is_locked_by(&dev->struct_mutex, current)) > + unlock = false; > + else > + return 0; > + } > > if (nr_to_scan) { > nr_to_scan -= i915_gem_purge(dev_priv, nr_to_scan); > @@ -4618,6 +4635,7 @@ i915_gem_inactive_shrink(struct shrinker *shrinker, struct shrink_control *sc) > if (obj->pin_count == 0 && obj->pages_pin_count == 0) > cnt += obj->base.size >> PAGE_SHIFT; > > - mutex_unlock(&dev->struct_mutex); > + if (unlock) > + mutex_unlock(&dev->struct_mutex); > return cnt; > } -- Ben Widawsky, Intel Open Source Technology Center