From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1756898AbcJHOPT (ORCPT <rfc822;w@1wt.eu>);
        Sat, 8 Oct 2016 10:15:19 -0400
Received: from Galois.linutronix.de ([146.0.238.70]:46392 "EHLO
        Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1756430AbcJHOPL (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Sat, 8 Oct 2016 10:15:11 -0400
Date: Sat, 8 Oct 2016 16:11:25 +0200 (CEST)
From: Thomas Gleixner <tglx@linutronix.de>
To: Peter Zijlstra <peterz@infradead.org>
cc: Linus Torvalds <torvalds@linux-foundation.org>,
        Waiman Long <waiman.long@hpe.com>, Jason Low <jason.low2@hpe.com>,
        Ding Tianhong <dingtianhong@huawei.com>,
        Will Deacon <Will.Deacon@arm.com>, Ingo Molnar <mingo@redhat.com>,
        Imre Deak <imre.deak@intel.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Davidlohr Bueso <dave@stgolabs.net>,
        Tim Chen <tim.c.chen@linux.intel.com>, Terry Rudd <terry.rudd@hpe.com>,
        "Paul E. McKenney" <paulmck@us.ibm.com>,
        Chris Wilson <chris@chris-wilson.co.uk>,
        Daniel Vetter <daniel.vetter@ffwll.ch>,
        Rob Clark <robdclark@gmail.com>
Subject: Re: [PATCH -v4 1/8] locking/drm: Kill mutex trickery
In-Reply-To: <20161008140136.GG3142@twins.programming.kicks-ass.net>
Message-ID: <alpine.DEB.2.20.1610081608390.5222@nanos>
References: <20161007145243.361481786@infradead.org> <20161007150210.927453282@infradead.org> <20161007154351.GL3117@twins.programming.kicks-ass.net> <alpine.DEB.2.20.1610081347170.5222@nanos> <20161008140136.GG3142@twins.programming.kicks-ass.net>
User-Agent: Alpine 2.20 (DEB 67 2015-01-07)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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