Re: [PATCH v4 2/2] clocksource: Use migrate_disable() to avoid calling get_random_u32() in atomic context

[Sebastian Andrzej Siewior]

On 2025-01-31 12:33:23 [-0500], Waiman Long wrote:
> The following bug report happened with a PREEMPT_RT kernel.
> 
> [   30.957705] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
> [   30.957711] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 2012, name: kwatchdog
> [   30.962673] preempt_count: 1, expected: 0
> [   30.962676] RCU nest depth: 0, expected: 0
> 
> It is due to the fact that clocksource_verify_choose_cpus() is invoked
> with preemption disabled.  This function invokes get_random_u32()
> to obtain random numbers for choosing CPUs.  The batched_entropy_32
> local lock and/or the base_crng.lock spinlock in driver/char/random.c
> will be acquired during the call. In PREEMPT_RT kernel, they are both
> sleeping locks and so cannot be acquired in atomic context.
> 
> Fix this problem by using migrate_disable() to allow
> smp_processor_id() to be reliably used without introducing atomic
> context. The preempt_disable() function is then called after
> clocksource_verify_choose_cpus() but before the clocksource measurement
> is being run to avoid introducing unexpected latency.
> 
> Fixes: 7560c02bdffb ("clocksource: Check per-CPU clock synchronization when marked unstable")
> Suggested-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> Signed-off-by: Waiman Long <longman@xxxxxxxxxx>

Reviewed-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>

I would have moved the preempt-disable into the for_each_cpu() loop. But
given that the clocksource is only updated on boot and by the watchdog
this does not matter at runtime.

Sebastian