Re: timers: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected

[Thomas Gleixner]

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