2013-03-21 22:03:34

by Thomas Gleixner

[permalink] [raw]
Subject: [patch 00/34] idle: Consolidate idle implementations

Each architecture implements its own cpu_idle() code, which is more or
less the same on all architectures (plus/minus a few bugs and a few
missing extra functionalities, instrumentation ...). That also forces
everyone who is interested in idle related features to add new
functionality to every architecture. What a waste.

Aside of that pointless code duplicaiton the ongoing quest to
consolidate the cpu hotplug code needs a common entry point for the
non boot cpus.

The following series implements a generic idle function and converts
most architectures over. I left out SPARC (it involves sparc asm) and
UM (it made me barf). If we can move those architectures as well, we
can get rid of the extra config switch and have everything
consolidated.

I spent a lot of time to make sure that the conversion preserved the
non obvious differences of the architecture implementations, but I
really need help from the affected maintainers to prove the
correctness.

Thanks,

tglx
---
arch/openrisc/kernel/idle.c | 73 ------------
linux-2.6/arch/Kconfig | 3
linux-2.6/arch/alpha/Kconfig | 1
linux-2.6/arch/alpha/include/asm/thread_info.h | 2
linux-2.6/arch/alpha/kernel/process.c | 19 ---
linux-2.6/arch/alpha/kernel/smp.c | 3
linux-2.6/arch/arc/Kconfig | 1
linux-2.6/arch/arc/kernel/process.c | 27 ----
linux-2.6/arch/arc/kernel/smp.c | 2
linux-2.6/arch/arm/Kconfig | 2
linux-2.6/arch/arm/include/asm/system_misc.h | 3
linux-2.6/arch/arm/kernel/process.c | 100 ++++-------------
linux-2.6/arch/arm/kernel/smp.c | 2
linux-2.6/arch/arm/mach-gemini/idle.c | 4
linux-2.6/arch/arm/mach-gemini/irq.c | 2
linux-2.6/arch/arm/mach-ixp4xx/common.c | 2
linux-2.6/arch/arm/mach-omap1/pm.c | 5
linux-2.6/arch/arm/mach-omap2/omap_hwmod.c | 6 -
linux-2.6/arch/arm/mach-omap2/pm.c | 5
linux-2.6/arch/arm/mach-orion5x/board-dt.c | 2
linux-2.6/arch/arm/mach-orion5x/common.c | 2
linux-2.6/arch/arm/mach-shark/core.c | 2
linux-2.6/arch/arm/mach-shmobile/suspend.c | 4
linux-2.6/arch/arm/mach-w90x900/dev.c | 2
linux-2.6/arch/arm64/Kconfig | 1
linux-2.6/arch/arm64/kernel/process.c | 43 -------
linux-2.6/arch/arm64/kernel/smp.c | 2
linux-2.6/arch/avr32/Kconfig | 1
linux-2.6/arch/avr32/kernel/process.c | 13 --
linux-2.6/arch/avr32/kernel/time.c | 8 +
linux-2.6/arch/avr32/mach-at32ap/include/mach/pm.h | 24 ----
linux-2.6/arch/avr32/mach-at32ap/pm-at32ap700x.S | 7 -
linux-2.6/arch/blackfin/Kconfig | 1
linux-2.6/arch/blackfin/kernel/process.c | 30 -----
linux-2.6/arch/blackfin/mach-common/smp.c | 2
linux-2.6/arch/c6x/Kconfig | 1
linux-2.6/arch/c6x/kernel/process.c | 28 ----
linux-2.6/arch/cris/Kconfig | 1
linux-2.6/arch/cris/arch-v10/kernel/process.c | 3
linux-2.6/arch/cris/arch-v32/kernel/process.c | 12 --
linux-2.6/arch/cris/arch-v32/kernel/smp.c | 4
linux-2.6/arch/cris/include/asm/processor.h | 7 -
linux-2.6/arch/cris/kernel/process.c | 49 --------
linux-2.6/arch/frv/Kconfig | 1
linux-2.6/arch/frv/kernel/process.c | 27 ----
linux-2.6/arch/h8300/Kconfig | 1
linux-2.6/arch/h8300/kernel/process.c | 35 ------
linux-2.6/arch/hexagon/Kconfig | 1
linux-2.6/arch/hexagon/kernel/process.c | 23 ----
linux-2.6/arch/hexagon/kernel/smp.c | 2
linux-2.6/arch/ia64/Kconfig | 1
linux-2.6/arch/ia64/include/asm/thread_info.h | 2
linux-2.6/arch/ia64/kernel/perfmon.c | 13 --
linux-2.6/arch/ia64/kernel/process.c | 83 ++------------
linux-2.6/arch/ia64/kernel/smpboot.c | 2
linux-2.6/arch/m32r/Kconfig | 1
linux-2.6/arch/m32r/kernel/process.c | 18 ---
linux-2.6/arch/m32r/kernel/smpboot.c | 2
linux-2.6/arch/m68k/Kconfig | 1
linux-2.6/arch/m68k/kernel/process.c | 32 -----
linux-2.6/arch/metag/Kconfig | 1
linux-2.6/arch/metag/include/asm/thread_info.h | 2
linux-2.6/arch/metag/kernel/process.c | 32 -----
linux-2.6/arch/metag/kernel/smp.c | 2
linux-2.6/arch/microblaze/Kconfig | 2
linux-2.6/arch/microblaze/include/asm/processor.h | 5
linux-2.6/arch/microblaze/include/asm/thread_info.h | 1
linux-2.6/arch/microblaze/kernel/process.c | 65 -----------
linux-2.6/arch/mips/Kconfig | 1
linux-2.6/arch/mips/kernel/process.c | 46 ++------
linux-2.6/arch/mips/kernel/smp.c | 2
linux-2.6/arch/mn10300/Kconfig | 1
linux-2.6/arch/mn10300/include/asm/thread_info.h | 2
linux-2.6/arch/mn10300/kernel/process.c | 70 +-----------
linux-2.6/arch/mn10300/kernel/smp.c | 7 -
linux-2.6/arch/openrisc/Kconfig | 1
linux-2.6/arch/openrisc/include/asm/thread_info.h | 2
linux-2.6/arch/parisc/Kconfig | 1
linux-2.6/arch/parisc/include/asm/thread_info.h | 2
linux-2.6/arch/parisc/kernel/process.c | 22 ---
linux-2.6/arch/parisc/kernel/smp.c | 2
linux-2.6/arch/powerpc/Kconfig | 1
linux-2.6/arch/powerpc/include/asm/thread_info.h | 4
linux-2.6/arch/powerpc/kernel/idle.c | 78 +++----------
linux-2.6/arch/powerpc/kernel/smp.c | 2
linux-2.6/arch/s390/kernel/process.c | 25 +---
linux-2.6/arch/s390/kernel/smp.c | 3
linux-2.6/arch/score/Kconfig | 1
linux-2.6/arch/score/kernel/process.c | 18 ---
linux-2.6/arch/sh/Kconfig | 2
linux-2.6/arch/sh/include/asm/thread_info.h | 4
linux-2.6/arch/sh/kernel/idle.c | 102 +----------------
linux-2.6/arch/sh/kernel/smp.c | 2
linux-2.6/arch/sparc/include/asm/thread_info_32.h | 2
linux-2.6/arch/sparc/include/asm/thread_info_64.h | 2
linux-2.6/arch/tile/include/asm/thread_info.h | 2
linux-2.6/arch/tile/kernel/process.c | 61 +---------
linux-2.6/arch/tile/kernel/smpboot.c | 4
linux-2.6/arch/unicore32/Kconfig | 1
linux-2.6/arch/unicore32/kernel/process.c | 21 ---
linux-2.6/arch/x86/Kconfig | 1
linux-2.6/arch/x86/include/asm/thread_info.h | 2
linux-2.6/arch/x86/kernel/process.c | 106 +++++-------------
linux-2.6/arch/x86/kernel/smpboot.c | 2
linux-2.6/arch/x86/xen/smp.c | 2
linux-2.6/arch/xtensa/Kconfig | 1
linux-2.6/arch/xtensa/kernel/process.c | 14 --
linux-2.6/include/linux/cpu.h | 16 ++
linux-2.6/include/linux/sched.h | 41 +++++++
linux-2.6/init/main.c | 2
linux-2.6/kernel/Makefile | 1
linux-2.6/kernel/cpu/Makefile | 1
linux-2.6/kernel/cpu/idle.c | 115 ++++++++++++++++++++
linux-2.6/kernel/sched/core.c | 5
114 files changed, 432 insertions(+), 1237 deletions(-)


2013-03-22 20:09:11

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [patch 00/34] idle: Consolidate idle implementations

Hi Thomas.
On Thu, Mar 21, 2013 at 09:52:56PM -0000, Thomas Gleixner wrote:
> Each architecture implements its own cpu_idle() code, which is more or
> less the same on all architectures (plus/minus a few bugs and a few
> missing extra functionalities, instrumentation ...). That also forces
> everyone who is interested in idle related features to add new
> functionality to every architecture. What a waste.
>
> Aside of that pointless code duplicaiton the ongoing quest to
> consolidate the cpu hotplug code needs a common entry point for the
> non boot cpus.
>
> The following series implements a generic idle function and converts
> most architectures over. I left out SPARC (it involves sparc asm) and
> UM (it made me barf). If we can move those architectures as well, we
> can get rid of the extra config switch and have everything
> consolidated.
I wanted to take a quick look at sparc - but build failed
after applying patch 1-5. Looks like the same issue Tony already
reaported.

At very first glnce it looks straightforward to convert
both sparc32 and sparc64.
The assembler stuff used by sparc64 fits into one of the arch functions
as far as I could see.

When I get back from vacation I may take a look.

Sam

2013-03-28 09:24:59

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 00/34] idle: Consolidate idle implementations

On Thu, 28 Mar 2013, Chris Zankel wrote:

> For Xtensa:
> Acked-by: Chris Zankel <[email protected]>
>
> Thanks for going the extra mile and test-compiling it.

Though, the build fails later with:

arch/xtensa/kernel/built-in.o:(.init.literal+0x90): undefined reference to `platform_pcibios_init'
arch/xtensa/kernel/built-in.o: In function `setup_arch':
(.init.text+0x1de): undefined reference to `platform_pcibios_init'
drivers/built-in.o: In function `pci_assign_unassigned_resources':
(.init.text+0x52b): dangerous relocation: call8: call target out of range: (.ref.text+0x3ac)
drivers/built-in.o: In function `pci_assign_unassigned_resources':
(.init.text+0x5b5): dangerous relocation: call8: call target out of range: (.ref.text+0x2c8)
drivers/built-in.o: In function `sysrq_init':
sysrq.c:(.init.text+0xb61): dangerous relocation: call8: call target out of range: panic
drivers/built-in.o: In function `console_map_init':
(.init.text+0xc97): dangerous relocation: call8: call target out of range: printk
drivers/built-in.o: In function `vtconsole_class_init':
vt.c:(.init.text+0xcdd): dangerous relocation: call8: call target out of range: printk
drivers/built-in.o: In function `con_init':
vt.c:(.init.text+0xe75): dangerous relocation: call8: call target out of range: printk
vt.c:(.init.text+0xece): dangerous relocation: call8: call target out of range: panic
drivers/built-in.o: In function `vty_init':
(.init.text+0xf32): dangerous relocation: call8: call target out of range: panic
drivers/built-in.o: In function `vty_init':
(.init.text+0xf8e): dangerous relocation: call8: call target out of range: panic
drivers/built-in.o: In function `serial8250_isa_init_ports':
8250.c:(.init.text+0x10ea): dangerous relocation: call8: call target out of range: printk
drivers/built-in.o: In function `setup_early_serial8250_console':
(.init.text+0x1450): dangerous relocation: call8: call target out of range: printk
drivers/built-in.o: In function `setup_early_serial8250_console':
(.init.text+0x1456): dangerous relocation: call8: call target out of range: panic
drivers/built-in.o: In function `setup_early_serial8250_console':
(.init.text+0x1461): dangerous relocation: call8: call target out of range: printk
drivers/built-in.o: In function `setup_early_serial8250_console':
(.init.text+0x155d): dangerous relocation: call8: call target out of range: printk
drivers/built-in.o: In function `setup_early_serial8250_console':
(.init.text+0x156e): dangerous relocation: call8: call target out of range: printk
drivers/built-in.o: In function `setup_early_serial8250_console':
(.init.text+0x1625): dangerous relocation: call8: call target out of range: printk
drivers/built-in.o: In function `chr_dev_init':
mem.c:(.init.text+0x16e6): dangerous relocation: call8: call target out of range: printk
drivers/built-in.o: In function `random_int_secret_init':
random.c:(.init.text+0x171a): dangerous relocation: call8: call target out of range: printk
drivers/built-in.o: In function `misc_init':
misc.c:(.init.text+0x1765): dangerous relocation: call8: call target out of range: printk
misc.c:(.init.text+0x1789): dangerous relocation: call8: call target out of range: printk
net/built-in.o: In function `synchronize_net':
(.text+0xc650): dangerous relocation: call8: misaligned call target: (.text.unlikely+0x43)
net/built-in.o: In function `alloc_netdev_mqs':
(.text+0xc7ed): dangerous relocation: call8: misaligned call target: (.text.unlikely+0x43)
net/built-in.o: In function `netlink_proto_init':
af_netlink.c:(.init.text+0x7a9): dangerous relocation: call8: call target out of range: (.text.unlikely+0xa4)
make[1]: *** [vmlinux] Error 1

2013-03-28 22:16:15

by Chris Zankel

[permalink] [raw]
Subject: Re: [patch 00/34] idle: Consolidate idle implementations

Hi Thomas,

On 3/28/13 2:24 AM, Thomas Gleixner wrote:
> On Thu, 28 Mar 2013, Chris Zankel wrote:
>
>> For Xtensa:
>> Acked-by: Chris Zankel <[email protected]>
>>
>> Thanks for going the extra mile and test-compiling it.
> Though, the build fails later with:
>
> arch/xtensa/kernel/built-in.o:(.init.literal+0x90): undefined reference to `platform_pcibios_init'
> arch/xtensa/kernel/built-in.o: In function `setup_arch':
> (.init.text+0x1de): undefined reference to `platform_pcibios_init'
> drivers/built-in.o: In function `pci_assign_unassigned_resources':
[...]

Thanks for the info. With KALLSYMS enabled, the .rodata section between
the .text and the .init.text section is extended to a point where it
exceeds the call range between those sections. I'm working on a fix
(basically, move the .init.text section close to the .init section).

Thanks,
-Chris

2013-03-29 16:19:08

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [patch 00/34] idle: Consolidate idle implementations

On Thu, Mar 21, 2013 at 09:52:56PM -0000, Thomas Gleixner wrote:
> Each architecture implements its own cpu_idle() code, which is more or
> less the same on all architectures (plus/minus a few bugs and a few
> missing extra functionalities, instrumentation ...). That also forces
> everyone who is interested in idle related features to add new
> functionality to every architecture. What a waste.
>
> Aside of that pointless code duplicaiton the ongoing quest to
> consolidate the cpu hotplug code needs a common entry point for the
> non boot cpus.
>
> The following series implements a generic idle function and converts
> most architectures over. I left out SPARC (it involves sparc asm) and
> UM (it made me barf). If we can move those architectures as well, we
> can get rid of the extra config switch and have everything
> consolidated.
>
> I spent a lot of time to make sure that the conversion preserved the
> non obvious differences of the architecture implementations, but I
> really need help from the affected maintainers to prove the
> correctness.

Hi Thomas.

Where can I find the git tree with the latest patches?
Or do you plan to submit a v2 soon?

Sam

2013-03-29 20:29:30

by Sam Ravnborg

[permalink] [raw]
Subject: [PATCH] sparc: Use generic idle loop

Add generic cpu_idle support

sparc32:
- replace call to cpu_idle() with cpu_startup_entry()
- add arch_cpu_idle()

sparc64:
- smp_callin() includes cpu_startup_entry() call so we can
skip calling cpu_idle from assembler
- add arch_cpu_idle_enter() and arch_cpu_idle_dead()

Signed-off-by: Sam Ravnborg <[email protected]>
Cc: David S. Miller <[email protected]>

---
This patch is on top of smp/testing in the tip tree.
It is build tested on sparc32 + sparc64

Sam


diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 3d361f2..5f22169 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -39,6 +39,7 @@ config SPARC
select GENERIC_CLOCKEVENTS
select GENERIC_STRNCPY_FROM_USER
select GENERIC_STRNLEN_USER
+ select GENERIC_IDLE_LOOP
select MODULES_USE_ELF_RELA
select ODD_RT_SIGACTION
select OLD_SIGSUSPEND
diff --git a/arch/sparc/kernel/hvtramp.S b/arch/sparc/kernel/hvtramp.S
index 9365432..605c960 100644
--- a/arch/sparc/kernel/hvtramp.S
+++ b/arch/sparc/kernel/hvtramp.S
@@ -128,8 +128,7 @@ hv_cpu_startup:

call smp_callin
nop
- call cpu_idle
- mov 0, %o0
+
call cpu_panic
nop

diff --git a/arch/sparc/kernel/process_32.c b/arch/sparc/kernel/process_32.c
index 62eede1..ec92959 100644
--- a/arch/sparc/kernel/process_32.c
+++ b/arch/sparc/kernel/process_32.c
@@ -64,23 +64,13 @@ extern void fpsave(unsigned long *, unsigned long *, void *, unsigned long *);
struct task_struct *last_task_used_math = NULL;
struct thread_info *current_set[NR_CPUS];

-/*
- * the idle loop on a Sparc... ;)
- */
-void cpu_idle(void)
+/* the idle loop on a Sparc... ;) */
+void arch_cpu_idle(void)
{
- set_thread_flag(TIF_POLLING_NRFLAG);
-
- /* endless idle loop with no priority at all */
- for (;;) {
- while (!need_resched()) {
- if (sparc_idle)
- (*sparc_idle)();
- else
- cpu_relax();
- }
- schedule_preempt_disabled();
- }
+ if (sparc_idle)
+ (*sparc_idle)();
+ else
+ cpu_relax();
}

/* XXX cli/sti -> local_irq_xxx here, check this works once SMP is fixed. */
diff --git a/arch/sparc/kernel/process_64.c b/arch/sparc/kernel/process_64.c
index cdb80b2..5ed6b02 100644
--- a/arch/sparc/kernel/process_64.c
+++ b/arch/sparc/kernel/process_64.c
@@ -52,8 +52,13 @@

#include "kstack.h"

-static void sparc64_yield(int cpu)
+/* The idle loop on sparc64. */
+void arch_cpu_idle_enter()
{
+ int cpu;
+
+ cpu = smp_processor_id();
+
if (tlb_type != hypervisor) {
touch_nmi_watchdog();
return;
@@ -88,31 +93,10 @@ static void sparc64_yield(int cpu)
set_thread_flag(TIF_POLLING_NRFLAG);
}

-/* The idle loop on sparc64. */
-void cpu_idle(void)
+void arch_cpu_idle_dead()
{
- int cpu = smp_processor_id();
-
- set_thread_flag(TIF_POLLING_NRFLAG);
-
- while(1) {
- tick_nohz_idle_enter();
- rcu_idle_enter();
-
- while (!need_resched() && !cpu_is_offline(cpu))
- sparc64_yield(cpu);
-
- rcu_idle_exit();
- tick_nohz_idle_exit();
-
-#ifdef CONFIG_HOTPLUG_CPU
- if (cpu_is_offline(cpu)) {
- sched_preempt_enable_no_resched();
- cpu_play_dead();
- }
-#endif
- schedule_preempt_disabled();
- }
+ sched_preempt_enable_no_resched();
+ cpu_play_dead();
}

#ifdef CONFIG_COMPAT
diff --git a/arch/sparc/kernel/smp_32.c b/arch/sparc/kernel/smp_32.c
index 9e7e6d7..e3f2b81 100644
--- a/arch/sparc/kernel/smp_32.c
+++ b/arch/sparc/kernel/smp_32.c
@@ -369,7 +369,7 @@ void __cpuinit sparc_start_secondary(void *arg)
local_irq_enable();

wmb();
- cpu_idle();
+ cpu_startup_entry(CPUHP_ONLINE);

/* We should never reach here! */
BUG();
diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c
index 537eb66..c025ffc 100644
--- a/arch/sparc/kernel/smp_64.c
+++ b/arch/sparc/kernel/smp_64.c
@@ -127,6 +127,8 @@ void __cpuinit smp_callin(void)

/* idle thread is expected to have preempt disabled */
preempt_disable();
+
+ cpu_startup_entry(CPUHP_ONLINE);
}

void cpu_panic(void)
diff --git a/arch/sparc/kernel/trampoline_64.S b/arch/sparc/kernel/trampoline_64.S
index da1b781..2e973a2 100644
--- a/arch/sparc/kernel/trampoline_64.S
+++ b/arch/sparc/kernel/trampoline_64.S
@@ -407,8 +407,7 @@ after_lock_tlb:

call smp_callin
nop
- call cpu_idle
- mov 0, %o0
+
call cpu_panic
nop
1: b,a,pt %xcc, 1b

2013-03-31 23:46:19

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] sparc: Use generic idle loop

From: Sam Ravnborg <[email protected]>
Date: Fri, 29 Mar 2013 21:29:26 +0100

> Add generic cpu_idle support
>
> sparc32:
> - replace call to cpu_idle() with cpu_startup_entry()
> - add arch_cpu_idle()
>
> sparc64:
> - smp_callin() includes cpu_startup_entry() call so we can
> skip calling cpu_idle from assembler
> - add arch_cpu_idle_enter() and arch_cpu_idle_dead()
>
> Signed-off-by: Sam Ravnborg <[email protected]>

Acked-by: David S. Miller <[email protected]>

2013-04-01 06:56:17

by Srivatsa S. Bhat

[permalink] [raw]
Subject: Re: [PATCH] sparc: Use generic idle loop

On 03/30/2013 01:59 AM, Sam Ravnborg wrote:
> Add generic cpu_idle support
>
> sparc32:
> - replace call to cpu_idle() with cpu_startup_entry()
> - add arch_cpu_idle()
>
> sparc64:
> - smp_callin() includes cpu_startup_entry() call so we can
> skip calling cpu_idle from assembler
> - add arch_cpu_idle_enter() and arch_cpu_idle_dead()
>
> Signed-off-by: Sam Ravnborg <[email protected]>
> Cc: David S. Miller <[email protected]>
>
> ---
> This patch is on top of smp/testing in the tip tree.
> It is build tested on sparc32 + sparc64
>
> Sam
>
[...]
>
> diff --git a/arch/sparc/kernel/process_32.c b/arch/sparc/kernel/process_32.c
> index 62eede1..ec92959 100644
> --- a/arch/sparc/kernel/process_32.c
> +++ b/arch/sparc/kernel/process_32.c
> @@ -64,23 +64,13 @@ extern void fpsave(unsigned long *, unsigned long *, void *, unsigned long *);
> struct task_struct *last_task_used_math = NULL;
> struct thread_info *current_set[NR_CPUS];
>
> -/*
> - * the idle loop on a Sparc... ;)
> - */
> -void cpu_idle(void)
> +/* the idle loop on a Sparc... ;) */
> +void arch_cpu_idle(void)
> {
> - set_thread_flag(TIF_POLLING_NRFLAG);
> -
> - /* endless idle loop with no priority at all */
> - for (;;) {
> - while (!need_resched()) {
> - if (sparc_idle)
> - (*sparc_idle)();
> - else
> - cpu_relax();
> - }
> - schedule_preempt_disabled();
> - }
> + if (sparc_idle)
> + (*sparc_idle)();
> + else
> + cpu_relax();
> }

You need to enable interrupts when coming out in the 'else' case,
because we enter with interrupts disabled and expect to come out
of arch_cpu_idle() with interrupts enabled. (Otherwise you'll hit
that WARN_ON() in the generic code). Thomas has handled a similar
case in the mips patch.

>
> /* XXX cli/sti -> local_irq_xxx here, check this works once SMP is fixed. */
> diff --git a/arch/sparc/kernel/process_64.c b/arch/sparc/kernel/process_64.c
> index cdb80b2..5ed6b02 100644
> --- a/arch/sparc/kernel/process_64.c
> +++ b/arch/sparc/kernel/process_64.c
> @@ -52,8 +52,13 @@
>
> #include "kstack.h"
>
> -static void sparc64_yield(int cpu)
> +/* The idle loop on sparc64. */
> +void arch_cpu_idle_enter()
> {
> + int cpu;
> +
> + cpu = smp_processor_id();
> +
> if (tlb_type != hypervisor) {
> touch_nmi_watchdog();
> return;
> @@ -88,31 +93,10 @@ static void sparc64_yield(int cpu)
> set_thread_flag(TIF_POLLING_NRFLAG);
> }
>

Hmm, this looks weird. The contents of this function should have
been inside arch_cpu_idle() (not arch_cpu_idle_enter). And it is
unnecessary to set/clear POLLING flags here, since they are handled
in the generic code. (Same is the case with the need_resched and
the cpu_is_offline check).

I agree that the naming is a little subtle, but in the current
scheme of things in this patchset, arch_cpu_idle_enter() is what
you call to do initialization _before_ you perform cpu idle, whereas
arch_cpu_idle() is the function that is supposed to do the actual
arch-specific idling.

Regards,
Srivatsa S. Bhat

> -/* The idle loop on sparc64. */
> -void cpu_idle(void)
> +void arch_cpu_idle_dead()
> {
> - int cpu = smp_processor_id();
> -
> - set_thread_flag(TIF_POLLING_NRFLAG);
> -
> - while(1) {
> - tick_nohz_idle_enter();
> - rcu_idle_enter();
> -
> - while (!need_resched() && !cpu_is_offline(cpu))
> - sparc64_yield(cpu);
> -
> - rcu_idle_exit();
> - tick_nohz_idle_exit();
> -
> -#ifdef CONFIG_HOTPLUG_CPU
> - if (cpu_is_offline(cpu)) {
> - sched_preempt_enable_no_resched();
> - cpu_play_dead();
> - }
> -#endif
> - schedule_preempt_disabled();
> - }
> + sched_preempt_enable_no_resched();
> + cpu_play_dead();
> }

2013-04-01 09:06:27

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] sparc: Use generic idle loop

Hi Srivatsa,

thanks for the feedback!

@davem - I need you to look at this again. I am not sure about the changes
I do in sparc64_yield().

> > +/* the idle loop on a Sparc... ;) */
> > +void arch_cpu_idle(void)
> > {
> > + if (sparc_idle)
> > + (*sparc_idle)();
> > + else
> > + cpu_relax();
> > }
>
> You need to enable interrupts when coming out in the 'else' case,
> because we enter with interrupts disabled and expect to come out
> of arch_cpu_idle() with interrupts enabled. (Otherwise you'll hit
> that WARN_ON() in the generic code). Thomas has handled a similar
> case in the mips patch.

Something like this should do it then:

/* Idle loop support */
void arch_cpu_idle(void)
{
if (sparc_idle)
(*sparc_idle)();
else
local_irq_enable();
}

I dropped cpu_relax() as this is a simple barrier() which is
part of the generic code (using rmb()).


> > -static void sparc64_yield(int cpu)
> > +/* The idle loop on sparc64. */
> > +void arch_cpu_idle_enter()
> > {
> > + int cpu;
> > +
> > + cpu = smp_processor_id();
> > +
> > if (tlb_type != hypervisor) {
> > touch_nmi_watchdog();
> > return;
> > @@ -88,31 +93,10 @@ static void sparc64_yield(int cpu)
> > set_thread_flag(TIF_POLLING_NRFLAG);
> > }
> >
>
> Hmm, this looks weird. The contents of this function should have
> been inside arch_cpu_idle() (not arch_cpu_idle_enter).
Yep - I see now.

> And it is
> unnecessary to set/clear POLLING flags here, since they are handled
> in the generic code.
OK

> (Same is the case with the need_resched and
> the cpu_is_offline check).
There are some comments about race-conditions in the commit where they were added.
I think the out-most pair can be dropped as this is done in generic code but
that the inner ones needs to stay.

I simplified arch_cpu_idle() - former sparc64_yield() based on your comments.
- dropped TIF_POLLING_NRFLAG handling
- dropped smp_mb__after_clear_bit() - this is done in generic code using rmb()
- dropped the out-most while (!need_resched() && !cpu_is_offline(cpu))
- always enable irq on exit (but I am not sure if this is correct)

But I need davem to have a look at this - as I am fooling around in
code that I do not know much about..
So this is *NOT* to be applied.

Sam

diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 3d361f2..ee5eacc 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -37,6 +37,7 @@ config SPARC
select GENERIC_SMP_IDLE_THREAD
select GENERIC_CMOS_UPDATE
select GENERIC_CLOCKEVENTS
+ select GENERIC_IDLE_LOOP
select GENERIC_STRNCPY_FROM_USER
select GENERIC_STRNLEN_USER
select MODULES_USE_ELF_RELA
diff --git a/arch/sparc/kernel/hvtramp.S b/arch/sparc/kernel/hvtramp.S
index 9365432..605c960 100644
--- a/arch/sparc/kernel/hvtramp.S
+++ b/arch/sparc/kernel/hvtramp.S
@@ -128,8 +128,7 @@ hv_cpu_startup:

call smp_callin
nop
- call cpu_idle
- mov 0, %o0
+
call cpu_panic
nop

diff --git a/arch/sparc/kernel/process_32.c b/arch/sparc/kernel/process_32.c
index 62eede1..890d7c4 100644
--- a/arch/sparc/kernel/process_32.c
+++ b/arch/sparc/kernel/process_32.c
@@ -64,23 +64,13 @@ extern void fpsave(unsigned long *, unsigned long *, void *, unsigned long *);
struct task_struct *last_task_used_math = NULL;
struct thread_info *current_set[NR_CPUS];

-/*
- * the idle loop on a Sparc... ;)
- */
-void cpu_idle(void)
+/* Idle loop support. */
+void arch_cpu_idle(void)
{
- set_thread_flag(TIF_POLLING_NRFLAG);
-
- /* endless idle loop with no priority at all */
- for (;;) {
- while (!need_resched()) {
- if (sparc_idle)
- (*sparc_idle)();
- else
- cpu_relax();
- }
- schedule_preempt_disabled();
- }
+ if (sparc_idle)
+ (*sparc_idle)();
+ else
+ local_irq_enable();
}

/* XXX cli/sti -> local_irq_xxx here, check this works once SMP is fixed. */
diff --git a/arch/sparc/kernel/process_64.c b/arch/sparc/kernel/process_64.c
index cdb80b2..cecb0d6 100644
--- a/arch/sparc/kernel/process_64.c
+++ b/arch/sparc/kernel/process_64.c
@@ -52,17 +52,12 @@

#include "kstack.h"

-static void sparc64_yield(int cpu)
+/* Idle loop support on sparc64. */
+void arch_cpu_idle(void)
{
if (tlb_type != hypervisor) {
touch_nmi_watchdog();
- return;
- }
-
- clear_thread_flag(TIF_POLLING_NRFLAG);
- smp_mb__after_clear_bit();
-
- while (!need_resched() && !cpu_is_offline(cpu)) {
+ } else {
unsigned long pstate;

/* Disable interrupts. */
@@ -73,7 +68,7 @@ static void sparc64_yield(int cpu)
: "=&r" (pstate)
: "i" (PSTATE_IE));

- if (!need_resched() && !cpu_is_offline(cpu))
+ if (!need_resched() && !cpu_is_offline(smp_processor_id()))
sun4v_cpu_yield();

/* Re-enable interrupts. */
@@ -84,36 +79,16 @@ static void sparc64_yield(int cpu)
: "=&r" (pstate)
: "i" (PSTATE_IE));
}
-
- set_thread_flag(TIF_POLLING_NRFLAG);
+ local_irq_enable();
}

-/* The idle loop on sparc64. */
-void cpu_idle(void)
-{
- int cpu = smp_processor_id();
-
- set_thread_flag(TIF_POLLING_NRFLAG);
-
- while(1) {
- tick_nohz_idle_enter();
- rcu_idle_enter();
-
- while (!need_resched() && !cpu_is_offline(cpu))
- sparc64_yield(cpu);
-
- rcu_idle_exit();
- tick_nohz_idle_exit();
-
#ifdef CONFIG_HOTPLUG_CPU
- if (cpu_is_offline(cpu)) {
- sched_preempt_enable_no_resched();
- cpu_play_dead();
- }
-#endif
- schedule_preempt_disabled();
- }
+void arch_cpu_idle_dead()
+{
+ sched_preempt_enable_no_resched();
+ cpu_play_dead();
}
+#endif

#ifdef CONFIG_COMPAT
static void show_regwindow32(struct pt_regs *regs)
diff --git a/arch/sparc/kernel/smp_32.c b/arch/sparc/kernel/smp_32.c
index 9e7e6d7..e3f2b81 100644
--- a/arch/sparc/kernel/smp_32.c
+++ b/arch/sparc/kernel/smp_32.c
@@ -369,7 +369,7 @@ void __cpuinit sparc_start_secondary(void *arg)
local_irq_enable();

wmb();
- cpu_idle();
+ cpu_startup_entry(CPUHP_ONLINE);

/* We should never reach here! */
BUG();
diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c
index 537eb66..c025ffc 100644
--- a/arch/sparc/kernel/smp_64.c
+++ b/arch/sparc/kernel/smp_64.c
@@ -127,6 +127,8 @@ void __cpuinit smp_callin(void)

/* idle thread is expected to have preempt disabled */
preempt_disable();
+
+ cpu_startup_entry(CPUHP_ONLINE);
}

void cpu_panic(void)
diff --git a/arch/sparc/kernel/trampoline_64.S b/arch/sparc/kernel/trampoline_64.S
index da1b781..2e973a2 100644
--- a/arch/sparc/kernel/trampoline_64.S
+++ b/arch/sparc/kernel/trampoline_64.S
@@ -407,8 +407,7 @@ after_lock_tlb:

call smp_callin
nop
- call cpu_idle
- mov 0, %o0
+
call cpu_panic
nop
1: b,a,pt %xcc, 1b

2013-04-08 12:36:34

by Srivatsa S. Bhat

[permalink] [raw]
Subject: Re: [PATCH] sparc: Use generic idle loop

Hi Sam,

On 04/01/2013 02:36 PM, Sam Ravnborg wrote:
> Hi Srivatsa,
>
> thanks for the feedback!
>
> @davem - I need you to look at this again. I am not sure about the changes
> I do in sparc64_yield().
>
>>> +/* the idle loop on a Sparc... ;) */
>>> +void arch_cpu_idle(void)
>>> {
>>> + if (sparc_idle)
>>> + (*sparc_idle)();
>>> + else
>>> + cpu_relax();
>>> }
>>
>> You need to enable interrupts when coming out in the 'else' case,
>> because we enter with interrupts disabled and expect to come out
>> of arch_cpu_idle() with interrupts enabled. (Otherwise you'll hit
>> that WARN_ON() in the generic code). Thomas has handled a similar
>> case in the mips patch.
>
> Something like this should do it then:
>
> /* Idle loop support */
> void arch_cpu_idle(void)
> {
> if (sparc_idle)
> (*sparc_idle)();
> else
> local_irq_enable();
> }
>
> I dropped cpu_relax() as this is a simple barrier() which is
> part of the generic code (using rmb()).
>

Yep, that sounds good.

[...]
>> (Same is the case with the need_resched and
>> the cpu_is_offline check).
> There are some comments about race-conditions in the commit where they were added.
> I think the out-most pair can be dropped as this is done in generic code but
> that the inner ones needs to stay.
>
> I simplified arch_cpu_idle() - former sparc64_yield() based on your comments.
> - dropped TIF_POLLING_NRFLAG handling
> - dropped smp_mb__after_clear_bit() - this is done in generic code using rmb()
> - dropped the out-most while (!need_resched() && !cpu_is_offline(cpu))
> - always enable irq on exit (but I am not sure if this is correct)
>
> But I need davem to have a look at this - as I am fooling around in
> code that I do not know much about..
> So this is *NOT* to be applied.
>
> Sam
>
[...]
> /* XXX cli/sti -> local_irq_xxx here, check this works once SMP is fixed. */
> diff --git a/arch/sparc/kernel/process_64.c b/arch/sparc/kernel/process_64.c
> index cdb80b2..cecb0d6 100644
> --- a/arch/sparc/kernel/process_64.c
> +++ b/arch/sparc/kernel/process_64.c
> @@ -52,17 +52,12 @@
>
> #include "kstack.h"
>
> -static void sparc64_yield(int cpu)
> +/* Idle loop support on sparc64. */
> +void arch_cpu_idle(void)
> {
> if (tlb_type != hypervisor) {
> touch_nmi_watchdog();
> - return;
> - }
> -
> - clear_thread_flag(TIF_POLLING_NRFLAG);
> - smp_mb__after_clear_bit();
> -
> - while (!need_resched() && !cpu_is_offline(cpu)) {
> + } else {
> unsigned long pstate;
>
> /* Disable interrupts. */
> @@ -73,7 +68,7 @@ static void sparc64_yield(int cpu)
> : "=&r" (pstate)
> : "i" (PSTATE_IE));
>
> - if (!need_resched() && !cpu_is_offline(cpu))
> + if (!need_resched() && !cpu_is_offline(smp_processor_id()))
> sun4v_cpu_yield();
>
> /* Re-enable interrupts. */
> @@ -84,36 +79,16 @@ static void sparc64_yield(int cpu)
> : "=&r" (pstate)
> : "i" (PSTATE_IE));
> }
> -
> - set_thread_flag(TIF_POLLING_NRFLAG);
> + local_irq_enable();
> }

Nitpick: you can probably move the local_irq_enable() to the
'if' block, since the else block already has assembly code to enable
the interrupts. But anyway its up to you.

This version of your code looks fine to me.

Reviewed-by: Srivatsa S. Bhat <[email protected]>

Regards,
Srivatsa S. Bhat

2013-04-08 17:10:40

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] sparc: Use generic idle loop

> > @@ -52,17 +52,12 @@
> >
> > #include "kstack.h"
> >
> > -static void sparc64_yield(int cpu)
> > +/* Idle loop support on sparc64. */
> > +void arch_cpu_idle(void)
> > {
> > if (tlb_type != hypervisor) {
> > touch_nmi_watchdog();
> > - return;
> > - }
> > -
> > - clear_thread_flag(TIF_POLLING_NRFLAG);
> > - smp_mb__after_clear_bit();
> > -
> > - while (!need_resched() && !cpu_is_offline(cpu)) {
> > + } else {
> > unsigned long pstate;
> >
> > /* Disable interrupts. */
> > @@ -73,7 +68,7 @@ static void sparc64_yield(int cpu)
> > : "=&r" (pstate)
> > : "i" (PSTATE_IE));
> >
> > - if (!need_resched() && !cpu_is_offline(cpu))
> > + if (!need_resched() && !cpu_is_offline(smp_processor_id()))
> > sun4v_cpu_yield();
> >
> > /* Re-enable interrupts. */
> > @@ -84,36 +79,16 @@ static void sparc64_yield(int cpu)
> > : "=&r" (pstate)
> > : "i" (PSTATE_IE));
> > }
> > -
> > - set_thread_flag(TIF_POLLING_NRFLAG);
> > + local_irq_enable();
> > }
>
> Nitpick: you can probably move the local_irq_enable() to the
> 'if' block, since the else block already has assembly code to enable
> the interrupts. But anyway its up to you.

I think not.
local_irq_disable writes 0 to the PIL register,
whereas the above code set the IE (Interrupt enable) bit to 0.

So the implementations differs - and I think there is a good
reason for being so.

But this is the part where I refer to that I am fooling around
in code that I do not understand.
I re-checked the SPARC V9 manual - but I did not within a few minutes
reading understand what is the difference between the twoo.

Sam

2013-04-08 19:24:55

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] sparc: Use generic idle loop

From: Sam Ravnborg <[email protected]>
Date: Mon, 8 Apr 2013 19:10:35 +0200

> I think not.
> local_irq_disable writes 0 to the PIL register,
> whereas the above code set the IE (Interrupt enable) bit to 0.
>
> So the implementations differs - and I think there is a good
> reason for being so.
>
> But this is the part where I refer to that I am fooling around
> in code that I do not understand.
> I re-checked the SPARC V9 manual - but I did not within a few minutes
> reading understand what is the difference between the twoo.

Device interrupts arrive first as high-priority interrupt packets
that are serviced by traps which are enabled only if PSTATE.IE is
set.

These trap handlers reschedule the interrupt quickly into a PIL
levelled interrupt, whose delivery is covered by (%pil & PSTATE.IE)

The sun4v sleeping code requires that we have PSTATE.IE clear over
the cpu sleep hypervisor call.

2013-04-11 19:38:55

by Sam Ravnborg

[permalink] [raw]
Subject: [PATCH v2] sparc: Use generic idle loop

Add generic cpu_idle support

sparc32:
- replace call to cpu_idle() with cpu_startup_entry()
- add arch_cpu_idle()

sparc64:
- smp_callin() now include cpu_startup_entry() call so we can
skip calling cpu_idle from assembler
- add arch_cpu_idle() and arch_cpu_idle_dead()

Signed-off-by: Sam Ravnborg <[email protected]>
Reviewed-by: "Srivatsa S. Bhat" <[email protected]>
Cc: David S. Miller <[email protected]>
---
v1->v2:
- simplified arch_cpu_idle() - former sparc64_yield() based on comments from Srivatsa
- dropped TIF_POLLING_NRFLAG handling
- dropped smp_mb__after_clear_bit() - this is done in generic code using rmb()
- dropped the out-most while (!need_resched() && !cpu_is_offline(cpu))
- always enable irq on exit in arch_cpu_idle (both sparc32 and sparc64)
- added comment from davem why we need to clear IE bit in sparc64 arch_cpu_idle()

Sam


diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 3d361f2..ee5eacc 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -37,6 +37,7 @@ config SPARC
select GENERIC_SMP_IDLE_THREAD
select GENERIC_CMOS_UPDATE
select GENERIC_CLOCKEVENTS
+ select GENERIC_IDLE_LOOP
select GENERIC_STRNCPY_FROM_USER
select GENERIC_STRNLEN_USER
select MODULES_USE_ELF_RELA
diff --git a/arch/sparc/kernel/hvtramp.S b/arch/sparc/kernel/hvtramp.S
index 9365432..605c960 100644
--- a/arch/sparc/kernel/hvtramp.S
+++ b/arch/sparc/kernel/hvtramp.S
@@ -128,8 +128,7 @@ hv_cpu_startup:

call smp_callin
nop
- call cpu_idle
- mov 0, %o0
+
call cpu_panic
nop

diff --git a/arch/sparc/kernel/process_32.c b/arch/sparc/kernel/process_32.c
index 62eede1..c852410 100644
--- a/arch/sparc/kernel/process_32.c
+++ b/arch/sparc/kernel/process_32.c
@@ -64,23 +64,12 @@ extern void fpsave(unsigned long *, unsigned long *, void *, unsigned long *);
struct task_struct *last_task_used_math = NULL;
struct thread_info *current_set[NR_CPUS];

-/*
- * the idle loop on a Sparc... ;)
- */
-void cpu_idle(void)
+/* Idle loop support. */
+void arch_cpu_idle(void)
{
- set_thread_flag(TIF_POLLING_NRFLAG);
-
- /* endless idle loop with no priority at all */
- for (;;) {
- while (!need_resched()) {
- if (sparc_idle)
- (*sparc_idle)();
- else
- cpu_relax();
- }
- schedule_preempt_disabled();
- }
+ if (sparc_idle)
+ (*sparc_idle)();
+ local_irq_enable();
}

/* XXX cli/sti -> local_irq_xxx here, check this works once SMP is fixed. */
diff --git a/arch/sparc/kernel/process_64.c b/arch/sparc/kernel/process_64.c
index cdb80b2..9fbf0d1 100644
--- a/arch/sparc/kernel/process_64.c
+++ b/arch/sparc/kernel/process_64.c
@@ -52,20 +52,17 @@

#include "kstack.h"

-static void sparc64_yield(int cpu)
+/* Idle loop support on sparc64. */
+void arch_cpu_idle(void)
{
if (tlb_type != hypervisor) {
touch_nmi_watchdog();
- return;
- }
-
- clear_thread_flag(TIF_POLLING_NRFLAG);
- smp_mb__after_clear_bit();
-
- while (!need_resched() && !cpu_is_offline(cpu)) {
+ } else {
unsigned long pstate;

- /* Disable interrupts. */
+ /* The sun4v sleeping code requires that we have PSTATE.IE cleared over
+ * the cpu sleep hypervisor call.
+ */
__asm__ __volatile__(
"rdpr %%pstate, %0\n\t"
"andn %0, %1, %0\n\t"
@@ -73,7 +70,7 @@ static void sparc64_yield(int cpu)
: "=&r" (pstate)
: "i" (PSTATE_IE));

- if (!need_resched() && !cpu_is_offline(cpu))
+ if (!need_resched() && !cpu_is_offline(smp_processor_id()))
sun4v_cpu_yield();

/* Re-enable interrupts. */
@@ -84,36 +81,16 @@ static void sparc64_yield(int cpu)
: "=&r" (pstate)
: "i" (PSTATE_IE));
}
-
- set_thread_flag(TIF_POLLING_NRFLAG);
+ local_irq_enable();
}

-/* The idle loop on sparc64. */
-void cpu_idle(void)
-{
- int cpu = smp_processor_id();
-
- set_thread_flag(TIF_POLLING_NRFLAG);
-
- while(1) {
- tick_nohz_idle_enter();
- rcu_idle_enter();
-
- while (!need_resched() && !cpu_is_offline(cpu))
- sparc64_yield(cpu);
-
- rcu_idle_exit();
- tick_nohz_idle_exit();
-
#ifdef CONFIG_HOTPLUG_CPU
- if (cpu_is_offline(cpu)) {
- sched_preempt_enable_no_resched();
- cpu_play_dead();
- }
-#endif
- schedule_preempt_disabled();
- }
+void arch_cpu_idle_dead()
+{
+ sched_preempt_enable_no_resched();
+ cpu_play_dead();
}
+#endif

#ifdef CONFIG_COMPAT
static void show_regwindow32(struct pt_regs *regs)
diff --git a/arch/sparc/kernel/smp_32.c b/arch/sparc/kernel/smp_32.c
index 9e7e6d7..e3f2b81 100644
--- a/arch/sparc/kernel/smp_32.c
+++ b/arch/sparc/kernel/smp_32.c
@@ -369,7 +369,7 @@ void __cpuinit sparc_start_secondary(void *arg)
local_irq_enable();

wmb();
- cpu_idle();
+ cpu_startup_entry(CPUHP_ONLINE);

/* We should never reach here! */
BUG();
diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c
index 537eb66..c025ffc 100644
--- a/arch/sparc/kernel/smp_64.c
+++ b/arch/sparc/kernel/smp_64.c
@@ -127,6 +127,8 @@ void __cpuinit smp_callin(void)

/* idle thread is expected to have preempt disabled */
preempt_disable();
+
+ cpu_startup_entry(CPUHP_ONLINE);
}

void cpu_panic(void)
diff --git a/arch/sparc/kernel/trampoline_64.S b/arch/sparc/kernel/trampoline_64.S
index da1b781..2e973a2 100644
--- a/arch/sparc/kernel/trampoline_64.S
+++ b/arch/sparc/kernel/trampoline_64.S
@@ -407,8 +407,7 @@ after_lock_tlb:

call smp_callin
nop
- call cpu_idle
- mov 0, %o0
+
call cpu_panic
nop
1: b,a,pt %xcc, 1b

2013-04-12 18:56:19

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v2] sparc: Use generic idle loop

On Thu, 11 Apr 2013, Sam Ravnborg wrote:

> Add generic cpu_idle support
>
> sparc32:
> - replace call to cpu_idle() with cpu_startup_entry()
> - add arch_cpu_idle()
>
> sparc64:
> - smp_callin() now include cpu_startup_entry() call so we can
> skip calling cpu_idle from assembler
> - add arch_cpu_idle() and arch_cpu_idle_dead()

Nice !

If Dave acks it, I'll pick it up

Thanks,

tglx


> Signed-off-by: Sam Ravnborg <[email protected]>
> Reviewed-by: "Srivatsa S. Bhat" <[email protected]>
> Cc: David S. Miller <[email protected]>
> ---
> v1->v2:
> - simplified arch_cpu_idle() - former sparc64_yield() based on comments from Srivatsa
> - dropped TIF_POLLING_NRFLAG handling
> - dropped smp_mb__after_clear_bit() - this is done in generic code using rmb()
> - dropped the out-most while (!need_resched() && !cpu_is_offline(cpu))
> - always enable irq on exit in arch_cpu_idle (both sparc32 and sparc64)
> - added comment from davem why we need to clear IE bit in sparc64 arch_cpu_idle()
>
> Sam
>
>
> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
> index 3d361f2..ee5eacc 100644
> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -37,6 +37,7 @@ config SPARC
> select GENERIC_SMP_IDLE_THREAD
> select GENERIC_CMOS_UPDATE
> select GENERIC_CLOCKEVENTS
> + select GENERIC_IDLE_LOOP
> select GENERIC_STRNCPY_FROM_USER
> select GENERIC_STRNLEN_USER
> select MODULES_USE_ELF_RELA
> diff --git a/arch/sparc/kernel/hvtramp.S b/arch/sparc/kernel/hvtramp.S
> index 9365432..605c960 100644
> --- a/arch/sparc/kernel/hvtramp.S
> +++ b/arch/sparc/kernel/hvtramp.S
> @@ -128,8 +128,7 @@ hv_cpu_startup:
>
> call smp_callin
> nop
> - call cpu_idle
> - mov 0, %o0
> +
> call cpu_panic
> nop
>
> diff --git a/arch/sparc/kernel/process_32.c b/arch/sparc/kernel/process_32.c
> index 62eede1..c852410 100644
> --- a/arch/sparc/kernel/process_32.c
> +++ b/arch/sparc/kernel/process_32.c
> @@ -64,23 +64,12 @@ extern void fpsave(unsigned long *, unsigned long *, void *, unsigned long *);
> struct task_struct *last_task_used_math = NULL;
> struct thread_info *current_set[NR_CPUS];
>
> -/*
> - * the idle loop on a Sparc... ;)
> - */
> -void cpu_idle(void)
> +/* Idle loop support. */
> +void arch_cpu_idle(void)
> {
> - set_thread_flag(TIF_POLLING_NRFLAG);
> -
> - /* endless idle loop with no priority at all */
> - for (;;) {
> - while (!need_resched()) {
> - if (sparc_idle)
> - (*sparc_idle)();
> - else
> - cpu_relax();
> - }
> - schedule_preempt_disabled();
> - }
> + if (sparc_idle)
> + (*sparc_idle)();
> + local_irq_enable();
> }
>
> /* XXX cli/sti -> local_irq_xxx here, check this works once SMP is fixed. */
> diff --git a/arch/sparc/kernel/process_64.c b/arch/sparc/kernel/process_64.c
> index cdb80b2..9fbf0d1 100644
> --- a/arch/sparc/kernel/process_64.c
> +++ b/arch/sparc/kernel/process_64.c
> @@ -52,20 +52,17 @@
>
> #include "kstack.h"
>
> -static void sparc64_yield(int cpu)
> +/* Idle loop support on sparc64. */
> +void arch_cpu_idle(void)
> {
> if (tlb_type != hypervisor) {
> touch_nmi_watchdog();
> - return;
> - }
> -
> - clear_thread_flag(TIF_POLLING_NRFLAG);
> - smp_mb__after_clear_bit();
> -
> - while (!need_resched() && !cpu_is_offline(cpu)) {
> + } else {
> unsigned long pstate;
>
> - /* Disable interrupts. */
> + /* The sun4v sleeping code requires that we have PSTATE.IE cleared over
> + * the cpu sleep hypervisor call.
> + */
> __asm__ __volatile__(
> "rdpr %%pstate, %0\n\t"
> "andn %0, %1, %0\n\t"
> @@ -73,7 +70,7 @@ static void sparc64_yield(int cpu)
> : "=&r" (pstate)
> : "i" (PSTATE_IE));
>
> - if (!need_resched() && !cpu_is_offline(cpu))
> + if (!need_resched() && !cpu_is_offline(smp_processor_id()))
> sun4v_cpu_yield();
>
> /* Re-enable interrupts. */
> @@ -84,36 +81,16 @@ static void sparc64_yield(int cpu)
> : "=&r" (pstate)
> : "i" (PSTATE_IE));
> }
> -
> - set_thread_flag(TIF_POLLING_NRFLAG);
> + local_irq_enable();
> }
>
> -/* The idle loop on sparc64. */
> -void cpu_idle(void)
> -{
> - int cpu = smp_processor_id();
> -
> - set_thread_flag(TIF_POLLING_NRFLAG);
> -
> - while(1) {
> - tick_nohz_idle_enter();
> - rcu_idle_enter();
> -
> - while (!need_resched() && !cpu_is_offline(cpu))
> - sparc64_yield(cpu);
> -
> - rcu_idle_exit();
> - tick_nohz_idle_exit();
> -
> #ifdef CONFIG_HOTPLUG_CPU
> - if (cpu_is_offline(cpu)) {
> - sched_preempt_enable_no_resched();
> - cpu_play_dead();
> - }
> -#endif
> - schedule_preempt_disabled();
> - }
> +void arch_cpu_idle_dead()
> +{
> + sched_preempt_enable_no_resched();
> + cpu_play_dead();
> }
> +#endif
>
> #ifdef CONFIG_COMPAT
> static void show_regwindow32(struct pt_regs *regs)
> diff --git a/arch/sparc/kernel/smp_32.c b/arch/sparc/kernel/smp_32.c
> index 9e7e6d7..e3f2b81 100644
> --- a/arch/sparc/kernel/smp_32.c
> +++ b/arch/sparc/kernel/smp_32.c
> @@ -369,7 +369,7 @@ void __cpuinit sparc_start_secondary(void *arg)
> local_irq_enable();
>
> wmb();
> - cpu_idle();
> + cpu_startup_entry(CPUHP_ONLINE);
>
> /* We should never reach here! */
> BUG();
> diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c
> index 537eb66..c025ffc 100644
> --- a/arch/sparc/kernel/smp_64.c
> +++ b/arch/sparc/kernel/smp_64.c
> @@ -127,6 +127,8 @@ void __cpuinit smp_callin(void)
>
> /* idle thread is expected to have preempt disabled */
> preempt_disable();
> +
> + cpu_startup_entry(CPUHP_ONLINE);
> }
>
> void cpu_panic(void)
> diff --git a/arch/sparc/kernel/trampoline_64.S b/arch/sparc/kernel/trampoline_64.S
> index da1b781..2e973a2 100644
> --- a/arch/sparc/kernel/trampoline_64.S
> +++ b/arch/sparc/kernel/trampoline_64.S
> @@ -407,8 +407,7 @@ after_lock_tlb:
>
> call smp_callin
> nop
> - call cpu_idle
> - mov 0, %o0
> +
> call cpu_panic
> nop
> 1: b,a,pt %xcc, 1b
>

2013-04-12 18:58:27

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v2] sparc: Use generic idle loop

From: Thomas Gleixner <[email protected]>
Date: Fri, 12 Apr 2013 20:56:10 +0200 (CEST)

> On Thu, 11 Apr 2013, Sam Ravnborg wrote:
>
>> Add generic cpu_idle support
>>
>> sparc32:
>> - replace call to cpu_idle() with cpu_startup_entry()
>> - add arch_cpu_idle()
>>
>> sparc64:
>> - smp_callin() now include cpu_startup_entry() call so we can
>> skip calling cpu_idle from assembler
>> - add arch_cpu_idle() and arch_cpu_idle_dead()
>
> Nice !
>
> If Dave acks it, I'll pick it up

Acked-by: David S. Miller <[email protected]>

Subject: [tip:smp/hotplug] sparc: Use generic idle loop

Commit-ID: 87fa05aeb3a5e8e21b1a5510eef6983650eff092
Gitweb: http://git.kernel.org/tip/87fa05aeb3a5e8e21b1a5510eef6983650eff092
Author: Sam Ravnborg <[email protected]>
AuthorDate: Thu, 11 Apr 2013 21:38:50 +0200
Committer: Thomas Gleixner <[email protected]>
CommitDate: Sat, 13 Apr 2013 21:36:27 +0200

sparc: Use generic idle loop

Add generic cpu_idle support

sparc32:
- replace call to cpu_idle() with cpu_startup_entry()
- add arch_cpu_idle()

sparc64:
- smp_callin() now include cpu_startup_entry() call so we can
skip calling cpu_idle from assembler
- add arch_cpu_idle() and arch_cpu_idle_dead()

Signed-off-by: Sam Ravnborg <[email protected]>
Reviewed-by: "Srivatsa S. Bhat" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Acked-by: David Miller <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/sparc/Kconfig | 1 +
arch/sparc/kernel/hvtramp.S | 3 +--
arch/sparc/kernel/process_32.c | 21 ++++-------------
arch/sparc/kernel/process_64.c | 49 +++++++++++----------------------------
arch/sparc/kernel/smp_32.c | 2 +-
arch/sparc/kernel/smp_64.c | 2 ++
arch/sparc/kernel/trampoline_64.S | 3 +--
7 files changed, 24 insertions(+), 57 deletions(-)

diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 3d361f2..ee5eacc 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -37,6 +37,7 @@ config SPARC
select GENERIC_SMP_IDLE_THREAD
select GENERIC_CMOS_UPDATE
select GENERIC_CLOCKEVENTS
+ select GENERIC_IDLE_LOOP
select GENERIC_STRNCPY_FROM_USER
select GENERIC_STRNLEN_USER
select MODULES_USE_ELF_RELA
diff --git a/arch/sparc/kernel/hvtramp.S b/arch/sparc/kernel/hvtramp.S
index 9365432..605c960 100644
--- a/arch/sparc/kernel/hvtramp.S
+++ b/arch/sparc/kernel/hvtramp.S
@@ -128,8 +128,7 @@ hv_cpu_startup:

call smp_callin
nop
- call cpu_idle
- mov 0, %o0
+
call cpu_panic
nop

diff --git a/arch/sparc/kernel/process_32.c b/arch/sparc/kernel/process_32.c
index 62eede1..c852410 100644
--- a/arch/sparc/kernel/process_32.c
+++ b/arch/sparc/kernel/process_32.c
@@ -64,23 +64,12 @@ extern void fpsave(unsigned long *, unsigned long *, void *, unsigned long *);
struct task_struct *last_task_used_math = NULL;
struct thread_info *current_set[NR_CPUS];

-/*
- * the idle loop on a Sparc... ;)
- */
-void cpu_idle(void)
+/* Idle loop support. */
+void arch_cpu_idle(void)
{
- set_thread_flag(TIF_POLLING_NRFLAG);
-
- /* endless idle loop with no priority at all */
- for (;;) {
- while (!need_resched()) {
- if (sparc_idle)
- (*sparc_idle)();
- else
- cpu_relax();
- }
- schedule_preempt_disabled();
- }
+ if (sparc_idle)
+ (*sparc_idle)();
+ local_irq_enable();
}

/* XXX cli/sti -> local_irq_xxx here, check this works once SMP is fixed. */
diff --git a/arch/sparc/kernel/process_64.c b/arch/sparc/kernel/process_64.c
index cdb80b2..9fbf0d1 100644
--- a/arch/sparc/kernel/process_64.c
+++ b/arch/sparc/kernel/process_64.c
@@ -52,20 +52,17 @@

#include "kstack.h"

-static void sparc64_yield(int cpu)
+/* Idle loop support on sparc64. */
+void arch_cpu_idle(void)
{
if (tlb_type != hypervisor) {
touch_nmi_watchdog();
- return;
- }
-
- clear_thread_flag(TIF_POLLING_NRFLAG);
- smp_mb__after_clear_bit();
-
- while (!need_resched() && !cpu_is_offline(cpu)) {
+ } else {
unsigned long pstate;

- /* Disable interrupts. */
+ /* The sun4v sleeping code requires that we have PSTATE.IE cleared over
+ * the cpu sleep hypervisor call.
+ */
__asm__ __volatile__(
"rdpr %%pstate, %0\n\t"
"andn %0, %1, %0\n\t"
@@ -73,7 +70,7 @@ static void sparc64_yield(int cpu)
: "=&r" (pstate)
: "i" (PSTATE_IE));

- if (!need_resched() && !cpu_is_offline(cpu))
+ if (!need_resched() && !cpu_is_offline(smp_processor_id()))
sun4v_cpu_yield();

/* Re-enable interrupts. */
@@ -84,36 +81,16 @@ static void sparc64_yield(int cpu)
: "=&r" (pstate)
: "i" (PSTATE_IE));
}
-
- set_thread_flag(TIF_POLLING_NRFLAG);
+ local_irq_enable();
}

-/* The idle loop on sparc64. */
-void cpu_idle(void)
-{
- int cpu = smp_processor_id();
-
- set_thread_flag(TIF_POLLING_NRFLAG);
-
- while(1) {
- tick_nohz_idle_enter();
- rcu_idle_enter();
-
- while (!need_resched() && !cpu_is_offline(cpu))
- sparc64_yield(cpu);
-
- rcu_idle_exit();
- tick_nohz_idle_exit();
-
#ifdef CONFIG_HOTPLUG_CPU
- if (cpu_is_offline(cpu)) {
- sched_preempt_enable_no_resched();
- cpu_play_dead();
- }
-#endif
- schedule_preempt_disabled();
- }
+void arch_cpu_idle_dead()
+{
+ sched_preempt_enable_no_resched();
+ cpu_play_dead();
}
+#endif

#ifdef CONFIG_COMPAT
static void show_regwindow32(struct pt_regs *regs)
diff --git a/arch/sparc/kernel/smp_32.c b/arch/sparc/kernel/smp_32.c
index 9e7e6d7..e3f2b81 100644
--- a/arch/sparc/kernel/smp_32.c
+++ b/arch/sparc/kernel/smp_32.c
@@ -369,7 +369,7 @@ void __cpuinit sparc_start_secondary(void *arg)
local_irq_enable();

wmb();
- cpu_idle();
+ cpu_startup_entry(CPUHP_ONLINE);

/* We should never reach here! */
BUG();
diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c
index 537eb66..c025ffc 100644
--- a/arch/sparc/kernel/smp_64.c
+++ b/arch/sparc/kernel/smp_64.c
@@ -127,6 +127,8 @@ void __cpuinit smp_callin(void)

/* idle thread is expected to have preempt disabled */
preempt_disable();
+
+ cpu_startup_entry(CPUHP_ONLINE);
}

void cpu_panic(void)
diff --git a/arch/sparc/kernel/trampoline_64.S b/arch/sparc/kernel/trampoline_64.S
index da1b781..2e973a2 100644
--- a/arch/sparc/kernel/trampoline_64.S
+++ b/arch/sparc/kernel/trampoline_64.S
@@ -407,8 +407,7 @@ after_lock_tlb:

call smp_callin
nop
- call cpu_idle
- mov 0, %o0
+
call cpu_panic
nop
1: b,a,pt %xcc, 1b

2013-05-03 09:47:49

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [patch 00/34] idle: Consolidate idle implementations

Hi Thomas,

On Thu, Mar 21, 2013 at 10:52 PM, Thomas Gleixner <[email protected]> wrote:
> Each architecture implements its own cpu_idle() code, which is more or
> less the same on all architectures (plus/minus a few bugs and a few
> missing extra functionalities, instrumentation ...). That also forces
> everyone who is interested in idle related features to add new
> functionality to every architecture. What a waste.

I noticed there are still prototypes for cpu_idle() in include/linux/cpu.h
and include/linux/smp.h, so I ran a full "git grep -w cpu_idle", which
suggested a few more places that needs updates:

Documentation/s390/Debugging390.txt:0001b304 T cpu_idle
Documentation/s390/Debugging390.txt:is cpu_idle+0x66 ( quiet the cpu
is asleep, don't wake it )
Documentation/scheduler/sched-arch.txt:Your cpu_idle routines need to
obey the following rules:
Documentation/scheduler/sched-arch.txt:3. When cpu_idle finds
(need_resched() == 'true'), it should call
Documentation/trace/events-power.txt:cpu_idle "state=%lu cpu_id=%lu"
Documentation/trace/ftrace.txt: <idle>-0 [002] .N.1
21169.031484: rcu_idle_exit <-cpu_idle
Documentation/trace/ftrace.txt: => cpu_idle
Documentation/trace/ftrace.txt: <idle>-0 3.N.1 11us :
rcu_idle_exit <-cpu_idle
Documentation/trace/ftrace.txt: <idle>-0 3.N.1 11us :
tick_nohz_idle_exit <-cpu_idle
Documentation/trace/ftrace.txt: <idle>-0 3.N.1 25us :
sub_preempt_count <-cpu_idle
Documentation/trace/ftrace.txt: <idle>-0 3.N.. 25us :
schedule <-cpu_idle
Documentation/trace/ftrace.txt: <idle>-0 2.N.2 3us :
cpu_idle: state=4294967295 cpu_id=2
Documentation/virtual/uml/UserModeLinux-HOWTO.txt: #3
0x100a5508 in cpu_idle () at process_kern.c:471
arch/arm/Kconfig: This option adds a write barrier to the
cpu_idle loop so that,
arch/cris/arch-v10/drivers/gpio.c: * from cpu_idle() in kernel/process.c
arch/cris/arch-v10/drivers/gpio.c: * The check in cpu_idle()
reduces latency from ~15 ms to ~6 ms
arch/cris/arch-v32/drivers/mach-fs/gpio.c: * from cpu_idle() in
kernel/process.c
arch/cris/arch-v32/drivers/mach-fs/gpio.c: * The check in
cpu_idle() reduces latency from ~15 ms to ~6 ms
arch/mips/kernel/smtc.c: printk("Dangling IXMT in cpu_idle()\n");
arch/powerpc/kernel/irq.c: * in cpu_idle() will properly re-enable everything.
arch/sparc/kernel/leon_pmc.c:/* leon_pmc.c: LEON Power-down cpu_idle() handler
arch/sparc/kernel/smp_32.c: printk("CPU[%d]: Returns from
cpu_idle!\n", smp_processor_id());
arch/sparc/kernel/smp_64.c: printk("CPU[%d]: Returns from
cpu_idle!\n", smp_processor_id());
arch/tile/include/asm/thread_info.h:/* Enable interrupts racelessly
and nap forever: helper for cpu_idle(). */
drivers/acpi/processor_driver.c: * Set flag to delay
cpu_idle/throttling initialization
include/linux/cpu.h:void cpu_idle(void);
include/linux/smp.h:extern void cpu_idle(void);
include/trace/events/power.h:DEFINE_EVENT(cpu, cpu_idle,
init/main.c: * cpu_idle.
init/main.c: /* Call into cpu_idle with preempt disabled */
init/main.c: * fragile until we cpu_idle() for the first time.
kernel/trace/power-traces.c:EXPORT_TRACEPOINT_SYMBOL_GPL(cpu_idle);
tools/perf/builtin-timechart.c: * Mapping all these
"power:cpu_idle" strings to the tracepoint
tools/perf/builtin-timechart.c: if (strcmp(event_str,
"power:cpu_idle") == 0) {
tools/perf/builtin-timechart.c: "-e", "power:cpu_idle",
tools/perf/builtin-timechart.c: if (!is_valid_tracepoint("power:cpu_idle") &&
tools/perf/builtin-top.c: "cpu_idle",

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds