[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230422082211.GA1215777@hirez.programming.kicks-ass.net>
Date: Sat, 22 Apr 2023 10:22:11 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Frederic Weisbecker <frederic@...nel.org>,
Huacai Chen <chenhuacai@...nel.org>,
WANG Xuerui <kernel@...0n.name>,
"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
Anna-Maria Behnsen <anna-maria@...utronix.de>,
LKML <linux-kernel@...r.kernel.org>, tsbogend@...ha.franken.de
Subject: Re: Loongson (and other $ARCHs?) idle VS timer enqueue
+ MIPS Thomas
On Sat, Apr 22, 2023 at 10:17:00AM +0200, Peter Zijlstra wrote:
> On Fri, Apr 21, 2023 at 05:24:36PM +0200, Thomas Gleixner wrote:
> > On Fri, Apr 21 2023 at 14:36, Frederic Weisbecker wrote:
> > > I'm looking at the __arch_cpu_idle() implementation in Loongarch
> > > and I'm wondering about the rollback code. I don't understand well
> > > that code but with the help from PeterZ I might have seen something,
> > > so tell me if I'm wrong: when an interrupt happens within
> > > __arch_cpu_idle(), handle_vint() rolls back the return value to the
> > > beginning of __arch_cpu_idle(), so that TIF_NEED_RESCHED is checked
> > > again. Is that correct?
> > >
> > > Because if an interrupt fires while in __arch_cpu_idle(), that IRQ
> > > might enqueue a new timer and that new timer needs to be reprogrammed
> > > from the main idle loop and just checking TIF_NEED_RESCHED doesn't
> > > tell about that information.
> >
> > The check for TIF_NEED_RESCHED as loop termination condition is simply
> > wrong. The architecture is not to supposed to loop in arch_cpu_idle().
> >
> > That loop is from Linux 0.9 days. Seems MIPS::__r4k_wait() and
> > loongarch, which copied that muck are still stuck in the 1990'ies.
> >
> > It has to return when an interrupt brings it out of the "idle wait"
> > instruction.
>
> So I think the below is enough for these two...
>
> diff --git a/arch/loongarch/kernel/genex.S b/arch/loongarch/kernel/genex.S
> index 44ff1ff64260..5a102ff80de0 100644
> --- a/arch/loongarch/kernel/genex.S
> +++ b/arch/loongarch/kernel/genex.S
> @@ -40,6 +40,7 @@ SYM_FUNC_START(handle_vint)
> ori t0, t0, 0x1f
> xori t0, t0, 0x1f
> bne t0, t1, 1f
> + addi.d t0, t0, 0x20
> LONG_S t0, sp, PT_ERA
> 1: move a0, sp
> move a1, sp
> diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S
> index b6de8e88c1bd..cd6aae441ad9 100644
> --- a/arch/mips/kernel/genex.S
> +++ b/arch/mips/kernel/genex.S
> @@ -140,6 +140,7 @@ LEAF(__r4k_wait)
> ori k0, 0x1f /* 32 byte rollback region */
> xori k0, 0x1f
> bne k0, k1, \handler
> + addiu k0, 0x20
> MTC0 k0, CP0_EPC
> .set pop
> .endm
Powered by blists - more mailing lists