| Message ID | 20170727120803.27848-1-wens@csie.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 27, 2017 at 08:08:03PM +0800, Chen-Yu Tsai wrote: > On ARM, the kernel busy loops after cleaning up when a core is to be > halted. This may have the undesired effect of increasing the core > temperature, at a time when no power or thermal management is > available, such as a kernel panic or shutdown to halt. > > x86 uses the more efficient HLT (halt) instruction. The equivalent > instruction on ARM is WFI (wait for interrupt). > > This patch makes the busy loops in the ARM halt/reboot/panic paths > use WFI when available (as defined in arch/arm/include/asm/barrier.h). > > A touch test indicates that this lowers the surface temperature of the > Allwinner H3 SoC, with all 4 cores brought up with SMP, from painfully > hot to just warm to the touch after shutdown to halt. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > --- > > I asked about this some time ago, and the answer was no one had done > it. So here it is. It is somewhat similar to the patch "arm64: > Improve parking of stopped CPUs", for arm64 obviously. There are various errata around the "wfi" instruction on various CPUs which means that this simple implementation isn't going to work reliably. This is where cpu_do_idle() comes in - please use that instead.
On Thu, Jul 27, 2017 at 8:17 PM, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Thu, Jul 27, 2017 at 08:08:03PM +0800, Chen-Yu Tsai wrote: >> On ARM, the kernel busy loops after cleaning up when a core is to be >> halted. This may have the undesired effect of increasing the core >> temperature, at a time when no power or thermal management is >> available, such as a kernel panic or shutdown to halt. >> >> x86 uses the more efficient HLT (halt) instruction. The equivalent >> instruction on ARM is WFI (wait for interrupt). >> >> This patch makes the busy loops in the ARM halt/reboot/panic paths >> use WFI when available (as defined in arch/arm/include/asm/barrier.h). >> >> A touch test indicates that this lowers the surface temperature of the >> Allwinner H3 SoC, with all 4 cores brought up with SMP, from painfully >> hot to just warm to the touch after shutdown to halt. >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> --- >> >> I asked about this some time ago, and the answer was no one had done >> it. So here it is. It is somewhat similar to the patch "arm64: >> Improve parking of stopped CPUs", for arm64 obviously. > > There are various errata around the "wfi" instruction on various CPUs > which means that this simple implementation isn't going to work > reliably. > > This is where cpu_do_idle() comes in - please use that instead. Thanks for the tip. Looks like this one is implemented for every platform. No need for #ifdefs anymore. ChenYu
On Thu, Jul 27, 2017 at 8:45 PM, Chen-Yu Tsai <wens@csie.org> wrote: > On Thu, Jul 27, 2017 at 8:17 PM, Russell King - ARM Linux > <linux@armlinux.org.uk> wrote: >> On Thu, Jul 27, 2017 at 08:08:03PM +0800, Chen-Yu Tsai wrote: >>> On ARM, the kernel busy loops after cleaning up when a core is to be >>> halted. This may have the undesired effect of increasing the core >>> temperature, at a time when no power or thermal management is >>> available, such as a kernel panic or shutdown to halt. >>> >>> x86 uses the more efficient HLT (halt) instruction. The equivalent >>> instruction on ARM is WFI (wait for interrupt). >>> >>> This patch makes the busy loops in the ARM halt/reboot/panic paths >>> use WFI when available (as defined in arch/arm/include/asm/barrier.h). >>> >>> A touch test indicates that this lowers the surface temperature of the >>> Allwinner H3 SoC, with all 4 cores brought up with SMP, from painfully >>> hot to just warm to the touch after shutdown to halt. >>> >>> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >>> --- >>> >>> I asked about this some time ago, and the answer was no one had done >>> it. So here it is. It is somewhat similar to the patch "arm64: >>> Improve parking of stopped CPUs", for arm64 obviously. >> >> There are various errata around the "wfi" instruction on various CPUs >> which means that this simple implementation isn't going to work >> reliably. >> >> This is where cpu_do_idle() comes in - please use that instead. > > Thanks for the tip. Looks like this one is implemented for every > platform. No need for #ifdefs anymore. So I've done the modifications and re-tested. Unfortunately it no longer works, either using wfi() directly or cpu_do_idle(). It worked back in January this year. I'm not sure what's happening. The chip is still hot. (Hotter than when it is idle in userspace.) I added some printk calls around the wfi() / cpu_do_idle() calls and found it continuously looping, instead of being parked. But of course printk might add to the problem. AFAIK all interrupts should be disabled by the time the call is reached. What could possibly bring the CPU out of WFI? Regards ChenYu
diff --git a/arch/arm/kernel/reboot.c b/arch/arm/kernel/reboot.c index 3b2aa9a9fe26..068aef796de4 100644 --- a/arch/arm/kernel/reboot.c +++ b/arch/arm/kernel/reboot.c @@ -10,6 +10,7 @@ #include <linux/delay.h> #include <linux/reboot.h> +#include <asm/barrier.h> #include <asm/cacheflush.h> #include <asm/idmap.h> #include <asm/virt.h> @@ -107,7 +108,11 @@ void machine_halt(void) { local_irq_disable(); smp_send_stop(); - while (1); + while (1) { +#ifdef wfi + wfi(); +#endif + } } /* @@ -151,5 +156,9 @@ void machine_restart(char *cmd) /* Whoops - the platform was unable to reboot. Tell the user! */ printk("Reboot failed -- System halted\n"); - while (1); + while (1) { +#ifdef wfi + wfi(); +#endif + } } diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index c9a0a5299827..7666fb2ed481 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -32,6 +32,7 @@ #include <linux/atomic.h> #include <asm/smp.h> +#include <asm/barrier.h> #include <asm/cacheflush.h> #include <asm/cpu.h> #include <asm/cputype.h> @@ -567,8 +568,12 @@ static void ipi_cpu_stop(unsigned int cpu) local_fiq_disable(); local_irq_disable(); - while (1) + while (1) { cpu_relax(); +#ifdef wfi + wfi(); +#endif + } } static DEFINE_PER_CPU(struct completion *, cpu_completion);
On ARM, the kernel busy loops after cleaning up when a core is to be halted. This may have the undesired effect of increasing the core temperature, at a time when no power or thermal management is available, such as a kernel panic or shutdown to halt. x86 uses the more efficient HLT (halt) instruction. The equivalent instruction on ARM is WFI (wait for interrupt). This patch makes the busy loops in the ARM halt/reboot/panic paths use WFI when available (as defined in arch/arm/include/asm/barrier.h). A touch test indicates that this lowers the surface temperature of the Allwinner H3 SoC, with all 4 cores brought up with SMP, from painfully hot to just warm to the touch after shutdown to halt. Signed-off-by: Chen-Yu Tsai <wens@csie.org> --- I asked about this some time ago, and the answer was no one had done it. So here it is. It is somewhat similar to the patch "arm64: Improve parking of stopped CPUs", for arm64 obviously. --- arch/arm/kernel/reboot.c | 13 +++++++++++-- arch/arm/kernel/smp.c | 7 ++++++- 2 files changed, 17 insertions(+), 3 deletions(-)