On Thu, Apr 09, 2015 at 09:47:36AM -0700, Linus Torvalds wrote: > On Thu, Apr 9, 2015 at 12:53 AM, Ingo Molnar <mi...@kernel.org> wrote: > > > > The point is to generally unify the 'out' paths - i.e. to merge it > > with the rcu_read_unlock() as well, so that we have really simple > > gotos and only a single exit path. > > Maybe just have the rcu read-locking be done in the *caller* (possibly > through using just a helper wrapper function that does nothing but the > locking), so that you can just do a simple "return false" in the > function itself. > > That said, it worries me a bit that we do that spinning while holding > the RCU read lock in the first place. Yes, we stop spinning if > "need_resched()" is set, but what effect - if any - does all of this > have on RCU latency? If somebody is waiting for a RCU grace period, > I'm not seeing that setting need-resched... > > At least with CONFIG_PREEMPT_RCU, the read-unlock is *not* just doing > a preempt-disable, so it's not necessarily just about need_resched(). > It does all the magic with 'rcu_read_unlock_special.s' too.. > > Adding Paul. From a RCU locking standpoint, the thing is basically > (not the real code, edited down): > > rcu_read_lock(); > while (sem->owner == owner) { > if (!owner->on_cpu || need_resched()) > break; > cpu_relax_lowlatency(); > } > rcu_read_unlock(); > > so we busy-loop while holding the RCU read lock while > > sem->owner == owner && owner->on_cpu && !need_resched() > > is true. That is usually not very long, but we've already had > watchdogs go off when we get this wrong, so.. > > Paul, comments? Are there particular latency concerns wrt > CONFIG_PREEMPT_RCU here? Or am I just being silly?
If this was a pure spinlock, then the effects of spinning would overwhelm any problems from extended grace periods. But this is a sleeplock. Of course, we stay in the loop only as long as the lock holder is actually running. But given that this is a sleeplock, I am worried that some lock holders might run for long time periods. After all, that is one of the traditional uses for a sleeplock. :-/ If the RCU read-side critical section lasts a few hundred milliseconds, no problem. If it lasts for more than 500 milliseconds, I would start getting concerned. And if such long-term spins are likely, I cannot resist asking if this should be instead using SRCU. If you have your own srcu_struct, you get to delay your own SRCU grace periods as long as you want. ;-) Thanx, Paul -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/