Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1470333 > unrolled thread

[RFC][PATCH -v2 1/4] locking/drm/i915: Kill mutex trickery

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFC][PATCH -v2 1/4] locking/drm/i915: Kill mutex trickery Peter Zijlstra <peterz@infradead.org> - 2016-08-25 20:50 +0200
    Re: [RFC][PATCH -v2 1/4] locking/drm/i915: Kill mutex trickery Daniel Vetter <daniel.vetter@ffwll.ch> - 2016-08-25 21:40 +0200

#1470333 — [RFC][PATCH -v2 1/4] locking/drm/i915: Kill mutex trickery

Poking at lock internals is not cool. Since I'm going to change the
implementation this will break, take it out.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 drivers/gpu/drm/i915/i915_gem_shrinker.c |   26 +++-----------------------
 1 file changed, 3 insertions(+), 23 deletions(-)

--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -35,19 +35,6 @@
 #include "i915_drv.h"
 #include "i915_trace.h"
 
-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_MUTEX_SPIN_ON_OWNER)
-	return mutex->owner == task;
-#else
-	/* Since UP may be pre-empted, we cannot assume that we own the lock */
-	return false;
-#endif
-}
-
 static int num_vma_bound(struct drm_i915_gem_object *obj)
 {
 	struct i915_vma *vma;
@@ -238,17 +225,10 @@ unsigned long i915_gem_shrink_all(struct
 
 static bool i915_gem_shrinker_lock(struct drm_device *dev, bool *unlock)
 {
-	if (!mutex_trylock(&dev->struct_mutex)) {
-		if (!mutex_is_locked_by(&dev->struct_mutex, current))
-			return false;
-
-		if (to_i915(dev)->mm.shrinker_no_lock_stealing)
-			return false;
-
-		*unlock = false;
-	} else
-		*unlock = true;
+	if (!mutex_trylock(&dev->struct_mutex))
+		return false;
 
+	*unlock = true;
 	return true;
 }
 

[toc] | [next] | [standalone]

#1470355

On Thu, Aug 25, 2016 at 8:37 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> Poking at lock internals is not cool. Since I'm going to change the
> implementation this will break, take it out.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

It's horrible, but we die without this in spurious oom. And we haven't
made much progress in recent years to throw out the locking scheme and
replace it by something else that doesn't need a recursive mutex.

But initerim I guess we could set our own owner field and check that
to keep the duct-tape from getting off completely.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem_shrinker.c |   26 +++-----------------------
>  1 file changed, 3 insertions(+), 23 deletions(-)
>
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -35,19 +35,6 @@
>  #include "i915_drv.h"
>  #include "i915_trace.h"
>
> -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_MUTEX_SPIN_ON_OWNER)
> -       return mutex->owner == task;
> -#else
> -       /* Since UP may be pre-empted, we cannot assume that we own the lock */
> -       return false;
> -#endif
> -}
> -
>  static int num_vma_bound(struct drm_i915_gem_object *obj)
>  {
>         struct i915_vma *vma;
> @@ -238,17 +225,10 @@ unsigned long i915_gem_shrink_all(struct
>
>  static bool i915_gem_shrinker_lock(struct drm_device *dev, bool *unlock)
>  {
> -       if (!mutex_trylock(&dev->struct_mutex)) {
> -               if (!mutex_is_locked_by(&dev->struct_mutex, current))
> -                       return false;
> -
> -               if (to_i915(dev)->mm.shrinker_no_lock_stealing)
> -                       return false;
> -
> -               *unlock = false;
> -       } else
> -               *unlock = true;
> +       if (!mutex_trylock(&dev->struct_mutex))
> +               return false;
>
> +       *unlock = true;
>         return true;
>  }
>
>
>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web