linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] arm/arm64: entry: Remove need_resched() loop
@ 2019-01-31 18:23 Valentin Schneider
  2019-01-31 18:23 ` [RFC PATCH 1/3] arm64: entry: Remove unneeded " Valentin Schneider
  2019-01-31 18:23 ` [RFC PATCH 2/3] ARM: " Valentin Schneider
  0 siblings, 2 replies; 7+ messages in thread
From: Valentin Schneider @ 2019-01-31 18:23 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mark Rutland, Jonathan Corbet, linux-doc, Marc Zyngier,
	Catalin Marinas, Julien Thierry, Will Deacon, Russell King,
	Peter Zijlstra, Julien Grall, Ingo Molnar, Thomas Gleixner,
	linux-arm-kernel

A while back (before [1]), i386 had this in the tail of its irq
handling code:

  need_resched:
	  movl TI_flags(%ebp), %ecx       # need_resched set ?
	  testb $_TIF_NEED_RESCHED, %cl
	  jz restore_all
	  testl $IF_MASK,EFLAGS(%esp)     # interrupts off (exception path) ?
	  jz restore_all
	  sti
	  call preempt_schedule
	  cli
	  movl $0,TI_preempt_count(%ebp)
	  jmp need_resched

preempt_schedule() already had an inner need_resched() loop introduced by

  commit 4d0b85ea4610 ("[PATCH] kernel preemption bits (2/2)")

and the outer loop was needed since, as the above commit explains, it
was possible to return from preempt_schedule(), get an interrupt and
have need_resched() true.

This was eventually changed when preempt_schedule_irq() was introduced
by [1]:

  commit b268264c6299 ("[PATCH] sched: fix preemption race (Core/i386)")

which moved the enabling & disabling of interrupts inside the then
new preempt_schedule_irq(). From then on, the arch-code loop was no
longer necessary, providing preempt_schedule_irq() was used. This was
talked over on LKML in [2].

It's worth noting that it seems most archs calling
preempt_schedule_irq() have that outer loop, and for most of them it
really looks like they could get rid of it as well.

FWIW the suspects are

$ 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
  s390
  sh
  sparc
  x86
  xtensa


- Patches 1-2 remove the loop for arm & arm64
- Patch 3 adds a bit of documentation to point out the loop isn't needed
  to try and spread the word.

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

Valentin Schneider (3):
  arm64: entry: Remove unneeded need_resched() loop
  ARM: entry: Remove unneeded need_resched() loop
  sched/Documentation: Point out use of preempt_schedule_irq()

 Documentation/scheduler/sched-arch.txt | 10 ++++++++++
 arch/arm/kernel/entry-armv.S           | 12 +-----------
 arch/arm64/kernel/entry.S              | 11 +----------
 3 files changed, 12 insertions(+), 21 deletions(-)

--
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [RFC PATCH 1/3] arm64: entry: Remove unneeded need_resched() loop
  2019-01-31 18:23 [RFC PATCH 0/3] arm/arm64: entry: Remove need_resched() loop Valentin Schneider
@ 2019-01-31 18:23 ` Valentin Schneider
  2019-02-01  8:46   ` Julien Thierry
                     ` (2 more replies)
  2019-01-31 18:23 ` [RFC PATCH 2/3] ARM: " Valentin Schneider
  1 sibling, 3 replies; 7+ messages in thread
From: Valentin Schneider @ 2019-01-31 18:23 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mark Rutland, Julien Thierry, Marc Zyngier, Catalin Marinas,
	Will Deacon, Julien Grall, Thomas Gleixner, linux-arm-kernel

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.

Reported-by: Julien Thierry <julien.thierry@arm.com>
Reported-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Julien Thierry <julien.thierry@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
---
 arch/arm64/kernel/entry.S | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 0ec0c46b2c0c..4d0c81f29a60 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -611,7 +611,7 @@ el1_irq:
 #ifdef CONFIG_PREEMPT
 	ldr	x24, [tsk, #TSK_TI_PREEMPT]	// get preempt count
 	cbnz	x24, 1f				// preempt count != 0
-	bl	el1_preempt
+	bl	preempt_schedule_irq		// irq en/disable is done inside
 1:
 #endif
 #ifdef CONFIG_TRACE_IRQFLAGS
@@ -620,15 +620,6 @@ el1_irq:
 	kernel_exit 1
 ENDPROC(el1_irq)
 
-#ifdef CONFIG_PREEMPT
-el1_preempt:
-	mov	x24, lr
-1:	bl	preempt_schedule_irq		// irq en/disable is done inside
-	ldr	x0, [tsk, #TSK_TI_FLAGS]	// get new tasks TI_FLAGS
-	tbnz	x0, #TIF_NEED_RESCHED, 1b	// needs rescheduling?
-	ret	x24
-#endif
-
 /*
  * EL0 mode handlers.
  */
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RFC PATCH 2/3] ARM: entry: Remove unneeded need_resched() loop
  2019-01-31 18:23 [RFC PATCH 0/3] arm/arm64: entry: Remove need_resched() loop Valentin Schneider
  2019-01-31 18:23 ` [RFC PATCH 1/3] arm64: entry: Remove unneeded " Valentin Schneider
@ 2019-01-31 18:23 ` Valentin Schneider
  2019-02-01  8:49   ` Julien Thierry
  1 sibling, 1 reply; 7+ messages in thread
From: Valentin Schneider @ 2019-01-31 18:23 UTC (permalink / raw)
  To: linux-kernel
  Cc: Thomas Gleixner, Will Deacon, Russell King, linux-arm-kernel,
	Julien Thierry

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: Russell King <linux@armlinux.org.uk>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Julien Thierry <julien.thierry@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
---
 arch/arm/kernel/entry-armv.S | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index e85a3af9ddeb..a7ac22e09bb2 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -220,7 +220,7 @@ __irq_svc:
 	teq	r8, #0				@ if preempt count != 0
 	movne	r0, #0				@ force flags to 0
 	tst	r0, #_TIF_NEED_RESCHED
-	blne	svc_preempt
+	blne	preempt_schedule_irq            @ irq en/disable is done inside
 #endif
 
 	svc_exit r5, irq = 1			@ return from exception
@@ -229,16 +229,6 @@ ENDPROC(__irq_svc)
 
 	.ltorg
 
-#ifdef CONFIG_PREEMPT
-svc_preempt:
-	mov	r8, lr
-1:	bl	preempt_schedule_irq		@ irq en/disable is done inside
-	ldr	r0, [tsk, #TI_FLAGS]		@ get new tasks TI_FLAGS
-	tst	r0, #_TIF_NEED_RESCHED
-	reteq	r8				@ go again
-	b	1b
-#endif
-
 __und_fault:
 	@ Correct the PC such that it is pointing at the instruction
 	@ which caused the fault.  If the faulting instruction was ARM
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH 1/3] arm64: entry: Remove unneeded need_resched() loop
  2019-01-31 18:23 ` [RFC PATCH 1/3] arm64: entry: Remove unneeded " Valentin Schneider
@ 2019-02-01  8:46   ` Julien Thierry
  2019-02-01 10:21   ` Will Deacon
  2019-02-04 16:55   ` Catalin Marinas
  2 siblings, 0 replies; 7+ messages in thread
From: Julien Thierry @ 2019-02-01  8:46 UTC (permalink / raw)
  To: Valentin Schneider, linux-kernel
  Cc: Mark Rutland, Marc Zyngier, Catalin Marinas, Will Deacon,
	Julien Grall, Thomas Gleixner, linux-arm-kernel

Hi Valentin,

On 31/01/2019 18:23, 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.
> 
> Reported-by: Julien Thierry <julien.thierry@arm.com>
> Reported-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Julien Thierry <julien.thierry@arm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org

Reviewed-by: Julien Thierry <julien.thierry@arm.com>

> ---
>  arch/arm64/kernel/entry.S | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 0ec0c46b2c0c..4d0c81f29a60 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -611,7 +611,7 @@ el1_irq:
>  #ifdef CONFIG_PREEMPT
>  	ldr	x24, [tsk, #TSK_TI_PREEMPT]	// get preempt count
>  	cbnz	x24, 1f				// preempt count != 0
> -	bl	el1_preempt
> +	bl	preempt_schedule_irq		// irq en/disable is done inside
>  1:
>  #endif
>  #ifdef CONFIG_TRACE_IRQFLAGS
> @@ -620,15 +620,6 @@ el1_irq:
>  	kernel_exit 1
>  ENDPROC(el1_irq)
>  
> -#ifdef CONFIG_PREEMPT
> -el1_preempt:
> -	mov	x24, lr
> -1:	bl	preempt_schedule_irq		// irq en/disable is done inside
> -	ldr	x0, [tsk, #TSK_TI_FLAGS]	// get new tasks TI_FLAGS
> -	tbnz	x0, #TIF_NEED_RESCHED, 1b	// needs rescheduling?
> -	ret	x24
> -#endif
> -
>  /*
>   * EL0 mode handlers.
>   */
> 

-- 
Julien Thierry

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH 2/3] ARM: entry: Remove unneeded need_resched() loop
  2019-01-31 18:23 ` [RFC PATCH 2/3] ARM: " Valentin Schneider
@ 2019-02-01  8:49   ` Julien Thierry
  0 siblings, 0 replies; 7+ messages in thread
From: Julien Thierry @ 2019-02-01  8:49 UTC (permalink / raw)
  To: Valentin Schneider, linux-kernel
  Cc: Thomas Gleixner, Will Deacon, Russell King, linux-arm-kernel

Hi Valentin,

On 31/01/2019 18:23, 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: Russell King <linux@armlinux.org.uk>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Julien Thierry <julien.thierry@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org

Reviewed-by: Julien Thierry <julien.thierry@arm.com>

Cheers,

> ---
>  arch/arm/kernel/entry-armv.S | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
> index e85a3af9ddeb..a7ac22e09bb2 100644
> --- a/arch/arm/kernel/entry-armv.S
> +++ b/arch/arm/kernel/entry-armv.S
> @@ -220,7 +220,7 @@ __irq_svc:
>  	teq	r8, #0				@ if preempt count != 0
>  	movne	r0, #0				@ force flags to 0
>  	tst	r0, #_TIF_NEED_RESCHED
> -	blne	svc_preempt
> +	blne	preempt_schedule_irq            @ irq en/disable is done inside
>  #endif
>  
>  	svc_exit r5, irq = 1			@ return from exception
> @@ -229,16 +229,6 @@ ENDPROC(__irq_svc)
>  
>  	.ltorg
>  
> -#ifdef CONFIG_PREEMPT
> -svc_preempt:
> -	mov	r8, lr
> -1:	bl	preempt_schedule_irq		@ irq en/disable is done inside
> -	ldr	r0, [tsk, #TI_FLAGS]		@ get new tasks TI_FLAGS
> -	tst	r0, #_TIF_NEED_RESCHED
> -	reteq	r8				@ go again
> -	b	1b
> -#endif
> -
>  __und_fault:
>  	@ Correct the PC such that it is pointing at the instruction
>  	@ which caused the fault.  If the faulting instruction was ARM
> 

-- 
Julien Thierry

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH 1/3] arm64: entry: Remove unneeded need_resched() loop
  2019-01-31 18:23 ` [RFC PATCH 1/3] arm64: entry: Remove unneeded " Valentin Schneider
  2019-02-01  8:46   ` Julien Thierry
@ 2019-02-01 10:21   ` Will Deacon
  2019-02-04 16:55   ` Catalin Marinas
  2 siblings, 0 replies; 7+ messages in thread
From: Will Deacon @ 2019-02-01 10:21 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: Mark Rutland, Julien Thierry, Marc Zyngier, Catalin Marinas,
	linux-kernel, Julien Grall, Thomas Gleixner, linux-arm-kernel

On Thu, Jan 31, 2019 at 06:23:37PM +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.
> 
> Reported-by: Julien Thierry <julien.thierry@arm.com>
> Reported-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Julien Thierry <julien.thierry@arm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  arch/arm64/kernel/entry.S | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 0ec0c46b2c0c..4d0c81f29a60 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -611,7 +611,7 @@ el1_irq:
>  #ifdef CONFIG_PREEMPT
>  	ldr	x24, [tsk, #TSK_TI_PREEMPT]	// get preempt count
>  	cbnz	x24, 1f				// preempt count != 0
> -	bl	el1_preempt
> +	bl	preempt_schedule_irq		// irq en/disable is done inside
>  1:
>  #endif
>  #ifdef CONFIG_TRACE_IRQFLAGS
> @@ -620,15 +620,6 @@ el1_irq:
>  	kernel_exit 1
>  ENDPROC(el1_irq)
>  
> -#ifdef CONFIG_PREEMPT
> -el1_preempt:
> -	mov	x24, lr
> -1:	bl	preempt_schedule_irq		// irq en/disable is done inside
> -	ldr	x0, [tsk, #TSK_TI_FLAGS]	// get new tasks TI_FLAGS
> -	tbnz	x0, #TIF_NEED_RESCHED, 1b	// needs rescheduling?
> -	ret	x24
> -#endif
> -

Acked-by: Will Deacon <will.deacon@arm.com>

Catalin can pick this up for 5.1.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH 1/3] arm64: entry: Remove unneeded need_resched() loop
  2019-01-31 18:23 ` [RFC PATCH 1/3] arm64: entry: Remove unneeded " Valentin Schneider
  2019-02-01  8:46   ` Julien Thierry
  2019-02-01 10:21   ` Will Deacon
@ 2019-02-04 16:55   ` Catalin Marinas
  2 siblings, 0 replies; 7+ messages in thread
From: Catalin Marinas @ 2019-02-04 16:55 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: Mark Rutland, Julien Thierry, Marc Zyngier, Will Deacon,
	linux-kernel, Julien Grall, Thomas Gleixner, linux-arm-kernel

On Thu, Jan 31, 2019 at 06:23:37PM +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.
> 
> Reported-by: Julien Thierry <julien.thierry@arm.com>
> Reported-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Julien Thierry <julien.thierry@arm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org

Queued for 5.1. Thanks

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-02-04 16:55 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-31 18:23 [RFC PATCH 0/3] arm/arm64: entry: Remove need_resched() loop Valentin Schneider
2019-01-31 18:23 ` [RFC PATCH 1/3] arm64: entry: Remove unneeded " Valentin Schneider
2019-02-01  8:46   ` Julien Thierry
2019-02-01 10:21   ` Will Deacon
2019-02-04 16:55   ` Catalin Marinas
2019-01-31 18:23 ` [RFC PATCH 2/3] ARM: " Valentin Schneider
2019-02-01  8:49   ` Julien Thierry

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).