[PATCH powerpc] fix a lockdep complaint in start_secondary
From: Li Zhong
Date: Thu May 17 2012 - 00:02:27 EST
This patch tries to fix following lockdep complaints:
[ 81.882506] =================================
[ 81.882508] [ INFO: inconsistent lock state ]
[ 81.882511] 3.4.0-rc4-autokern1 #1 Not tainted
[ 81.882513] ---------------------------------
[ 81.882516] inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage.
[ 81.882519] swapper/5/0 [HC0[0]:SC0[0]:HE1:SE1] takes:
[ 81.882522] (call_function.lock){?.....}, at:
[<c0000000000fdaa0>] .ipi_call_lock+0x20/0x40
[ 81.882536] {IN-HARDIRQ-W} state was registered at:
[ 81.882538] [<c0000000000f5a9c>] .__lock_acquire+0x44c/0x9e0
[ 81.882543] [<c0000000000f60f4>] .lock_acquire+0xc4/0x260
[ 81.882548] [<c00000000063f648>] ._raw_spin_lock+0x48/0x70
[ 81.882554]
[<c0000000000fede4>] .generic_smp_call_function_interrupt+0x1d4/0x320
[ 81.882559] [<c000000000037020>] .smp_ipi_demux+0x90/0x100
[ 81.882565] [<c00000000004f98c>] .icp_hv_ipi_action+0x5c/0xc0
[ 81.882571] [<c00000000013420c>] .handle_irq_event_percpu
+0xec/0x570
[ 81.882577] [<c000000000138ab4>] .handle_percpu_irq+0x84/0xd0
[ 81.882582] [<c0000000000221b4>] .call_handle_irq+0x1c/0x2c
[ 81.882588] [<c0000000000103fc>] .do_IRQ+0x16c/0x500
[ 81.882593] [<c0000000000038d0>] hardware_interrupt_common
+0x150/0x180
[ 81.882598] [<c000000000010a38>] .arch_local_irq_restore+0x38/0x90
[ 81.882603] [<c000000000017450>] .cpu_idle+0x250/0x2d0
[ 81.882607] [<c000000000651ce0>] .start_secondary+0x378/0x384
[ 81.882613] [<c00000000000936c>] .start_secondary_prolog+0x10/0x14
[ 81.882618] irq event stamp: 332475
[ 81.882620] hardirqs last enabled at (332475):
[<c000000000640414>] ._raw_spin_unlock_irqrestore+0x94/0xc0
[ 81.882625] hardirqs last disabled at (332474):
[<c00000000063f7c0>] ._raw_spin_lock_irqsave+0x30/0x90
[ 81.882631] softirqs last enabled at (332288):
[<c0000000000873c4>] .irq_enter+0x94/0xd0
[ 81.882636] softirqs last disabled at (332287):
[<c0000000000873b4>] .irq_enter+0x84/0xd0
[ 81.882640]
[ 81.882641] other info that might help us debug this:
[ 81.882644] Possible unsafe locking scenario:
[ 81.882645]
[ 81.882647] CPU0
[ 81.882649] ----
[ 81.882650] lock(call_function.lock);
[ 81.882654] <Interrupt>
[ 81.882656] lock(call_function.lock);
[ 81.882660]
[ 81.882661] *** DEADLOCK ***
[ 81.882662]
[ 81.882664] no locks held by swapper/5/0.
[ 81.882666]
[ 81.882667] stack backtrace:
[ 81.882669] Call Trace:
[ 81.882672] [c0000003c07bf860] [c0000000000146f4] .show_stack
+0x74/0x1c0 (unreliable)
[ 81.882678] [c0000003c07bf910] [c0000000000f1304] .print_usage_bug
+0x1e4/0x230
[ 81.882683] [c0000003c07bf9d0] [c0000000000f150c] .mark_lock_irq
+0x1bc/0x3c0
[ 81.882688] [c0000003c07bfa90] [c0000000000f18a0] .mark_lock
+0x190/0x4b0
[ 81.882693] [c0000003c07bfb40] [c0000000000f1d10] .mark_irqflags
+0x150/0x240
[ 81.882697] [c0000003c07bfbd0] [c0000000000f5a9c] .__lock_acquire
+0x44c/0x9e0
[ 81.882702] [c0000003c07bfce0] [c0000000000f60f4] .lock_acquire
+0xc4/0x260
[ 81.882707] [c0000003c07bfdc0] [c00000000063f648] ._raw_spin_lock
+0x48/0x70
[ 81.882712] [c0000003c07bfe50] [c0000000000fdaa0] .ipi_call_lock
+0x20/0x40
[ 81.882717] [c0000003c07bfed0] [c000000000651aa0] .start_secondary
+0x138/0x384
[ 81.882722] [c0000003c07bff90]
[c00000000000936c] .start_secondary_prolog+0x10/0x14
>From the log, ipi_call_lock() is called in start_secondary() with irqs
enabled. The irqs are enabled by smp_ops->setup_cpu(), in following call
chain:
start_secondary --> smp_ops->setup_cpu --> smp_xics_setup_cpu -->
pseries_notify_cpu_idle_add_cpu --> cpuidle_disable_device -->
cpuidle_remove_state_sysfs --> cpuidle_free_state_kobj -->
wait_for_completion --> wait_for_common
>From my understanding of the codes, I think it's not necessary to call
pseries_notify_cpu_idle_add_cpu() in the early start_secondary()
function before irqs could be enabled.
pseries_notify_cpu_idle_add_cpu() actually does
cpuidle_disable_device(), and then cpuidle_enable_device(), which
releases and allocates the resources respectively. ( Also, all the data
are cleared and reinitialized after this cycle). The problem here is:
something like kzalloc(GFP_KERNEL), wait_for_completion() would have
problems running here where irqs are still disabled.
Actually, cpuidle_enable_device() is called for each possible cpu when
the driver is registered. So I don't think the resources needed to be
released and allocated each time cpu becomes online. Something like
cpuidle_reset_device() would be enough to clear and reinitialize the
data.
However, after some studying of the data to be cleared, I think it's
also reasonable to keep the previous data. For example:
/sys/devices/system/cpu/cpu#/cpuidle/state#/usage
the number of times this idle state has been entered
/sys/devices/system/cpu/cpu#/cpuidle/state#/time
the amount of time spent in this idle state
So I think we could just remove the function call doing the
disable/enable cycle:
Please correct me if I missed anything.
Reported-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Li Zhong <zhong@xxxxxxxxxxxxxxxxxx>
Tested-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
---
arch/powerpc/platforms/pseries/smp.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/smp.c
b/arch/powerpc/platforms/pseries/smp.c
index e16bb8d..71706bc 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -147,7 +147,6 @@ static void __devinit smp_xics_setup_cpu(int cpu)
set_cpu_current_state(cpu, CPU_STATE_ONLINE);
set_default_offline_state(cpu);
#endif
- pseries_notify_cpuidle_add_cpu(cpu);
}
static int __devinit smp_pSeries_kick_cpu(int nr)
--
1.7.5.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/