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 S1756615AbcANRoU (ORCPT <rfc822;w@1wt.eu>);
	Thu, 14 Jan 2016 12:44:20 -0500
Received: from www.linutronix.de ([62.245.132.108]:58827 "EHLO
	Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1756243AbcANRoT (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Thu, 14 Jan 2016 12:44:19 -0500
Date: Thu, 14 Jan 2016 18:43:16 +0100 (CET)
From: Thomas Gleixner <tglx@linutronix.de>
To: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
cc: Sasha Levin <sasha.levin@oracle.com>, LKML <linux-kernel@vger.kernel.org>,
        Peter Zijlstra <peterz@infradead.org>
Subject: Re: timers: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
In-Reply-To: <20160113161608.GN3818@linux.vnet.ibm.com>
Message-ID: <alpine.DEB.2.11.1601141812270.3575@nanos>
References: <56955C0F.1090005@oracle.com> <alpine.DEB.2.11.1601130930170.3575@nanos> <20160113161608.GN3818@linux.vnet.ibm.com>
User-Agent: Alpine 2.11 (DEB 23 2013-08-11)
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
X-Linutronix-Spam-Score: -1.0
X-Linutronix-Spam-Level: -
X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required,  ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, 13 Jan 2016, Paul E. McKenney wrote:
> On Wed, Jan 13, 2016 at 10:05:49AM +0100, Thomas Gleixner wrote:
> > We can fix that particular issue in the posix-timer code by making the
> > locking symetric:
> > 
> > 	rcu_read_lock();
> > 	spin_lock_irq(timer->lock);
> > 
> > ...
> > 
> > 	spin_unlock_irq(timer->lock);
> > 	rcu_read_unlock();
> > 
> > instead of:
> > 
> > 	rcu_read_lock();
> > 	spin_lock_irq(timer->lock);
> > 	rcu_read_unlock();
> > 
> > ...
> > 
> > 	spin_unlock_irq(timer->lock);
> > 
> > But the question is, whether this is the only offending code path in tree. We
> > can avoid the hassle by making rtmutex->wait_lock irq safe.
> > 
> > Thoughts?
> 
> Given that the lock is disabling irq, I don't see a problem with
> extending the RCU read-side critical section to cover the entire
> irq-disabled region.

I cannot follow here. What would be different if the lock would not disable
irqs? I mean you can get preempted right after rcu_read_lock() before
acquiring the spinlock.

> Your point about the hassle of finding and fixing all the other instances of
> this sort is well taken, however.

Right. We have the pattern 

     rcu_read_lock();
     x = lookup();
     if (x)
	   keep_hold(x)
     rcu_read_unlock();
     return x;

all over the place. Now that keep_hold() can be everything from a refcount to
a spinlock and I'm not sure that we can force stuff depending on the mechanism
to be completely symetric. So we are probably better off by making that rcu
unlock machinery more robust.

Thanks,

	tglx