Re: [PATCH RT v2 2/3] sched: migrate_enable: Use sleeping_lock to indicate involuntary sleep
From: Paul E. McKenney <hidden>
Date: 2019-08-26 18:13:05
Also in:
lkml
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:quoted
On Mon, Aug 26, 2019 at 05:25:23PM +0200, Sebastian Andrzej Siewior wrote:quoted
On 2019-08-23 23:10:14 [-0400], Joel Fernandes wrote:quoted
On Fri, Aug 23, 2019 at 02:28:46PM -0500, Scott Wood wrote:quoted
On Fri, 2019-08-23 at 18:20 +0200, Sebastian Andrzej Siewior wrote:quoted
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