* Peter Zijlstra <pet...@infradead.org> [2009-08-28 09:01:12]: > On Fri, 2009-08-28 at 08:48 +0200, Peter Zijlstra wrote: > > > > > void cpuidle_install_idle_handler(void) > > > { > > > ......... > > > ......... > > > cpuidle_pm_idle = cpuidle_idle_call; > > > } > > > > All I'm seeing here is a frigging mess. > > > > How on earths can something called: cpuidle_install_idle_handler() have > > a void argument, _WHAT_ handler is it going to install? > > Argh, now I see, it installs itself as the platform idle handler. > > so cpuidle_install_idle_handler() pokes at the unmanaged pm_idle pointer > to make cpuidle take control. > > On module load it does: > > pm_idle_old = pm_idle; > > then in the actual idle loop it does: > > if (!dev || !dev->enabled) { > if (pm_idle_old) > pm_idle_old(); > > who is to say that the pointer stored at module init time is still > around at that time? > > So cpuidle recognised the pm_idle stuff was a flaky, but instead of > fixing it, they build a whole new layer on top of it. Brilliant. > > /me goes mark this whole thread read, I've got enough things to do.
Hi Peter, I understand that you are frustrated with the mess. We are willing to clean up the pm_idle pointer at the moment before the cpuidle framework is exploited my more archs. At this moment we need your suggestions on what interface should we call 'clean' and safe. cpuidle.c and the arch independent cpuidle subsystem is not a module and its cpuidle_idle_call() routine is valid and can be safely called from arch dependent process.c The fragile part is how cpuidle_idle_call() is hooked onto arch specific cpu_idle() function at runtime. x86 has the pm_idle pointer exported while powerpc has ppc_md.power_save pointer being called. At cpuidle init time we can override the platform idle function, but that will mean we are including arch specific code in cpuidle.c Do you think having an exported function in cpuidle.c to give us the correct pointer to arch code be better than the current situation: in drivers/cpuidle/cpuidle.c void (*get_cpuidle_handler(void))(void) { return cpuidle_pm_idle; } EXPORT_SYMBOL(get_cpuidle_handler); and from pseries/processor_idle.c, ppc_md.power_save = get_cpuidle_handler(); --Vaidy _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev