Re: Loongson (and other $ARCHs?) idle VS timer enqueue

From: Peter Zijlstra
Date: Tue Apr 25 2023 - 07:49:57 EST


On Sun, Apr 23, 2023 at 09:52:49PM +0800, bibo, mao wrote:

> > @@ -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
> It is more reasonable with this patch, this will jump out of idle function
> directly after interrupt returns. If so, can we remove checking
> _TIF_NEED_RESCHED in idle ASM function?
>
> > + move t0, CSR_CRMD_IE
> > + csrxchg t0, t0, LOONGARCH_CSR_CRMD
> - LONG_L t0, tp, TI_FLAGS
> + nop
> > nop
> - andi t0, t0, _TIF_NEED_RESCHED
> - bnez t0, 1f
> + nop
> + nop
> > nop
> > - nop
> > - nop
> > idle 0

Would not something like the below be a more compact form?
That is; afaict there is no reason to keep it 32 bytes, we can easily go
16 and drop 4 nops.

Additionally, instead of truncating to the start, increase to the end by
doing:

ip |= 0xf;
ip++;

Also; I added a wee comment.


diff --git a/arch/loongarch/kernel/genex.S b/arch/loongarch/kernel/genex.S
index 44ff1ff64260..3c8a6bab98fe 100644
--- a/arch/loongarch/kernel/genex.S
+++ b/arch/loongarch/kernel/genex.S
@@ -18,27 +18,31 @@

.align 5
SYM_FUNC_START(__arch_cpu_idle)
- /* start of rollback region */
- LONG_L t0, tp, TI_FLAGS
- nop
- andi t0, t0, _TIF_NEED_RESCHED
- bnez t0, 1f
- nop
- nop
- nop
+ /* start of idle interrupt region */
+ move t0, CSR_CRMD_IE
+ csrxchg t0, t0, LOONGARCH_CSR_CRMD
+ /*
+ * If an interrupt lands here; between enabling interrupts above and
+ * going idle on the next instruction, we must *NOT* go idle since the
+ * interrupt could have set TIF_NEED_RESCHED or caused an timer to need
+ * reprogramming. Fall through -- see handle_vint() below -- and have
+ * the idle loop take care of things.
+ */
idle 0
- /* end of rollback region */
-1: jr ra
+ nop
+ /* end of idle interrupt region */
+SYM_INNER_LBEL(__arch_cpu_idle_exit, SYM_L_LOCAL)
+ jr ra
SYM_FUNC_END(__arch_cpu_idle)

SYM_FUNC_START(handle_vint)
BACKUP_T0T1
SAVE_ALL
- la_abs t1, __arch_cpu_idle
+ la_abs t1, __arch_cpu_idle_exit
LONG_L t0, sp, PT_ERA
- /* 32 byte rollback region */
- ori t0, t0, 0x1f
- xori t0, t0, 0x1f
+ /* 16 byte idle interrupt region */
+ ori t0, t0, 0x0f
+ addi.d t0, t0, 1
bne t0, t1, 1f
LONG_S t0, sp, PT_ERA
1: move a0, sp
diff --git a/arch/loongarch/kernel/idle.c b/arch/loongarch/kernel/idle.c
index 0b5dd2faeb90..5ba72d229920 100644
--- a/arch/loongarch/kernel/idle.c
+++ b/arch/loongarch/kernel/idle.c
@@ -11,7 +11,6 @@

void __cpuidle arch_cpu_idle(void)
{
- raw_local_irq_enable();
__arch_cpu_idle(); /* idle instruction needs irq enabled */
raw_local_irq_disable();
}