Re: [PATCH -v4 1/8] locking/drm: Kill mutex trickery

[Thomas Gleixner <tglx@linutronix.de>]

On Sat, 8 Oct 2016, Peter Zijlstra wrote:
> On Sat, Oct 08, 2016 at 01:58:07PM +0200, Thomas Gleixner wrote:
> > Hmm. I'm not a great fan of this, because that requires an conditional
> > unlock mechanism.
> > 
> >        res = trylock_recursive(lock);
> >        if (res == FAILED)
> >        	       goto out;
> >        .....
> > 
> >        if (res == SUCCESS)
> >        	       unlock(lock);
> > 
> > While if you actually keep track of recursion you can do:
> >   
> >       if (!trylock_recursive(lock))
> > 		goto out;
> > 
> >       ....
> > 
> >       unlock_recursive(lock);
> > 
> > or even:
> > 
> >      lock_recursive(lock);
> > 
> >      unlock_recursive(lock);
> > 
> > That's making lock/trylock and unlock symetric, so its obvious in the
> > source what's going on and the recursion tracking allows for better
> > debugability.
> 
> Hurm,. so I thought that in general we disliked recursive locking
> because it quickly turns in to a horrible mess.
> 
> Adding such primitives makes it 'easy' to use recursive locking and then
> where does it stop?

Well, when you add just trylock_recursive then people are going to use it
anyway no matter whether it is easy or not.

So if we decide to provide something which supports recursive locking for
mutexes then we are better off doing it with a proper set of functions and
not just a single undebugable wrapper.

Thanks,

	tglx