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 <[email protected]> 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 <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]> 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 */
