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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 25 Aug 2016 21:36:20 +0200
From:   Daniel Vetter <daniel.vetter@...ll.ch>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Waiman Long <waiman.long@....com>,
        Jason Low <jason.low2@....com>,
        Ding Tianhong <dingtianhong@...wei.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Will Deacon <Will.Deacon@....com>,
        Ingo Molnar <mingo@...hat.com>,
        Imre Deak <imre.deak@...el.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Davidlohr Bueso <dave@...olabs.net>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        Terry Rudd <terry.rudd@....com>,
        "Paul E. McKenney" <paulmck@...ibm.com>,
        Jason Low <jason.low2@...com>,
        Chris Wilson <chris@...is-wilson.co.uk>
Subject: Re: [RFC][PATCH -v2 1/4] locking/drm/i915: Kill mutex trickery

On Thu, Aug 25, 2016 at 8:37 PM, Peter Zijlstra <peterz@...radead.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@...is-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@...ll.ch>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.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