On 11/08/21 15:52, Sebastian Andrzej Siewior wrote:
> On 2021-08-11 14:14:05 [+0100], Valentin Schneider wrote:
>> Booting a recent PREEMPT_RT kernel (v5.14-rc5-rt8 with the previous version
>> of this fix reverted) on my arm4 Juno leads to the idle task blocking on a
>> sleeping spinlock down some notifier path:
>>
>> [ 5.163034] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:35
[...]
>> [ 5.163294] __secondary_switched (arch/arm64/kernel/head.S:661)
>
> I would shrink that part above. The important part is that the CPU-idle
> code runs with disabled interrupts. Then cpu_pm_notify_robust() invokes
> the notifier which requires to acquire the spinlock_t. On PREEMPT_RT the
> spinlock_t becomes a sleeping spinlock and must not be acquired with
> disabled interrupts.
Noted, I'll pluck the warning out.
>> +/*
>> + * atomic_notifiers use a regular spinlock, but notifications for this chain
>> + * will be issued by the idle task which cannot block.
>
> Maybe + a few details and make it more explicit
>
> * atomic_notifiers use a spinlock_t, but notifications for this chain
> * will be issued by the idle task with disabled interrupts which cannot
> * block on PREEMPT_RT.
>
> ?
>
More generally I'd say the idle task is never preemptible (as in
preempt_count > 0 at all times), so any notification issued by the idle
task itself cannot block. The fact those are also issued in an IRQ-off
region just further cements that.
> …
>> @@ -33,10 +45,13 @@ static int cpu_pm_notify(enum cpu_pm_event event)
>>
>> static int cpu_pm_notify_robust(enum cpu_pm_event event_up, enum cpu_pm_event event_down)
>> {
>> + unsigned long flags;
>> int ret;
>>
>> rcu_irq_enter_irqson();
>> - ret = atomic_notifier_call_chain_robust(&cpu_pm_notifier_chain, event_up, event_down, NULL);
>
> could we get rid of atomic_notifier_call_chain_robust() now that we have
> zero users?
>
No objections from my end, I'll add that in v3 and see if anyone complains.
>> + raw_spin_lock_irqsave(&cpu_pm_notifier.lock, flags);
>> + ret = raw_notifier_call_chain_robust(&cpu_pm_notifier.chain, event_up, event_down, NULL);
>> + raw_spin_unlock_irqrestore(&cpu_pm_notifier.lock, flags);
>> rcu_irq_exit_irqson();
>>
>> return notifier_to_errno(ret);
>
> Sebastian
