Re: [PATCH] sched/core: Schedule new worker even if PI-blocked

From: Sebastian Andrzej Siewior
Date: Tue Aug 20 2019 - 12:14:38 EST


On 2019-08-20 18:02:17 [+0200], Peter Zijlstra wrote:
> On Tue, Aug 20, 2019 at 05:54:01PM +0200, Sebastian Andrzej Siewior wrote:
> > On 2019-08-20 17:20:25 [+0200], Peter Zijlstra wrote:
>
> > > And am I right in thinking that that, again, is specific to the
> > > sleeping-spinlocks from PREEMPT_RT? Is there really nothing else that
> > > identifies those more specifically? It's been a while since I looked at
> > > them.
> >
> > Not really. I hacked "int sleeping_lock" into task_struct which is
> > incremented each time a "sleeping lock" version of rtmutex is requested.
> > We have two users as of now:
> > - RCU, which checks if we schedule() while holding rcu_read_lock() which
> > is okay if it is a sleeping lock.
> >
> > - NOHZ's pending softirq detection while going to idle. It is possible
> > that "ksoftirqd" and "current" are blocked on locks and the CPU goes
> > to idle (because nothing else is runnable) with pending softirqs.
> >
> > I wanted to let rtmutex invoke another schedule() function in case of a
> > sleeping lock to avoid the RCU warning. This would avoid incrementing
> > "sleeping_lock" in the fast path. But then I had no idea what to do with
> > the NOHZ thing.
>
> Once upon a time there was also a shadow task->state thing, that was
> specific to the sleeping locks, because normally spinlocks don't muck
> with task->state and so we have code relying on it not getting trampled.
>
> Can't we use that somewhow? Or is that gone?

we have ->state and ->saved_state. While sleeping on a sleeping lock
->state goes to ->saved_state (usually TASK_RUNNING) and ->state becomes
TASK_UNINTERRUPTIBLE. This is no different compared to regular
blocked-on-I/O wait.
We could add a state, say, TASK_LOCK_BLOCK to identify a task blocking
on sleeping lock. This shouldn't break anything. After all only a
regular "unlock" is allowed to wake such a task and "non-matching" wakes
are redirected to update ->saved_state.

> > > Also, I suppose it would be really good to put that in a comment.
> > So, what does that mean for that patch. According to my inbox it has
> > applied to an "urgent" branch. Do I resubmit the whole thing or just a
> > comment on top?
>
> Yeah, I'm not sure. I was surprised by that, because afaict all this is
> PREEMPT_RT specific and not really /urgent material in the first place.
> Ingo?

Sebastian