On Mon, Aug 26, 2019 at 12:49:22PM -0500, Scott Wood wrote: > On Mon, 2019-08-26 at 09:29 -0700, Paul E. McKenney wrote: > > On Mon, Aug 26, 2019 at 05:25:23PM +0200, Sebastian Andrzej Siewior wrote: > > > On 2019-08-23 23:10:14 [-0400], Joel Fernandes wrote: > > > > On Fri, Aug 23, 2019 at 02:28:46PM -0500, Scott Wood wrote: > > > > > On Fri, 2019-08-23 at 18:20 +0200, Sebastian Andrzej Siewior wrote: > > > > > > this looks like an ugly hack. This sleeping_lock_inc() is used > > > > > > where we > > > > > > actually hold a sleeping lock and schedule() which is okay. But > > > > > > this > > > > > > would mean we hold a RCU lock and schedule() anyway. Is that okay? > > > > > > > > > > Perhaps the name should be changed, but the concept is the same -- > > > > > RT- > > > > > specific sleeping which should be considered involuntary for the > > > > > purpose of > > > > > debug checks. Voluntary sleeping is not allowed in an RCU critical > > > > > section > > > > > because it will break the critical section on certain flavors of > > > > > RCU, but > > > > > that doesn't apply to the flavor used on RT. Sleeping for a long > > > > > time in an > > > > > RCU critical section would also be a bad thing, but that also > > > > > doesn't apply > > > > > here. > > > > > > > > I think the name should definitely be changed. At best, it is super > > > > confusing to > > > > call it "sleeping_lock" for this scenario. In fact here, you are not > > > > even > > > > blocking on a lock. > > > > > > > > Maybe "sleeping_allowed" or some such. > > > > > > The mechanism that is used here may change in future. I just wanted to > > > make sure that from RCU's side it is okay to schedule here. > > > > Good point. > > > > The effect from RCU's viewpoint will be to split any non-rcu_read_lock() > > RCU read-side critical section at this point. This alrady happens in a > > few places, for example, rcu_note_context_switch() constitutes an RCU > > quiescent state despite being invoked with interrupts disabled (as is > > required!). The __schedule() function just needs to understand (and does > > understand) that the RCU read-side critical section that would otherwise > > span that call to rcu_node_context_switch() is split in two by that call. > > > > However, if this was instead an rcu_read_lock() critical section within > > a PREEMPT=y kernel, then if a schedule() occured within stop_one_task(), > > RCU would consider that critical section to be preempted. This means > > that any RCU grace period that is blocked by this RCU read-side critical > > section would remain blocked until stop_one_cpu() resumed, returned, > > and so on until the matching rcu_read_unlock() was reached. In other > > words, RCU would consider that RCU read-side critical section to span > > the call to stop_one_cpu() even if stop_one_cpu() invoked schedule(). > > > > On the other hand, within a PREEMPT=n kernel, the call to schedule() > > would split even an rcu_read_lock() critical section. Which is why I > > asked earlier if sleeping_lock_inc() and sleeping_lock_dec() are no-ops > > in !PREEMPT_RT_BASE kernels. We would after all want the usual lockdep > > complaints in that case. > > migrate_enable() is PREEMPT_RT_BASE-specific -- this code won't execute at > all with PREEMPT=n. Understood! And yes, that was your answer to my question. Me, I was just answering Sebastian's question. ;-) Thanx, Paul
