[PATCH 00/14] entry: preempt_schedule_irq() callers scrub

[PATCH 00/14] entry: preempt_schedule_irq() callers scrub

[Valentin Schneider]

Hi,

This is the continuation of [1] where I'm hunting down
preempt_schedule_irq() callers because of [2].

I told myself the best way to get this moving forward wouldn't be to write
doc about it, but to go write some fixes and get some discussions going,
which is what this patch-set is about.

I've looked at users of preempt_schedule_irq(), and made sure they didn't
have one of those useless loops. The list of offenders is:

$ grep -r -I "preempt_schedule_irq" arch/ | cut -d/ -f2 | sort | uniq

  arc
  arm
  arm64
  c6x
  csky
  h8300
  ia64
  m68k
  microblaze
  mips
  nds32
  nios2
  parisc
  powerpc
  riscv
  s390
  sh
  sparc
  x86
  xtensa

Regarding that loop, archs seem to fall in 3 categories:
A) Those that don't have the loop
B) Those that have a small need_resched() loop around the
   preempt_schedule_irq() callsite
C) Those that branch to some more generic code further up the entry code
   and eventually branch back to preempt_schedule_irq()

arc, m68k, nios2 fall in A)
sparc, ia64, s390 fall in C)
all the others fall in B)

I've written patches for B) and C) EXCEPT for ia64 and s390 because I
haven't been able to tell if it's actually fine to kill that "long jump"
(and maybe I'm wrong on sparc). Hopefully folks who understand what goes on
in there might be able to shed some light.

Also, since I sent patches for arm & arm64 in [1] I'm not including them
here.

Boot-tested on:
- x86

Build-tested on:
- h8300
- c6x
- powerpc
- mips
- nds32
- microblaze
- sparc
- xtensa

Thanks,
Valentin

[1]: https://lore.kernel.org/lkml/20190131182339.9835-1-valentin.schneider@arm.com/
[2]: https://lore.kernel.org/lkml/cc989920-a13b-d53b-db83-1584a7f53edc@arm.com/

Valentin Schneider (14):
  sched/core: Fix preempt_schedule() interrupt return comment
  c6x: entry: Remove unneeded need_resched() loop
  csky: entry: Remove unneeded need_resched() loop
  h8300: entry: Remove unneeded need_resched() loop
  microblaze: entry: Remove unneeded need_resched() loop
  MIPS: entry: Remove unneeded need_resched() loop
  nds32: ex-exit: Remove unneeded need_resched() loop
  powerpc: entry: Remove unneeded need_resched() loop
  RISC-V: entry: Remove unneeded need_resched() loop
  sh: entry: Remove unneeded need_resched() loop
  sh64: entry: Remove unneeded need_resched() loop
  sparc64: rtrap: Remove unneeded need_resched() loop
  x86/entry: Remove unneeded need_resched() loop
  xtensa: entry: Remove unneeded need_resched() loop

 arch/c6x/kernel/entry.S        | 3 +--
 arch/csky/kernel/entry.S       | 4 ----
 arch/h8300/kernel/entry.S      | 3 +--
 arch/microblaze/kernel/entry.S | 5 -----
 arch/mips/kernel/entry.S       | 3 +--
 arch/nds32/kernel/ex-exit.S    | 4 ++--
 arch/powerpc/kernel/entry_32.S | 6 +-----
 arch/powerpc/kernel/entry_64.S | 8 +-------
 arch/riscv/kernel/entry.S      | 3 +--
 arch/sh/kernel/cpu/sh5/entry.S | 5 +----
 arch/sh/kernel/entry-common.S  | 4 +---
 arch/sparc/kernel/rtrap_64.S   | 1 -
 arch/x86/entry/entry_32.S      | 3 +--
 arch/x86/entry/entry_64.S      | 3 +--
 arch/xtensa/kernel/entry.S     | 2 +-
 kernel/sched/core.c            | 7 +++----
 16 files changed, 16 insertions(+), 48 deletions(-)

--
2.20.1

[PATCH 01/14] sched/core: Fix preempt_schedule() interrupt return comment

[Valentin Schneider]

preempt_schedule_irq() is the one that should be called on return from
interrupt, clean up the comment to avoid any ambiguity.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/sched/core.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index ead464a0f2e5..78c5aaa28278 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3652,9 +3652,8 @@ static void __sched notrace preempt_schedule_common(void)
 
 #ifdef CONFIG_PREEMPT
 /*
- * this is the entry point to schedule() from in-kernel preemption
- * off of preempt_enable. Kernel preemptions off return from interrupt
- * occur there and call schedule directly.
+ * This is the entry point to schedule() from in-kernel preemption
+ * off of preempt_enable.
  */
 asmlinkage __visible void __sched notrace preempt_schedule(void)
 {
@@ -3725,7 +3724,7 @@ EXPORT_SYMBOL_GPL(preempt_schedule_notrace);
 #endif /* CONFIG_PREEMPT */
 
 /*
- * this is the entry point to schedule() from kernel preemption
+ * This is the entry point to schedule() from kernel preemption
  * off of irq context.
  * Note, that this is called and return with irqs disabled. This will
  * protect us against recursive calling from irq.
--
2.20.1

[PATCH 02/14] c6x: entry: Remove unneeded need_resched() loop

[Valentin Schneider]

Since the enabling and disabling of IRQs within preempt_schedule_irq()
is contained in a need_resched() loop, we don't need the outer arch
code loop.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Cc: Mark Salter <msalter@redhat.com>
Cc: Aurelien Jacquiot <jacquiot.aurelien@gmail.com>
Cc: linux-c6x-dev@linux-c6x.org
---
 arch/c6x/kernel/entry.S | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/c6x/kernel/entry.S b/arch/c6x/kernel/entry.S
index 2721c90b0121..17926e942edb 100644
--- a/arch/c6x/kernel/entry.S
+++ b/arch/c6x/kernel/entry.S
@@ -567,7 +567,6 @@ resume_kernel:
 	NOP	4
  [A1]	BNOP	.S2	restore_all,5
 
-preempt_schedule:
 	GET_THREAD_INFO A2
 	LDW	.D1T1	*+A2(THREAD_INFO_FLAGS),A1
 #ifdef CONFIG_C6X_BIG_KERNEL
@@ -584,7 +583,7 @@ preempt_schedule:
 #else
 	B	.S2	preempt_schedule_irq
 #endif
-	ADDKPC	.S2	preempt_schedule,B3,4
+	ADDKPC	.S2	restore_all,B3,4
 #endif /* CONFIG_PREEMPT */
 
 ENTRY(enable_exception)
-- 
2.20.1

[PATCH 03/14] csky: entry: Remove unneeded need_resched() loop

[Valentin Schneider]

Since the enabling and disabling of IRQs within preempt_schedule_irq()
is contained in a need_resched() loop, we don't need the outer arch
code loop.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Cc: Guo Ren <guoren@kernel.org>
---
 arch/csky/kernel/entry.S | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/csky/kernel/entry.S b/arch/csky/kernel/entry.S
index 5137ed9062bd..2d9e4776ba7a 100644
--- a/arch/csky/kernel/entry.S
+++ b/arch/csky/kernel/entry.S
@@ -307,11 +307,7 @@ ENTRY(csky_irq)
 	ldw	r8, (r9, TINFO_FLAGS)
 	btsti	r8, TIF_NEED_RESCHED
 	bf	2f
-1:
 	jbsr	preempt_schedule_irq	/* irq en/disable is done inside */
-	ldw	r7, (r9, TINFO_FLAGS)	/* get new tasks TI_FLAGS */
-	btsti	r7, TIF_NEED_RESCHED
-	bt	1b			/* go again */
 #endif
 2:
 	jmpi	ret_from_exception
-- 
2.20.1

[PATCH 04/14] h8300: entry: Remove unneeded need_resched() loop

[Valentin Schneider]

Since the enabling and disabling of IRQs within preempt_schedule_irq()
is contained in a need_resched() loop, we don't need the outer arch
code loop.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: uclinux-h8-devel@lists.sourceforge.jp
---
 arch/h8300/kernel/entry.S | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/h8300/kernel/entry.S b/arch/h8300/kernel/entry.S
index 4ade5f8299ba..6bde028e7d4a 100644
--- a/arch/h8300/kernel/entry.S
+++ b/arch/h8300/kernel/entry.S
@@ -323,7 +323,6 @@ restore_all:
 resume_kernel:
 	mov.l	@(TI_PRE_COUNT:16,er4),er0
 	bne	restore_all:8
-need_resched:
 	mov.l	@(TI_FLAGS:16,er4),er0
 	btst	#TIF_NEED_RESCHED,r0l
 	beq	restore_all:8
@@ -332,7 +331,7 @@ need_resched:
 	mov.l	sp,er0
 	jsr	@set_esp0
 	jsr	@preempt_schedule_irq
-	bra	need_resched:8
+	bra	restore_all:8
 #endif
 
 ret_from_fork:
-- 
2.20.1

[PATCH 05/14] microblaze: entry: Remove unneeded need_resched() loop

[Valentin Schneider]

Since the enabling and disabling of IRQs within preempt_schedule_irq()
is contained in a need_resched() loop, we don't need the outer arch
code loop.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Cc: Michal Simek <monstr@monstr.eu>
---
 arch/microblaze/kernel/entry.S | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/microblaze/kernel/entry.S b/arch/microblaze/kernel/entry.S
index 4e1b567becd6..de7083bd1d24 100644
--- a/arch/microblaze/kernel/entry.S
+++ b/arch/microblaze/kernel/entry.S
@@ -738,14 +738,9 @@ no_intr_resched:
 	andi	r5, r5, _TIF_NEED_RESCHED;
 	beqi	r5, restore /* if zero jump over */
 
-preempt:
 	/* interrupts are off that's why I am calling preempt_chedule_irq */
 	bralid	r15, preempt_schedule_irq
 	nop
-	lwi	r11, CURRENT_TASK, TS_THREAD_INFO;	/* get thread info */
-	lwi	r5, r11, TI_FLAGS;		/* get flags in thread info */
-	andi	r5, r5, _TIF_NEED_RESCHED;
-	bnei	r5, preempt /* if non zero jump to resched */
 restore:
 #endif
 	VM_OFF /* MS: turn off MMU */
-- 
2.20.1

[PATCH 06/14] MIPS: entry: Remove unneeded need_resched() loop

[Valentin Schneider]

Since the enabling and disabling of IRQs within preempt_schedule_irq()
is contained in a need_resched() loop, we don't need the outer arch
code loop.

Do note that commit a18815abcdfd ("Use preempt_schedule_irq.")
initially removed the existing loop, but commit cdaed73afb61 ("Fix
preemption bug.") reintroduced it. It is however not clear what the
issue was and why such a loop was reintroduced, so I'm probably
missing something.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paul.burton@mips.com>
Cc: James Hogan <jhogan@kernel.org>
Cc: linux-mips@vger.kernel.org
---
 arch/mips/kernel/entry.S | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/mips/kernel/entry.S b/arch/mips/kernel/entry.S
index d7de8adcfcc8..2240faeda62a 100644
--- a/arch/mips/kernel/entry.S
+++ b/arch/mips/kernel/entry.S
@@ -58,7 +58,6 @@ resume_kernel:
 	local_irq_disable
 	lw	t0, TI_PRE_COUNT($28)
 	bnez	t0, restore_all
-need_resched:
 	LONG_L	t0, TI_FLAGS($28)
 	andi	t1, t0, _TIF_NEED_RESCHED
 	beqz	t1, restore_all
@@ -66,7 +65,7 @@ need_resched:
 	andi	t0, 1
 	beqz	t0, restore_all
 	jal	preempt_schedule_irq
-	b	need_resched
+	j	restore_all
 #endif
 
 FEXPORT(ret_from_kernel_thread)
-- 
2.20.1

[PATCH 07/14] nds32: ex-exit: Remove unneeded need_resched() loop

[Valentin Schneider]

Since the enabling and disabling of IRQs within preempt_schedule_irq()
is contained in a need_resched() loop, we don't need the outer arch
code loop.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Cc: Greentime Hu <green.hu@gmail.com>
Cc: Vincent Chen <deanbo422@gmail.com>
---
 arch/nds32/kernel/ex-exit.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/nds32/kernel/ex-exit.S b/arch/nds32/kernel/ex-exit.S
index 97ba15cd4180..1df02a793364 100644
--- a/arch/nds32/kernel/ex-exit.S
+++ b/arch/nds32/kernel/ex-exit.S
@@ -163,7 +163,7 @@ resume_kernel:
 	gie_disable
 	lwi	$t0, [tsk+#TSK_TI_PREEMPT]
 	bnez	$t0, no_work_pending
-need_resched:
+
 	lwi	$t0, [tsk+#TSK_TI_FLAGS]
 	andi	$p1, $t0, #_TIF_NEED_RESCHED
 	beqz	$p1, no_work_pending
@@ -173,7 +173,7 @@ need_resched:
 	beqz	$t0, no_work_pending
 
 	jal	preempt_schedule_irq
-	b	need_resched
+	b	no_work_pending
 #endif
 
 /*
-- 
2.20.1

[PATCH 08/14] powerpc: entry: Remove unneeded need_resched() loop

[Valentin Schneider]

Since the enabling and disabling of IRQs within preempt_schedule_irq()
is contained in a need_resched() loop, we don't need the outer arch
code loop.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
---
 arch/powerpc/kernel/entry_32.S | 6 +-----
 arch/powerpc/kernel/entry_64.S | 8 +-------
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 0768dfd8a64e..ff3fe3824a4a 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -896,11 +896,7 @@ resume_kernel:
 	 */
 	bl	trace_hardirqs_off
 #endif
-1:	bl	preempt_schedule_irq
-	CURRENT_THREAD_INFO(r9, r1)
-	lwz	r3,TI_FLAGS(r9)
-	andi.	r0,r3,_TIF_NEED_RESCHED
-	bne-	1b
+	bl	preempt_schedule_irq
 #ifdef CONFIG_TRACE_IRQFLAGS
 	/* And now, to properly rebalance the above, we tell lockdep they
 	 * are being turned back on, which will happen when we return
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 435927f549c4..9c86c6826856 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -857,13 +857,7 @@ resume_kernel:
 	 * sure we are soft-disabled first and reconcile irq state.
 	 */
 	RECONCILE_IRQ_STATE(r3,r4)
-1:	bl	preempt_schedule_irq
-
-	/* Re-test flags and eventually loop */
-	CURRENT_THREAD_INFO(r9, r1)
-	ld	r4,TI_FLAGS(r9)
-	andi.	r0,r4,_TIF_NEED_RESCHED
-	bne	1b
+	bl	preempt_schedule_irq
 
 	/*
 	 * arch_local_irq_restore() from preempt_schedule_irq above may
-- 
2.20.1

[PATCH 09/14] RISC-V: entry: Remove unneeded need_resched() loop

[Valentin Schneider]

Since the enabling and disabling of IRQs within preempt_schedule_irq()
is contained in a need_resched() loop, we don't need the outer arch
code loop.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Cc: Palmer Dabbelt <palmer@sifive.com>
Cc: Albert Ou <aou@eecs.berkeley.edu>
Cc: linux-riscv@lists.infradead.org
---
 arch/riscv/kernel/entry.S | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index fd9b57c8b4ce..18e3337e9c30 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -258,12 +258,11 @@ restore_all:
 resume_kernel:
 	REG_L s0, TASK_TI_PREEMPT_COUNT(tp)
 	bnez s0, restore_all
-need_resched:
 	REG_L s0, TASK_TI_FLAGS(tp)
 	andi s0, s0, _TIF_NEED_RESCHED
 	beqz s0, restore_all
 	call preempt_schedule_irq
-	j need_resched
+	j restore_all
 #endif
 
 work_pending:
-- 
2.20.1

[PATCH 10/14] sh: entry: Remove unneeded need_resched() loop

[Valentin Schneider]

Since the enabling and disabling of IRQs within preempt_schedule_irq()
is contained in a need_resched() loop, we don't need the outer arch
code loop.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: linux-sh@vger.kernel.org
---
 arch/sh/kernel/entry-common.S | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/sh/kernel/entry-common.S b/arch/sh/kernel/entry-common.S
index d31f66e82ce5..65a105de52a0 100644
--- a/arch/sh/kernel/entry-common.S
+++ b/arch/sh/kernel/entry-common.S
@@ -93,7 +93,7 @@ ENTRY(resume_kernel)
 	mov.l	@(TI_PRE_COUNT,r8), r0	! current_thread_info->preempt_count
 	tst	r0, r0
 	bf	noresched
-need_resched:
+
 	mov.l	@(TI_FLAGS,r8), r0	! current_thread_info->flags
 	tst	#_TIF_NEED_RESCHED, r0	! need_resched set?
 	bt	noresched
@@ -107,8 +107,6 @@ need_resched:
 	mov.l	1f, r0
 	jsr	@r0			! call preempt_schedule_irq
 	 nop
-	bra	need_resched
-	 nop
 
 noresched:
 	bra	__restore_all
-- 
2.20.1

[PATCH 11/14] sh64: entry: Remove unneeded need_resched() loop

[Valentin Schneider]

Since the enabling and disabling of IRQs within preempt_schedule_irq()
is contained in a need_resched() loop, we don't need the outer arch
code loop.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: linux-sh@vger.kernel.org
---
 arch/sh/kernel/cpu/sh5/entry.S | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/sh/kernel/cpu/sh5/entry.S b/arch/sh/kernel/cpu/sh5/entry.S
index de68ffdfffbf..40e6d9a7a6a2 100644
--- a/arch/sh/kernel/cpu/sh5/entry.S
+++ b/arch/sh/kernel/cpu/sh5/entry.S
@@ -897,7 +897,6 @@ resume_kernel:
 	ld.l	r6, TI_PRE_COUNT, r7
 	beq/u	r7, ZERO, tr0
 
-need_resched:
 	ld.l	r6, TI_FLAGS, r7
 	movi	(1 << TIF_NEED_RESCHED), r8
 	and	r8, r7, r8
@@ -911,9 +910,7 @@ need_resched:
 	ori	r7, 1, r7
 	ptabs	r7, tr1
 	blink	tr1, LINK
-
-	pta	need_resched, tr1
-	blink	tr1, ZERO
+	blink   tr0, ZERO
 #endif
 
 	.global ret_from_syscall
-- 
2.20.1

[PATCH 12/14] sparc64: rtrap: Remove unneeded need_resched() loop

[Valentin Schneider]

Since the enabling and disabling of IRQs within preempt_schedule_irq()
is contained in a need_resched() loop, we don't need the outer arch
code loop.

We seem to be looping back somewhere much higher than the usual
preempt/need_resched checks, but AFAICT we would just branch to
'rtrap_no_irq_enable' and then back to 'to_kernel', which is
a need_resched() loop with a few extra steps.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: sparclinux@vger.kernel.org
---
 arch/sparc/kernel/rtrap_64.S | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/sparc/kernel/rtrap_64.S b/arch/sparc/kernel/rtrap_64.S
index 29aa34f11720..57fb75686957 100644
--- a/arch/sparc/kernel/rtrap_64.S
+++ b/arch/sparc/kernel/rtrap_64.S
@@ -322,7 +322,6 @@ to_kernel:
 		 nop
 		call			preempt_schedule_irq
 		 nop
-		ba,pt			%xcc, rtrap
 #endif
 kern_fpucheck:	ldub			[%g6 + TI_FPDEPTH], %l5
 		brz,pt			%l5, rt_continue
-- 
2.20.1

[PATCH 13/14] x86/entry: Remove unneeded need_resched() loop

[Valentin Schneider]

Since the enabling and disabling of IRQs within preempt_schedule_irq()
is contained in a need_resched() loop, we don't need the outer arch
code loop.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
---
 arch/x86/entry/entry_32.S | 3 +--
 arch/x86/entry/entry_64.S | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index d309f30cf7af..b1856fe9decf 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -766,13 +766,12 @@ END(ret_from_exception)
 #ifdef CONFIG_PREEMPT
 ENTRY(resume_kernel)
 	DISABLE_INTERRUPTS(CLBR_ANY)
-.Lneed_resched:
 	cmpl	$0, PER_CPU_VAR(__preempt_count)
 	jnz	restore_all_kernel
 	testl	$X86_EFLAGS_IF, PT_EFLAGS(%esp)	# interrupts off (exception path) ?
 	jz	restore_all_kernel
 	call	preempt_schedule_irq
-	jmp	.Lneed_resched
+	jmp	restore_all_kernel
 END(resume_kernel)
 #endif
 
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 1f0efdb7b629..e7e270603fe7 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -645,10 +645,9 @@ retint_kernel:
 	/* Check if we need preemption */
 	btl	$9, EFLAGS(%rsp)		/* were interrupts off? */
 	jnc	1f
-0:	cmpl	$0, PER_CPU_VAR(__preempt_count)
+	cmpl	$0, PER_CPU_VAR(__preempt_count)
 	jnz	1f
 	call	preempt_schedule_irq
-	jmp	0b
 1:
 #endif
 	/*
-- 
2.20.1

[PATCH 14/14] xtensa: entry: Remove unneeded need_resched() loop

[Valentin Schneider]

Since the enabling and disabling of IRQs within preempt_schedule_irq()
is contained in a need_resched() loop, we don't need the outer arch
code loop.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Cc: Chris Zankel <chris@zankel.net>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: linux-xtensa@linux-xtensa.org
---
 arch/xtensa/kernel/entry.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/xtensa/kernel/entry.S b/arch/xtensa/kernel/entry.S
index e50f5124dc6f..43ecaac14ae6 100644
--- a/arch/xtensa/kernel/entry.S
+++ b/arch/xtensa/kernel/entry.S
@@ -529,7 +529,7 @@ common_exception_return:
 	l32i	a4, a2, TI_PRE_COUNT
 	bnez	a4, 4f
 	call4	preempt_schedule_irq
-	j	1b
+	j	4f
 #endif
 
 #if XTENSA_FAKE_NMI
-- 
2.20.1

Re: [PATCH 12/14] sparc64: rtrap: Remove unneeded need_resched() loop

[David Miller]

From: Valentin Schneider <valentin.schneider@arm.com>
Date: Mon, 11 Mar 2019 22:47:50 +0000

> Since the enabling and disabling of IRQs within preempt_schedule_irq()
> is contained in a need_resched() loop, we don't need the outer arch
> code loop.
> 
> We seem to be looping back somewhere much higher than the usual
> preempt/need_resched checks, but AFAICT we would just branch to
> 'rtrap_no_irq_enable' and then back to 'to_kernel', which is
> a need_resched() loop with a few extra steps.
> 
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: sparclinux@vger.kernel.org
> ---
>  arch/sparc/kernel/rtrap_64.S | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/sparc/kernel/rtrap_64.S b/arch/sparc/kernel/rtrap_64.S
> index 29aa34f11720..57fb75686957 100644
> --- a/arch/sparc/kernel/rtrap_64.S
> +++ b/arch/sparc/kernel/rtrap_64.S
> @@ -322,7 +322,6 @@ to_kernel:
>  		 nop
>  		call			preempt_schedule_irq
>  		 nop
> -		ba,pt			%xcc, rtrap
>  #endif
>  kern_fpucheck:	ldub			[%g6 + TI_FPDEPTH], %l5
>  		brz,pt			%l5, rt_continue
> -- 
> 2.20.1
> 

We must re-evaluate the %tstate value stored in ptregs, you cannot
make this change.

Re: [PATCH 14/14] xtensa: entry: Remove unneeded need_resched() loop

[Max Filippov]

On Mon, Mar 11, 2019 at 3:48 PM Valentin Schneider
<valentin.schneider@arm.com> wrote:
>
> Since the enabling and disabling of IRQs within preempt_schedule_irq()
> is contained in a need_resched() loop, we don't need the outer arch
> code loop.
>
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> Cc: Chris Zankel <chris@zankel.net>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Cc: linux-xtensa@linux-xtensa.org
> ---
>  arch/xtensa/kernel/entry.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Max Filippov <jcmvbkbc@gmail.com>

-- 
Thanks.
-- Max

Re: [PATCH 12/14] sparc64: rtrap: Remove unneeded need_resched() loop

[Valentin Schneider]

Hi,

On 11/03/2019 23:13, David Miller wrote:
[...]
> 
> We must re-evaluate the %tstate value stored in ptregs, you cannot
> make this change.
> 

That's the one I was the less sure about, thanks for clearing it up and
sorry for the noise.

Re: [PATCH 14/14] xtensa: entry: Remove unneeded need_resched() loop

[Valentin Schneider]

On 12/03/2019 01:10, Max Filippov wrote:
[...]
> 
> Acked-by: Max Filippov <jcmvbkbc@gmail.com>
> 

Thanks!

Re: [PATCH 00/14] entry: preempt_schedule_irq() callers scrub

[Vineet Gupta]

On 3/11/19 3:47 PM, Valentin Schneider wrote:
> Hi,
> 
> This is the continuation of [1] where I'm hunting down
> preempt_schedule_irq() callers because of [2].
> 
> I told myself the best way to get this moving forward wouldn't be to write
> doc about it, but to go write some fixes and get some discussions going,
> which is what this patch-set is about.
> 
> I've looked at users of preempt_schedule_irq(), and made sure they didn't
> have one of those useless loops. The list of offenders is:
> 
> $ grep -r -I "preempt_schedule_irq" arch/ | cut -d/ -f2 | sort | uniq
> 
...

> 
> Regarding that loop, archs seem to fall in 3 categories:
> A) Those that don't have the loop

Please clarify that this is the right thing to do (since core code already has the
loop) hence no fixing is required for this "category"

> B) Those that have a small need_resched() loop around the
>    preempt_schedule_irq() callsite
> C) Those that branch to some more generic code further up the entry code
>    and eventually branch back to preempt_schedule_irq()
> 
> arc, m68k, nios2 fall in A)

> sparc, ia64, s390 fall in C)
> all the others fall in B)
> 
> I've written patches for B) and C) EXCEPT for ia64 and s390 because I
> haven't been able to tell if it's actually fine to kill that "long jump"
> (and maybe I'm wrong on sparc). Hopefully folks who understand what goes on
> in there might be able to shed some light.

Re: [PATCH 00/14] entry: preempt_schedule_irq() callers scrub

[Valentin Schneider]

On 12/03/2019 18:03, Vineet Gupta wrote:
[...]
>> Regarding that loop, archs seem to fall in 3 categories:
>> A) Those that don't have the loop
> 
> Please clarify that this is the right thing to do (since core code already has the
> loop) hence no fixing is required for this "category"
> 

Right, those don't need any change. I had a brief look at them to double
check they had the proper need_resched() gate before calling
preempt_schedule_irq() (with no loop) and they all seem fine. Also...

>> B) Those that have a small need_resched() loop around the
>>    preempt_schedule_irq() callsite
>> C) Those that branch to some more generic code further up the entry code
>>    and eventually branch back to preempt_schedule_irq()
>>
>> arc, m68k, nios2 fall in A)
> 

I forgot to include parisc in here.

[...]

Re: [PATCH 02/14] c6x: entry: Remove unneeded need_resched() loop

[Mark Salter]

On Mon, 2019-03-11 at 22:47 +0000, Valentin Schneider wrote:
> Since the enabling and disabling of IRQs within preempt_schedule_irq()
> is contained in a need_resched() loop, we don't need the outer arch
> code loop.
> 
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: Aurelien Jacquiot <jacquiot.aurelien@gmail.com>
> Cc: linux-c6x-dev@linux-c6x.org
> ---
>  arch/c6x/kernel/entry.S | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/c6x/kernel/entry.S b/arch/c6x/kernel/entry.S
> index 2721c90b0121..17926e942edb 100644
> --- a/arch/c6x/kernel/entry.S
> +++ b/arch/c6x/kernel/entry.S
> @@ -567,7 +567,6 @@ resume_kernel:
>  	NOP	4
>   [A1]	BNOP	.S2	restore_all,5
>  
> -preempt_schedule:
>  	GET_THREAD_INFO A2
>  	LDW	.D1T1	*+A2(THREAD_INFO_FLAGS),A1
>  #ifdef CONFIG_C6X_BIG_KERNEL
> @@ -584,7 +583,7 @@ preempt_schedule:
>  #else
>  	B	.S2	preempt_schedule_irq
>  #endif
> -	ADDKPC	.S2	preempt_schedule,B3,4
> +	ADDKPC	.S2	restore_all,B3,4
>  #endif /* CONFIG_PREEMPT */
>  
>  ENTRY(enable_exception)

Acked-by: Mark Salter <msalter@redhat.com>

Re: [PATCH 06/14] MIPS: entry: Remove unneeded need_resched() loop

[Paul Burton]

Hi Valentin,

On Mon, Mar 11, 2019 at 10:47:44PM +0000, Valentin Schneider wrote:
> Since the enabling and disabling of IRQs within preempt_schedule_irq()
> is contained in a need_resched() loop, we don't need the outer arch
> code loop.
> 
> Do note that commit a18815abcdfd ("Use preempt_schedule_irq.")
> initially removed the existing loop, but commit cdaed73afb61 ("Fix
> preemption bug.") reintroduced it. It is however not clear what the
> issue was and why such a loop was reintroduced, so I'm probably
> missing something.

It looks to me like commit a18815abcdfd ("Use preempt_schedule_irq.")
forgot the branch to restore_all, so would have fallen through to
ret_from_fork() & done weird things.

Adding the branch to restore_all as you're doing here would have been a
better fix than commit cdaed73afb61 ("Fix preemption bug.").

> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Paul Burton <paul.burton@mips.com>
> Cc: James Hogan <jhogan@kernel.org>
> Cc: linux-mips@vger.kernel.org
> ---
>  arch/mips/kernel/entry.S | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/mips/kernel/entry.S b/arch/mips/kernel/entry.S
> index d7de8adcfcc8..2240faeda62a 100644
> --- a/arch/mips/kernel/entry.S
> +++ b/arch/mips/kernel/entry.S
> @@ -58,7 +58,6 @@ resume_kernel:
>  	local_irq_disable
>  	lw	t0, TI_PRE_COUNT($28)
>  	bnez	t0, restore_all
> -need_resched:
>  	LONG_L	t0, TI_FLAGS($28)
>  	andi	t1, t0, _TIF_NEED_RESCHED
>  	beqz	t1, restore_all
> @@ -66,7 +65,7 @@ need_resched:
>  	andi	t0, 1
>  	beqz	t0, restore_all
>  	jal	preempt_schedule_irq
> -	b	need_resched
> +	j	restore_all

One nit - why change from branch to jump? It's not a big deal, but I'd
prefer we stick with the branch ("b") instruction for a few reasons:

- restore_all is nearby so there's no issue with it being out of range
  of a branch in any variation of the MIPS ISA.

- It's more consistent with the future of the MIPS architecture, eg.
  nanoMIPS in which branch instructions all use PC-relative immediate
  offsets & jump instructions are always of the "register" variety where
  the destination is specified by a register rather than an immediate
  encoded in the instruction (the assembler will fix it up & emit a
  branch anyway, but I generally prefer to invoke less magic in these
  areas...).

- A PC-relative branch won't generate an extra reloc in a relocatable
  kernel, whereas a jump will.

Even better would be if we take advantage of this being a tail call & do
this:

	PTR_LA	ra, restore_all
	j	preempt_schedule_irq

(Where I left the call to preempt_schedule_irq using a jump because it
may be further away.)

Though I don't mind if you wanna just s/j/b/ & leave the tail call
optimisation for someone else to do as a later change.

Thanks,
    Paul

>  #endif
>  
>  FEXPORT(ret_from_kernel_thread)
> -- 
> 2.20.1
> 

Re: [PATCH 06/14] MIPS: entry: Remove unneeded need_resched() loop

[Valentin Schneider]

Hi Paul,

On 14/03/2019 18:13, Paul Burton wrote:
[...]
> 
> It looks to me like commit a18815abcdfd ("Use preempt_schedule_irq.")
> forgot the branch to restore_all, so would have fallen through to
> ret_from_fork() & done weird things.
> 
> Adding the branch to restore_all as you're doing here would have been a
> better fix than commit cdaed73afb61 ("Fix preemption bug.").
> 

I didn't notice the missing branch to restore_all in that first commit -
that makes (more) sense now.

[...]
>> @@ -66,7 +65,7 @@ need_resched:
>>  	andi	t0, 1
>>  	beqz	t0, restore_all
>>  	jal	preempt_schedule_irq
>> -	b	need_resched
>> +	j	restore_all
> 
> One nit - why change from branch to jump? 

No actual reason there, I most likely deleted the branch, looked around,
saw the "j restore_all" in @resume_userspace and went for that (shoddy
I know...)

> It's not a big deal, but I'd
> prefer we stick with the branch ("b") instruction for a few reasons:
> 
> - restore_all is nearby so there's no issue with it being out of range
>   of a branch in any variation of the MIPS ISA.
> 
> - It's more consistent with the future of the MIPS architecture, eg.
>   nanoMIPS in which branch instructions all use PC-relative immediate
>   offsets & jump instructions are always of the "register" variety where
>   the destination is specified by a register rather than an immediate
>   encoded in the instruction (the assembler will fix it up & emit a
>   branch anyway, but I generally prefer to invoke less magic in these
>   areas...).
> 
> - A PC-relative branch won't generate an extra reloc in a relocatable
>   kernel, whereas a jump will.
> 

Makes total sense, thanks for the detailed reasoning!

> Even better would be if we take advantage of this being a tail call & do
> this:
> 
> 	PTR_LA	ra, restore_all
> 	j	preempt_schedule_irq
> 
> (Where I left the call to preempt_schedule_irq using a jump because it
> may be further away.)
> 

Right, that's even better, I'll send a v2 with that.

> Though I don't mind if you wanna just s/j/b/ & leave the tail call
> optimisation for someone else to do as a later change.
> 
> Thanks,
>     Paul
> 
>>  #endif
>>  
>>  FEXPORT(ret_from_kernel_thread)
>> -- 
>> 2.20.1
>>

[PATCH v2] MIPS: entry: Remove unneeded need_resched() loop

[Valentin Schneider]

Since the enabling and disabling of IRQs within preempt_schedule_irq()
is contained in a need_resched() loop, we don't need the outer arch
code loop.

Note that commit a18815abcdfd ("Use preempt_schedule_irq.") initially
removed the existing loop, but missed the final branch to restore_all.
Commit cdaed73afb61 ("Fix preemption bug.") missed that and reintroduced
the loop.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paul.burton@mips.com>
Cc: James Hogan <jhogan@kernel.org>
Cc: linux-mips@vger.kernel.org
---
 arch/mips/kernel/entry.S | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/mips/kernel/entry.S b/arch/mips/kernel/entry.S
index d7de8adcfcc8..5469d43b6966 100644
--- a/arch/mips/kernel/entry.S
+++ b/arch/mips/kernel/entry.S
@@ -58,15 +58,14 @@ resume_kernel:
 	local_irq_disable
 	lw	t0, TI_PRE_COUNT($28)
 	bnez	t0, restore_all
-need_resched:
 	LONG_L	t0, TI_FLAGS($28)
 	andi	t1, t0, _TIF_NEED_RESCHED
 	beqz	t1, restore_all
 	LONG_L	t0, PT_STATUS(sp)		# Interrupts off?
 	andi	t0, 1
 	beqz	t0, restore_all
-	jal	preempt_schedule_irq
-	b	need_resched
+	PTR_LA	ra, restore_all
+	j	preempt_schedule_irq
 #endif
 
 FEXPORT(ret_from_kernel_thread)
--
2.20.1

Re: [PATCH v2] MIPS: entry: Remove unneeded need_resched() loop

[Paul Burton]

Hello,

Valentin Schneider wrote:
> Since the enabling and disabling of IRQs within preempt_schedule_irq()
> is contained in a need_resched() loop, we don't need the outer arch
> code loop.
> 
> Note that commit a18815abcdfd ("Use preempt_schedule_irq.") initially
> removed the existing loop, but missed the final branch to restore_all.
> Commit cdaed73afb61 ("Fix preemption bug.") missed that and reintroduced
> the loop.
> 
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Paul Burton <paul.burton@mips.com>
> Cc: James Hogan <jhogan@kernel.org>
> Cc: linux-mips@vger.kernel.org

Applied to mips-next.

Thanks,
    Paul

[ This message was auto-generated; if you believe anything is incorrect
  then please email paul.burton@mips.com to report it. ]

[tip:x86/entry] x86/entry: Remove unneeded need_resched() loop

[tip-bot for Valentin Schneider]

Commit-ID:  b5b447b6b4e899e0978b95cb42d272978f8c7b4d
Gitweb:     https://git.kernel.org/tip/b5b447b6b4e899e0978b95cb42d272978f8c7b4d
Author:     Valentin Schneider <valentin.schneider@arm.com>
AuthorDate: Mon, 11 Mar 2019 22:47:51 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 5 Apr 2019 15:08:03 +0200

x86/entry: Remove unneeded need_resched() loop

Since the enabling and disabling of IRQs within preempt_schedule_irq() is
contained in a need_resched() loop, there is no need for the outer
architecture specific loop.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Link: https://lkml.kernel.org/r/20190311224752.8337-14-valentin.schneider@arm.com

---
 arch/x86/entry/entry_32.S | 3 +--
 arch/x86/entry/entry_64.S | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index d309f30cf7af..b1856fe9decf 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -766,13 +766,12 @@ END(ret_from_exception)
 #ifdef CONFIG_PREEMPT
 ENTRY(resume_kernel)
 	DISABLE_INTERRUPTS(CLBR_ANY)
-.Lneed_resched:
 	cmpl	$0, PER_CPU_VAR(__preempt_count)
 	jnz	restore_all_kernel
 	testl	$X86_EFLAGS_IF, PT_EFLAGS(%esp)	# interrupts off (exception path) ?
 	jz	restore_all_kernel
 	call	preempt_schedule_irq
-	jmp	.Lneed_resched
+	jmp	restore_all_kernel
 END(resume_kernel)
 #endif
 
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 1f0efdb7b629..e7e270603fe7 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -645,10 +645,9 @@ retint_kernel:
 	/* Check if we need preemption */
 	btl	$9, EFLAGS(%rsp)		/* were interrupts off? */
 	jnc	1f
-0:	cmpl	$0, PER_CPU_VAR(__preempt_count)
+	cmpl	$0, PER_CPU_VAR(__preempt_count)
 	jnz	1f
 	call	preempt_schedule_irq
-	jmp	0b
 1:
 #endif
 	/*

Re: [PATCH 07/14] nds32: ex-exit: Remove unneeded need_resched() loop

[Greentime Hu]

Hi Valentin,

Valentin Schneider <valentin.schneider@arm.com> 於 2019年3月12日 週二 上午6:48寫道：
>
> Since the enabling and disabling of IRQs within preempt_schedule_irq()
> is contained in a need_resched() loop, we don't need the outer arch
> code loop.
>
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> Cc: Greentime Hu <green.hu@gmail.com>
> Cc: Vincent Chen <deanbo422@gmail.com>
> ---
>  arch/nds32/kernel/ex-exit.S | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/nds32/kernel/ex-exit.S b/arch/nds32/kernel/ex-exit.S
> index 97ba15cd4180..1df02a793364 100644
> --- a/arch/nds32/kernel/ex-exit.S
> +++ b/arch/nds32/kernel/ex-exit.S
> @@ -163,7 +163,7 @@ resume_kernel:
>         gie_disable
>         lwi     $t0, [tsk+#TSK_TI_PREEMPT]
>         bnez    $t0, no_work_pending
> -need_resched:
> +
>         lwi     $t0, [tsk+#TSK_TI_FLAGS]
>         andi    $p1, $t0, #_TIF_NEED_RESCHED
>         beqz    $p1, no_work_pending
> @@ -173,7 +173,7 @@ need_resched:
>         beqz    $t0, no_work_pending
>
>         jal     preempt_schedule_irq
> -       b       need_resched
> +       b       no_work_pending
>  #endif
>
>  /*

Thank you. :)
Acked-by: Greentime Hu <greentime@andestech.com>

Re: [PATCH 08/14] powerpc: entry: Remove unneeded need_resched() loop

[Michael Ellerman]

On Mon, 2019-03-11 at 22:47:46 UTC, Valentin Schneider wrote:
> Since the enabling and disabling of IRQs within preempt_schedule_irq()
> is contained in a need_resched() loop, we don't need the outer arch
> code loop.
> 
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/90437bffa5f9b1440ba03e023f4875d1

cheers

Re: [PATCH 06/14] MIPS: entry: Remove unneeded need_resched() loop

[Maciej W. Rozycki]

On Thu, 14 Mar 2019, Paul Burton wrote:

> > @@ -66,7 +65,7 @@ need_resched:
> >  	andi	t0, 1
> >  	beqz	t0, restore_all
> >  	jal	preempt_schedule_irq
> > -	b	need_resched
> > +	j	restore_all
> 
> One nit - why change from branch to jump? It's not a big deal, but I'd
> prefer we stick with the branch ("b") instruction for a few reasons:
> 
> - restore_all is nearby so there's no issue with it being out of range
>   of a branch in any variation of the MIPS ISA.

 FYI, if it does go out of range for whatever reason, then for non-PIC 
code GAS will relax it to a jump anyway (with a relocation attached); for 
the regular MIPS ISA that is, where it has been doing that since forever 
(I meant to implement this for the microMIPS ISA too, but IIRC there was a 
complication there, probably coming from the existing more complex branch 
relaxation code and/or slightly different use of relocations, and then it 
fell through the cracks).

  Maciej