Thread (43 messages) 43 messages, 5 authors, 2019-08-28

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help