Re: [patch V2 08/15] Documentation: Add lock ordering and nesting documentation

Thomas Gleixner
2020-03-20T22:36:39Z

"Paul E. McKenney" <paulmck@kernel.org> writes:
> On Fri, Mar 20, 2020 at 08:51:44PM +0100, Thomas Gleixner wrote:
>> "Paul E. McKenney" <paulmck@kernel.org> writes:
>> >
>> > - The soft interrupt related suffix (_bh()) still disables softirq
>> > handlers. However, unlike non-PREEMPT_RT kernels (which disable
>> > preemption to get this effect), PREEMPT_RT kernels use a per-CPU
>> > lock to exclude softirq handlers.
>>
>> I've made that:
>>
>> - The soft interrupt related suffix (_bh()) still disables softirq
>> handlers.
>>
>> Non-PREEMPT_RT kernels disable preemption to get this effect.
>>
>> PREEMPT_RT kernels use a per-CPU lock for serialization. The lock
>> disables softirq handlers and prevents reentrancy by a preempting
>> task.
>
> That works! At the end, I would instead say "prevents reentrancy
> due to task preemption", but what you have works.

Yours is better.

>> - Task state is preserved across spinlock acquisition, ensuring that the
>> task-state rules apply to all kernel configurations. Non-PREEMPT_RT
>> kernels leave task state untouched. However, PREEMPT_RT must change
>> task state if the task blocks during acquisition. Therefore, it
>> saves the current task state before blocking and the corresponding
>> lock wakeup restores it. A regular not lock related wakeup sets the
>> task state to RUNNING. If this happens while the task is blocked on
>> a spinlock then the saved task state is changed so that correct
>> state is restored on lock wakeup.
>>
>> Hmm?
>
> I of course cannot resist editing the last two sentences:
>
> ... Other types of wakeups unconditionally set task state to RUNNING.
> If this happens while a task is blocked while acquiring a spinlock,
> then the task state is restored to its pre-acquisition value at
> lock-wakeup time.

Errm no. That would mean

state = UNINTERRUPTIBLE
lock()
block()
real_state = state
state = SLEEPONLOCK

non lock wakeup
state = RUNNING <--- FAIL #1

lock wakeup
state = real_state <--- FAIL #2

How it works is:

state = UNINTERRUPTIBLE
lock()
block()
real_state = state
state = SLEEPONLOCK

non lock wakeup
real_state = RUNNING

lock wakeup
state = real_state == RUNNING

If there is no 'non lock wakeup' before the lock wakeup:

state = UNINTERRUPTIBLE
lock()
block()
real_state = state
state = SLEEPONLOCK

lock wakeup
state = real_state == UNINTERRUPTIBLE

I agree that what I tried to express is hard to parse, but it's at least
halfways correct :)

Thanks,

tglx