| Message ID | 1409815075-4180-3-git-send-email-chuansheng.liu@intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 09/04/2014 09:17 AM, Chuansheng Liu wrote: > Currently kick_all_cpus_sync() or smp_call_function() can not > break the polling idle cpu immediately. > > Here using wake_up_all_idle_cpus() which can wake up the polling idle > cpu quickly is much helpful for power. Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org> So IIUC, kick_all_cpus_sync is a broken function, right ? Wouldn't make sense to replace all calls to kick_all_cpus by wake_up_all_idle_cpus ? and remove this broken function ? > Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com> > --- > drivers/cpuidle/cpuidle.c | 9 ++------- > 1 file changed, 2 insertions(+), 7 deletions(-) > > diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c > index ee9df5e..d31e04c 100644 > --- a/drivers/cpuidle/cpuidle.c > +++ b/drivers/cpuidle/cpuidle.c > @@ -223,7 +223,7 @@ void cpuidle_uninstall_idle_handler(void) > { > if (enabled_devices) { > initialized = 0; > - kick_all_cpus_sync(); > + wake_up_all_idle_cpus(); > } > } > > @@ -530,11 +530,6 @@ EXPORT_SYMBOL_GPL(cpuidle_register); > > #ifdef CONFIG_SMP > > -static void smp_callback(void *v) > -{ > - /* we already woke the CPU up, nothing more to do */ > -} > - > /* > * This function gets called when a part of the kernel has a new latency > * requirement. This means we need to get all processors out of their C-state, > @@ -544,7 +539,7 @@ static void smp_callback(void *v) > static int cpuidle_latency_notify(struct notifier_block *b, > unsigned long l, void *v) > { > - smp_call_function(smp_callback, NULL, 1); > + wake_up_all_idle_cpus(); > return NOTIFY_OK; > } > >
> -----Original Message----- > From: Daniel Lezcano [mailto:daniel.lezcano@linaro.org] > Sent: Thursday, September 04, 2014 9:02 PM > To: Liu, Chuansheng; peterz@infradead.org; luto@amacapital.net; > rjw@rjwysocki.net; mingo@redhat.com > Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; Liu, Changcheng; > Wang, Xiaoming; Chakravarty, Souvik K > Subject: Re: [PATCH 3/3] cpuidle: Using the wake_up_all_idle_cpus() to wake up > all idle cpus > > On 09/04/2014 09:17 AM, Chuansheng Liu wrote: > > Currently kick_all_cpus_sync() or smp_call_function() can not > > break the polling idle cpu immediately. > > > > Here using wake_up_all_idle_cpus() which can wake up the polling idle > > cpu quickly is much helpful for power. > > Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org> > > So IIUC, kick_all_cpus_sync is a broken function, right ? > > Wouldn't make sense to replace all calls to kick_all_cpus by > wake_up_all_idle_cpus ? and remove this broken function ? My initial patch(V1) indeed do it, but Andy pointed out some callers of kick_all_cpus_sync() really want the old behavior. My fault again that do not have the detailed changelog for the patches. Pasted the comments from Andy: == > Currently using smp_call_function() just woke up the corresponding > cpu, but can not break the polling idle loop. > > Here using the new sched API wake_up_if_idle() to implement it. kick_all_cpus_sync has other callers, and those other callers want the old behavior. I think this should be a new function. --Andy ==
On Thu, Sep 04, 2014 at 01:39:38PM +0000, Liu, Chuansheng wrote: > > So IIUC, kick_all_cpus_sync is a broken function, right ? > kick_all_cpus_sync has other callers, and those other callers want the > old behavior. I think this should be a new function. Correct, things like arch/powerpc/mm/pgtable_64.c:pmdp_clear_flush() really want the old behaviour. It basically uses local_irq_disable()/local_irq_enable() vs kick_all_cpus_sync() as a RCU like serialization primitive. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index ee9df5e..d31e04c 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -223,7 +223,7 @@ void cpuidle_uninstall_idle_handler(void) { if (enabled_devices) { initialized = 0; - kick_all_cpus_sync(); + wake_up_all_idle_cpus(); } } @@ -530,11 +530,6 @@ EXPORT_SYMBOL_GPL(cpuidle_register); #ifdef CONFIG_SMP -static void smp_callback(void *v) -{ - /* we already woke the CPU up, nothing more to do */ -} - /* * This function gets called when a part of the kernel has a new latency * requirement. This means we need to get all processors out of their C-state, @@ -544,7 +539,7 @@ static void smp_callback(void *v) static int cpuidle_latency_notify(struct notifier_block *b, unsigned long l, void *v) { - smp_call_function(smp_callback, NULL, 1); + wake_up_all_idle_cpus(); return NOTIFY_OK; }
Currently kick_all_cpus_sync() or smp_call_function() can not break the polling idle cpu immediately. Here using wake_up_all_idle_cpus() which can wake up the polling idle cpu quickly is much helpful for power. Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com> --- drivers/cpuidle/cpuidle.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)