Re: [PATCH powerpc] fix a lockdep complaint in start_secondary

On 05/17/2012 09:58 AM, Benjamin Herrenschmidt wrote:

On Thu, 2012-05-17 at 12:01 +0800, Li Zhong wrote:

quoted

This patch tries to fix following lockdep complaints:

 .../...

quoted

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. 

This is true when the system is booting up.

So yes, it looks definitely fishy. I don't have time to study cpuidle
today to check whether that's correct or not so I'm CCing Deepthi
Dharwar who did all that cpuidle work for pseries.

Deepthi, can you check whether that patch is correct ?

pseries_notify_cpu_idle_add_cpu() is essential to be called for
hotplug event. So by removing this call completely wouldn't
support cpus registering under cpuidle on hotplug and default idle is
executed on those with do not give much powersavings. Ideal way it to
have a notifier in pseries backend driver for hotplug notification and
then remove this function from here.
I am currently working on this patch, will post it out soon.

quoted

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.

If removed, this would not handle cpu hotplug events for cpuidle.

 

quoted

Reported-by: Paul E. McKenney <redacted>
Signed-off-by: Li Zhong <redacted>
Tested-by: Paul E. McKenney <redacted>
---
 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)

Cheers,
Deepthi