From patchwork Tue Apr 2 13:30:58 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 2384601 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork2.kernel.org (Postfix) with ESMTP id 6A982DFB79 for ; Wed, 3 Apr 2013 05:40:46 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 602FCE5FEB for ; Tue, 2 Apr 2013 22:40:46 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org X-Greylist: delayed 2181 seconds by postgrey-1.32 at gabe; Tue, 02 Apr 2013 07:07:26 PDT Received: from custos.ou.linutronix.de (custos.ou.linutronix.de [212.62.202.73]) by gabe.freedesktop.org (Postfix) with ESMTP id 3DE7AE632F; Tue, 2 Apr 2013 07:07:26 -0700 (PDT) Received: from sandbox.lab.linutronix.de ([10.100.31.94]) by custos.ou.linutronix.de with esmtp (Exim 4.72) (envelope-from ) id 1UN16Q-0005xC-8T; Tue, 02 Apr 2013 15:18:18 +0200 From: Sebastian Andrzej Siewior To: Daniel Vetter Subject: [PATCH] drm/i915: drop the coditional mutex Date: Tue, 2 Apr 2013 15:30:58 +0200 Message-Id: <1364909458-59282-1-git-send-email-bigeasy@linutronix.de> X-Mailer: git-send-email 1.7.6.5 X-Mailman-Approved-At: Tue, 02 Apr 2013 22:33:54 -0700 Cc: intel-gfx@lists.freedesktop.org, Sebastian Andrzej Siewior , tglx@linutronix.de, dri-devel@lists.freedesktop.org X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org mutex_is_locked_by() checks the owner of the lock against current. This is done by accessing a private member of struct mutex which works on mainline but does not on RT. I did not figure out, why this "lock-owner-check" and the "lock stealing flag" is required. If the lock can not be acquire the lock (because it would deadlock) then it can return -1. The lock stealing makes actually no sense to me. If shrinker_no_lock_stealing is true then the same functions (i915_gem_purge(), i915_gem_shrink_all()) are called from i915_gem_object_create_mmap_offset() as from i915_gem_inactive_shrink(). I haven't found a path in which i915_gem_inactive_shrink() is invoked from i915_gem_object_create_mmap_offset() that means there is no way shrinker_no_lock_stealing is true _and_ the lock is owned by the current process. Since I don't see why the i915 needs this hack while all other user do not I recommend to remove this hack and return -1 in case of a dead lock. Here is the patch. Signed-off-by: Sebastian Andrzej Siewior --- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/i915_gem.c | 32 +++----------------------------- 2 files changed, 3 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 01769e2..47f28ee 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -724,7 +724,6 @@ struct i915_gem_mm { struct i915_hw_ppgtt *aliasing_ppgtt; struct shrinker inactive_shrinker; - bool shrinker_no_lock_stealing; /** * List of objects currently involved in rendering. diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 0e207e6..7949517 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1500,8 +1500,6 @@ static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj) if (obj->base.map_list.map) return 0; - dev_priv->mm.shrinker_no_lock_stealing = true; - ret = drm_gem_create_mmap_offset(&obj->base); if (ret != -ENOSPC) goto out; @@ -1521,8 +1519,6 @@ static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj) i915_gem_shrink_all(dev_priv); ret = drm_gem_create_mmap_offset(&obj->base); out: - dev_priv->mm.shrinker_no_lock_stealing = false; - return ret; } @@ -4368,19 +4364,6 @@ 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_SMP) || defined(CONFIG_DEBUG_MUTEXES) - return mutex->owner == task; -#else - /* Since UP may be pre-empted, we cannot assume that we own the lock */ - return false; -#endif -} - static int i915_gem_inactive_shrink(struct shrinker *shrinker, struct shrink_control *sc) { @@ -4391,18 +4374,10 @@ 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)) { - if (!mutex_is_locked_by(&dev->struct_mutex, current)) - return 0; - - if (dev_priv->mm.shrinker_no_lock_stealing) - return 0; - - unlock = false; - } + if (!mutex_trylock(&dev->struct_mutex)) + return -1; if (nr_to_scan) { nr_to_scan -= i915_gem_purge(dev_priv, nr_to_scan); @@ -4421,7 +4396,6 @@ 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; - if (unlock) - mutex_unlock(&dev->struct_mutex); + mutex_unlock(&dev->struct_mutex); return cnt; }