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 <sam@xxxxxxxxxxxx>
> Cc: David S. Miller <davem@xxxxxxxxxxxxx>
>
> ---
> 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();
> }
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
