Re: [Patch]cpuidle: Save current cpu as local variable instead of calling smp_processor_id() in loop
From: Peter Zijlstra
Date: Wed May 18 2016 - 08:31:00 EST
On Wed, May 18, 2016 at 01:47:21PM +0200, Peter Zijlstra wrote:
>
> Another optimization you could look at is removing that rmb(); I don't
> actually think its needed, but you'd need to find why it was added and
> then check it was in fact needed back then, and then check if it is in
> fact still needed now.
>
> Its a bit of a trek through git history, but lfence / dsb ld are
> expensive instructions.
>
OK; so I was curious ... :-)
the rmb() was introduced by the below commit (found in tglx/history.git).
which at the time ordered the cpu_idle_state vs pm_idle loads.
---
commit f2f1b44c75c478f541d87234c98606d7686c4209
Author: Zwane Mwaikambo <zwane@xxxxxxxxxxxxxxxx>
Date: Tue Jan 4 05:35:06 2005 -0800
[PATCH] Remove RCU abuse in cpu_idle()
Introduce cpu_idle_wait() on architectures requiring modification of
pm_idle from modules, this will ensure that all processors have updated
their cached values of pm_idle upon exit. This patch is to address the bug
report at http://bugme.osdl.org/show_bug.cgi?id=1716 and replaces the
current code fix which is in violation of normal RCU usage as pointed out
by Stephen, Dipankar and Paul.
Signed-off-by: Zwane Mwaikambo <zwane@xxxxxxxxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxx>
Signed-off-by: Linus Torvalds <torvalds@xxxxxxxx>
diff --git a/arch/i386/kernel/apm.c b/arch/i386/kernel/apm.c
index 904217f5361e..47712b5d5814 100644
--- a/arch/i386/kernel/apm.c
+++ b/arch/i386/kernel/apm.c
@@ -2369,7 +2369,7 @@ static void __exit apm_exit(void)
* (pm_idle), Wait for all processors to update cached/local
* copies of pm_idle before proceeding.
*/
- synchronize_kernel();
+ cpu_idle_wait();
}
if (((apm_info.bios.flags & APM_BIOS_DISENGAGED) == 0)
&& (apm_info.connection_version > 0x0100)) {
diff --git a/arch/i386/kernel/process.c b/arch/i386/kernel/process.c
index 7efce8e4c376..7b79a9b6a04f 100644
--- a/arch/i386/kernel/process.c
+++ b/arch/i386/kernel/process.c
@@ -72,6 +72,7 @@ unsigned long thread_saved_pc(struct task_struct *tsk)
* Powermanagement idle function, if any..
*/
void (*pm_idle)(void);
+static cpumask_t cpu_idle_map;
void disable_hlt(void)
{
@@ -93,7 +94,7 @@ EXPORT_SYMBOL(enable_hlt);
*/
void default_idle(void)
{
- if (!hlt_counter && current_cpu_data.hlt_works_ok) {
+ if (!hlt_counter && boot_cpu_data.hlt_works_ok) {
local_irq_disable();
if (!need_resched())
safe_halt();
@@ -144,29 +145,44 @@ static void poll_idle (void)
*/
void cpu_idle (void)
{
+ int cpu = smp_processor_id();
+
/* endless idle loop with no priority at all */
while (1) {
while (!need_resched()) {
void (*idle)(void);
- /*
- * Mark this as an RCU critical section so that
- * synchronize_kernel() in the unload path waits
- * for our completion.
- */
- rcu_read_lock();
+
+ if (cpu_isset(cpu, cpu_idle_map))
+ cpu_clear(cpu, cpu_idle_map);
+ rmb();
idle = pm_idle;
if (!idle)
idle = default_idle;
- irq_stat[smp_processor_id()].idle_timestamp = jiffies;
+ irq_stat[cpu].idle_timestamp = jiffies;
idle();
- rcu_read_unlock();
}
schedule();
}
}
+void cpu_idle_wait(void)
+{
+ int cpu;
+ cpumask_t map;
+
+ for_each_online_cpu(cpu)
+ cpu_set(cpu, cpu_idle_map);
+
+ wmb();
+ do {
+ ssleep(1);
+ cpus_and(map, cpu_idle_map, cpu_online_map);
+ } while (!cpus_empty(map));
+}
+EXPORT_SYMBOL_GPL(cpu_idle_wait);
+
/*
* This uses new MONITOR/MWAIT instructions on P4 processors with PNI,
* which can obviate IPI to trigger checking of need_resched.
diff --git a/arch/ia64/kernel/process.c b/arch/ia64/kernel/process.c
index 078075f7be3f..f96bfb005268 100644
--- a/arch/ia64/kernel/process.c
+++ b/arch/ia64/kernel/process.c
@@ -25,6 +25,7 @@
#include <linux/unistd.h>
#include <linux/efi.h>
#include <linux/interrupt.h>
+#include <linux/delay.h>
#include <asm/cpu.h>
#include <asm/delay.h>
@@ -46,6 +47,7 @@
#include "sigframe.h"
void (*ia64_mark_idle)(int);
+static cpumask_t cpu_idle_map;
unsigned long boot_option_idle_override = 0;
EXPORT_SYMBOL(boot_option_idle_override);
@@ -225,10 +227,28 @@ static inline void play_dead(void)
}
#endif /* CONFIG_HOTPLUG_CPU */
+
+void cpu_idle_wait(void)
+{
+ int cpu;
+ cpumask_t map;
+
+ for_each_online_cpu(cpu)
+ cpu_set(cpu, cpu_idle_map);
+
+ wmb();
+ do {
+ ssleep(1);
+ cpus_and(map, cpu_idle_map, cpu_online_map);
+ } while (!cpus_empty(map));
+}
+EXPORT_SYMBOL_GPL(cpu_idle_wait);
+
void __attribute__((noreturn))
cpu_idle (void *unused)
{
void (*mark_idle)(int) = ia64_mark_idle;
+ int cpu = smp_processor_id();
/* endless idle loop with no priority at all */
while (1) {
@@ -241,17 +261,14 @@ cpu_idle (void *unused)
if (mark_idle)
(*mark_idle)(1);
- /*
- * Mark this as an RCU critical section so that
- * synchronize_kernel() in the unload path waits
- * for our completion.
- */
- rcu_read_lock();
+
+ if (cpu_isset(cpu, cpu_idle_map))
+ cpu_clear(cpu, cpu_idle_map);
+ rmb();
idle = pm_idle;
if (!idle)
idle = default_idle;
(*idle)();
- rcu_read_unlock();
}
if (mark_idle)
diff --git a/arch/x86_64/kernel/process.c b/arch/x86_64/kernel/process.c
index 1ce2e1f9a316..129a20d3888c 100644
--- a/arch/x86_64/kernel/process.c
+++ b/arch/x86_64/kernel/process.c
@@ -61,6 +61,7 @@ EXPORT_SYMBOL(boot_option_idle_override);
* Powermanagement idle function, if any..
*/
void (*pm_idle)(void);
+static cpumask_t cpu_idle_map;
void disable_hlt(void)
{
@@ -123,6 +124,23 @@ static void poll_idle (void)
}
}
+
+void cpu_idle_wait(void)
+{
+ int cpu;
+ cpumask_t map;
+
+ for_each_online_cpu(cpu)
+ cpu_set(cpu, cpu_idle_map);
+
+ wmb();
+ do {
+ ssleep(1);
+ cpus_and(map, cpu_idle_map, cpu_online_map);
+ } while (!cpus_empty(map));
+}
+EXPORT_SYMBOL_GPL(cpu_idle_wait);
+
/*
* The idle thread. There's no useful work to be
* done, so just try to conserve power and have a
@@ -131,21 +149,20 @@ static void poll_idle (void)
*/
void cpu_idle (void)
{
+ int cpu = smp_processor_id();
+
/* endless idle loop with no priority at all */
while (1) {
while (!need_resched()) {
void (*idle)(void);
- /*
- * Mark this as an RCU critical section so that
- * synchronize_kernel() in the unload path waits
- * for our completion.
- */
- rcu_read_lock();
+
+ if (cpu_isset(cpu, cpu_idle_map))
+ cpu_clear(cpu, cpu_idle_map);
+ rmb();
idle = pm_idle;
if (!idle)
idle = default_idle;
idle();
- rcu_read_unlock();
}
schedule();
}
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index d9e4433177d3..47eb7463c009 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -988,7 +988,7 @@ int acpi_processor_power_exit(struct acpi_processor *pr, struct acpi_device *dev
* (pm_idle), Wait for all processors to update cached/local
* copies of pm_idle before proceeding.
*/
- synchronize_kernel();
+ cpu_idle_wait();
}
return_VALUE(0);
diff --git a/include/asm-i386/system.h b/include/asm-i386/system.h
index 9bb0f63ff64e..c705fa77b138 100644
--- a/include/asm-i386/system.h
+++ b/include/asm-i386/system.h
@@ -466,5 +466,6 @@ void disable_hlt(void);
void enable_hlt(void);
extern int es7000_plat;
+void cpu_idle_wait(void);
#endif
diff --git a/include/asm-ia64/system.h b/include/asm-ia64/system.h
index fdefa16fabc4..f28b920e9089 100644
--- a/include/asm-ia64/system.h
+++ b/include/asm-ia64/system.h
@@ -284,6 +284,7 @@ do { \
#define ia64_platform_is(x) (strcmp(x, platform_name) == 0)
+void cpu_idle_wait(void);
#endif /* __KERNEL__ */
#endif /* __ASSEMBLY__ */
diff --git a/include/asm-x86_64/system.h b/include/asm-x86_64/system.h
index 55ac71fd4a60..05acc62c2b61 100644
--- a/include/asm-x86_64/system.h
+++ b/include/asm-x86_64/system.h
@@ -326,6 +326,8 @@ static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
/* For spinlocks etc */
#define local_irq_save(x) do { warn_if_not_ulong(x); __asm__ __volatile__("# local_irq_save \n\t pushfq ; popq %0 ; cli":"=g" (x): /* no input */ :"memory"); } while (0)
+void cpu_idle_wait(void);
+
/*
* disable hlt during certain critical i/o operations
*/