powerpc/pseries/cpuidle: Fix preempt warning
When booting a pseries kernel with PREEMPT enabled, it dumps the following warning: BUG: using smp_processor_id() in preemptible [00000000] code: swapper/0/1 caller is pseries_processor_idle_init+0x5c/0x22c CPU: 13 PID: 1 Comm: swapper/0 Not tainted 4.20.0-rc3-00090-g12201a0128bc-dirty #828 Call Trace: [c000000429437ab0] [c0000000009c8878] dump_stack+0xec/0x164 (unreliable) [c000000429437b00] [c0000000005f2f24] check_preemption_disabled+0x154/0x160 [c000000429437b90] [c000000000cab8e8] pseries_processor_idle_init+0x5c/0x22c [c000000429437c10] [c000000000010ed4] do_one_initcall+0x64/0x300 [c000000429437ce0] [c000000000c54500] kernel_init_freeable+0x3f0/0x500 [c000000429437db0] [c0000000000112dc] kernel_init+0x2c/0x160 [c000000429437e20] [c00000000000c1d0] ret_from_kernel_thread+0x5c/0x6c This happens because the code calls get_lppaca() which calls get_paca() and it checks if preemption is disabled through check_preemption_disabled(). Preemption should be disabled because the per CPU variable may make no sense if there is a preemption (and a CPU switch) after it reads the per CPU data and when it is used. In this device driver specifically, it is not a problem, because this code just needs to have access to one lppaca struct, and it does not matter if it is the current per CPU lppaca struct or not (i.e. when there is a preemption and a CPU migration). That said, the most appropriate fix seems to be related to avoiding the debug_smp_processor_id() call at get_paca(), instead of calling preempt_disable() before get_paca(). Signed-off-by: Breno Leitao <leitao@debian.org> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
This commit is contained in:
parent
8d4a862276
commit
2b038cbc5f
@ -247,7 +247,13 @@ static int pseries_idle_probe(void)
|
||||
return -ENODEV;
|
||||
|
||||
if (firmware_has_feature(FW_FEATURE_SPLPAR)) {
|
||||
if (lppaca_shared_proc(get_lppaca())) {
|
||||
/*
|
||||
* Use local_paca instead of get_lppaca() since
|
||||
* preemption is not disabled, and it is not required in
|
||||
* fact, since lppaca_ptr does not need to be the value
|
||||
* associated to the current CPU, it can be from any CPU.
|
||||
*/
|
||||
if (lppaca_shared_proc(local_paca->lppaca_ptr)) {
|
||||
cpuidle_state_table = shared_states;
|
||||
max_idle_state = ARRAY_SIZE(shared_states);
|
||||
} else {
|
||||
|
Loading…
Reference in New Issue
Block a user