linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] arm64: Only call into preempt_schedule() if need_resched()
@ 2018-11-27 19:45 Will Deacon
  2018-11-27 19:45 ` [PATCH 1/2] preempt: Move PREEMPT_NEED_RESCHED definition into arch code Will Deacon
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Will Deacon @ 2018-11-27 19:45 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, ard.biesheuvel, catalin.marinas, rml, tglx, peterz,
	schwidefsky, Will Deacon

Hi all,

This pair of patches improves our preempt_enable() implementation slightly
on arm64 by making the resulting call to preempt_schedule() conditional
on need_resched(), which is tracked in bit 32 of the preempt count. The
logic is inverted so that we can detect the preempt count going to zero
and need_resched being set with a single CBZ instruction.

Consequently, our preempt_enable() code (including prologue/epilogue)
goes from:

  40:   a9bf7bfd        stp     x29, x30, [sp, #-16]!
  44:   910003fd        mov     x29, sp
  48:   d5384101        mrs     x1, sp_el0
  4c:   b9401020        ldr     w0, [x1, #16]
  50:   51000400        sub     w0, w0, #0x1
  54:   b9001020        str     w0, [x1, #16]
  58:   350000a0        cbnz    w0, 6c
  5c:   f9400020        ldr     x0, [x1]
  60:   721f001f        tst     w0, #0x2
  64:   54000040        b.eq    6c // b.none
  68:   94000000        bl      0 <preempt_schedule>
  6c:   a8c17bfd        ldp     x29, x30, [sp], #16
  70:   d65f03c0        ret

to:

  40:   a9bf7bfd        stp     x29, x30, [sp, #-16]!
  44:   910003fd        mov     x29, sp
  48:   d5384101        mrs     x1, sp_el0
  4c:   f9400820        ldr     x0, [x1, #16]
  50:   d1000400        sub     x0, x0, #0x1
  54:   b9001020        str     w0, [x1, #16]
  58:   b4000060        cbz     x0, 64 <will+0x24>
  5c:   a8c17bfd        ldp     x29, x30, [sp], #16
  60:   d65f03c0        ret
  64:   94000000        bl      0 <preempt_schedule>
  68:   a8c17bfd        ldp     x29, x30, [sp], #16
  6c:   d65f03c0        ret

Will

--->8

Will Deacon (2):
  preempt: Move PREEMPT_NEED_RESCHED definition into arch code
  arm64: preempt: Provide our own implementation of asm/preempt.h

 arch/arm64/include/asm/Kbuild        |  1 -
 arch/arm64/include/asm/preempt.h     | 78 ++++++++++++++++++++++++++++++++++++
 arch/arm64/include/asm/thread_info.h | 13 +++++-
 arch/s390/include/asm/preempt.h      |  2 +
 arch/x86/include/asm/preempt.h       |  3 ++
 include/linux/preempt.h              |  3 --
 6 files changed, 95 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm64/include/asm/preempt.h

-- 
2.1.4


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

* [PATCH 1/2] preempt: Move PREEMPT_NEED_RESCHED definition into arch code
  2018-11-27 19:45 [PATCH 0/2] arm64: Only call into preempt_schedule() if need_resched() Will Deacon
@ 2018-11-27 19:45 ` Will Deacon
  2018-11-27 19:45 ` [PATCH 2/2] arm64: preempt: Provide our own implementation of asm/preempt.h Will Deacon
  2018-11-28  8:56 ` [PATCH 0/2] arm64: Only call into preempt_schedule() if need_resched() Peter Zijlstra
  2 siblings, 0 replies; 9+ messages in thread
From: Will Deacon @ 2018-11-27 19:45 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, ard.biesheuvel, catalin.marinas, rml, tglx, peterz,
	schwidefsky, Will Deacon

PREEMPT_NEED_RESCHED is never used directly, so move it into the arch
code where it can potentially be implemented using either a different
bit in the preempt count or as an entirely separate entity.

Cc: Robert Love <rml@tech9.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/s390/include/asm/preempt.h | 2 ++
 arch/x86/include/asm/preempt.h  | 3 +++
 include/linux/preempt.h         | 3 ---
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/s390/include/asm/preempt.h b/arch/s390/include/asm/preempt.h
index 23a14d187fb1..b5ea9e14c017 100644
--- a/arch/s390/include/asm/preempt.h
+++ b/arch/s390/include/asm/preempt.h
@@ -8,6 +8,8 @@
 
 #ifdef CONFIG_HAVE_MARCH_Z196_FEATURES
 
+/* We use the MSB mostly because its available */
+#define PREEMPT_NEED_RESCHED	0x80000000
 #define PREEMPT_ENABLED	(0 + PREEMPT_NEED_RESCHED)
 
 static inline int preempt_count(void)
diff --git a/arch/x86/include/asm/preempt.h b/arch/x86/include/asm/preempt.h
index 90cb2f36c042..99a7fa9ab0a3 100644
--- a/arch/x86/include/asm/preempt.h
+++ b/arch/x86/include/asm/preempt.h
@@ -8,6 +8,9 @@
 
 DECLARE_PER_CPU(int, __preempt_count);
 
+/* We use the MSB mostly because its available */
+#define PREEMPT_NEED_RESCHED	0x80000000
+
 /*
  * We use the PREEMPT_NEED_RESCHED bit as an inverted NEED_RESCHED such
  * that a decrement hitting 0 means we can and should reschedule.
diff --git a/include/linux/preempt.h b/include/linux/preempt.h
index c01813c3fbe9..dd92b1a93919 100644
--- a/include/linux/preempt.h
+++ b/include/linux/preempt.h
@@ -53,9 +53,6 @@
 
 #define SOFTIRQ_DISABLE_OFFSET	(2 * SOFTIRQ_OFFSET)
 
-/* We use the MSB mostly because its available */
-#define PREEMPT_NEED_RESCHED	0x80000000
-
 #define PREEMPT_DISABLED	(PREEMPT_DISABLE_OFFSET + PREEMPT_ENABLED)
 
 /*
-- 
2.1.4


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

* [PATCH 2/2] arm64: preempt: Provide our own implementation of asm/preempt.h
  2018-11-27 19:45 [PATCH 0/2] arm64: Only call into preempt_schedule() if need_resched() Will Deacon
  2018-11-27 19:45 ` [PATCH 1/2] preempt: Move PREEMPT_NEED_RESCHED definition into arch code Will Deacon
@ 2018-11-27 19:45 ` Will Deacon
  2018-11-28 15:35   ` Ard Biesheuvel
  2018-11-28  8:56 ` [PATCH 0/2] arm64: Only call into preempt_schedule() if need_resched() Peter Zijlstra
  2 siblings, 1 reply; 9+ messages in thread
From: Will Deacon @ 2018-11-27 19:45 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, ard.biesheuvel, catalin.marinas, rml, tglx, peterz,
	schwidefsky, Will Deacon

The asm-generic/preempt.h implementation doesn't make use of the
PREEMPT_NEED_RESCHED flag, since this can interact badly with load/store
architectures which rely on the preempt_count word being unchanged across
an interrupt.

However, since we're a 64-bit architecture and the preempt count is
only 32 bits wide, we can simply pack it next to the resched flag and
load the whole thing in one go, so that a dec-and-test operation doesn't
need to load twice.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/Kbuild        |  1 -
 arch/arm64/include/asm/preempt.h     | 78 ++++++++++++++++++++++++++++++++++++
 arch/arm64/include/asm/thread_info.h | 13 +++++-
 3 files changed, 90 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm64/include/asm/preempt.h

diff --git a/arch/arm64/include/asm/Kbuild b/arch/arm64/include/asm/Kbuild
index 6cd5d77b6b44..33498f900390 100644
--- a/arch/arm64/include/asm/Kbuild
+++ b/arch/arm64/include/asm/Kbuild
@@ -14,7 +14,6 @@ generic-y += local64.h
 generic-y += mcs_spinlock.h
 generic-y += mm-arch-hooks.h
 generic-y += msi.h
-generic-y += preempt.h
 generic-y += qrwlock.h
 generic-y += qspinlock.h
 generic-y += rwsem.h
diff --git a/arch/arm64/include/asm/preempt.h b/arch/arm64/include/asm/preempt.h
new file mode 100644
index 000000000000..832227d5ebc0
--- /dev/null
+++ b/arch/arm64/include/asm/preempt.h
@@ -0,0 +1,78 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_PREEMPT_H
+#define __ASM_PREEMPT_H
+
+#include <linux/thread_info.h>
+
+#define PREEMPT_NEED_RESCHED	BIT(32)
+#define PREEMPT_ENABLED	(PREEMPT_NEED_RESCHED)
+
+static inline int preempt_count(void)
+{
+	return READ_ONCE(current_thread_info()->preempt.count);
+}
+
+static inline void preempt_count_set(u64 pc)
+{
+	/* Preserve existing value of PREEMPT_NEED_RESCHED */
+	WRITE_ONCE(current_thread_info()->preempt.count, pc);
+}
+
+#define init_task_preempt_count(p) do { \
+	task_thread_info(p)->preempt_count = FORK_PREEMPT_COUNT; \
+} while (0)
+
+#define init_idle_preempt_count(p, cpu) do { \
+	task_thread_info(p)->preempt_count = PREEMPT_ENABLED; \
+} while (0)
+
+static inline void set_preempt_need_resched(void)
+{
+	current_thread_info()->preempt.need_resched = 0;
+}
+
+static inline void clear_preempt_need_resched(void)
+{
+	current_thread_info()->preempt.need_resched = 1;
+}
+
+static inline bool test_preempt_need_resched(void)
+{
+	return !current_thread_info()->preempt.need_resched;
+}
+
+static inline void __preempt_count_add(int val)
+{
+	u32 pc = READ_ONCE(current_thread_info()->preempt.count);
+	pc += val;
+	WRITE_ONCE(current_thread_info()->preempt.count, pc);
+}
+
+static inline void __preempt_count_sub(int val)
+{
+	u32 pc = READ_ONCE(current_thread_info()->preempt.count);
+	pc -= val;
+	WRITE_ONCE(current_thread_info()->preempt.count, pc);
+}
+
+static inline bool __preempt_count_dec_and_test(void)
+{
+	u64 pc = READ_ONCE(current_thread_info()->preempt_count);
+	WRITE_ONCE(current_thread_info()->preempt.count, --pc);
+	return !pc;
+}
+
+static inline bool should_resched(int preempt_offset)
+{
+	u64 pc = READ_ONCE(current_thread_info()->preempt_count);
+	return pc == preempt_offset;
+}
+
+#ifdef CONFIG_PREEMPT
+void preempt_schedule(void);
+#define __preempt_schedule() preempt_schedule()
+void preempt_schedule_notrace(void);
+#define __preempt_schedule_notrace() preempt_schedule_notrace()
+#endif /* CONFIG_PREEMPT */
+
+#endif /* __ASM_PREEMPT_H */
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index cb2c10a8f0a8..bbca68b54732 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -42,7 +42,18 @@ struct thread_info {
 #ifdef CONFIG_ARM64_SW_TTBR0_PAN
 	u64			ttbr0;		/* saved TTBR0_EL1 */
 #endif
-	int			preempt_count;	/* 0 => preemptable, <0 => bug */
+	union {
+		u64		preempt_count;	/* 0 => preemptible, <0 => bug */
+		struct {
+#ifdef CONFIG_CPU_BIG_ENDIAN
+			u32	need_resched;
+			u32	count;
+#else
+			u32	count;
+			u32	need_resched;
+#endif
+		} preempt;
+	};
 };
 
 #define thread_saved_pc(tsk)	\
-- 
2.1.4


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

* Re: [PATCH 0/2] arm64: Only call into preempt_schedule() if need_resched()
  2018-11-27 19:45 [PATCH 0/2] arm64: Only call into preempt_schedule() if need_resched() Will Deacon
  2018-11-27 19:45 ` [PATCH 1/2] preempt: Move PREEMPT_NEED_RESCHED definition into arch code Will Deacon
  2018-11-27 19:45 ` [PATCH 2/2] arm64: preempt: Provide our own implementation of asm/preempt.h Will Deacon
@ 2018-11-28  8:56 ` Peter Zijlstra
  2018-11-28  9:01   ` Peter Zijlstra
  2 siblings, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2018-11-28  8:56 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-arm-kernel, linux-kernel, ard.biesheuvel, catalin.marinas,
	rml, tglx, schwidefsky

On Tue, Nov 27, 2018 at 07:45:00PM +0000, Will Deacon wrote:
> Hi all,
> 
> This pair of patches improves our preempt_enable() implementation slightly
> on arm64 by making the resulting call to preempt_schedule() conditional
> on need_resched(), which is tracked in bit 32 of the preempt count. The
> logic is inverted so that we can detect the preempt count going to zero
> and need_resched being set with a single CBZ instruction.

>   40:   a9bf7bfd        stp     x29, x30, [sp, #-16]!
>   44:   910003fd        mov     x29, sp
>   48:   d5384101        mrs     x1, sp_el0
>   4c:   f9400820        ldr     x0, [x1, #16]

We load x0 which is a u64, right?

>   50:   d1000400        sub     x0, x0, #0x1
>   54:   b9001020        str     w0, [x1, #16]

But we store w0, which is the low u32, such as to not touch the high
word which contains the preempt bit.

>   58:   b4000060        cbz     x0, 64 <will+0x24>
>   5c:   a8c17bfd        ldp     x29, x30, [sp], #16
>   60:   d65f03c0        ret
>   64:   94000000        bl      0 <preempt_schedule>
>   68:   a8c17bfd        ldp     x29, x30, [sp], #16
>   6c:   d65f03c0        ret

Why not?

   58:   b4000060        cbnz    x0, 60 <will+0x24>
   5c:   94000000        bl      0 <preempt_schedule>
   60:   a8c17bfd        ldp     x29, x30, [sp], #16
   64:   d65f03c0        ret

which seems shorter.


So it's still early, and I haven't finished (or really even started) my
pot 'o tea, but what about:


	ldr x0, [x1, #16]	// seees the high bit set -- no preempt needed
	sub x0, x0, #1

	<interrupt>
	  ...
	  resched_curr()
	    set_tsk_need_resched();
	    set_preempt_need_resched();
	</interrupt> // sees preempt_count != 0, does not preempt

	str w0, [x1, #16] // stores preempt_count == 0
	cbnz x0, 1f // taken, we still observe the high word from before

1:	ret


Which then ends with preempt_count==0, need_resched==0 and no actual
preemption afaict.

Can you use mis-matched ll x0 / sc w0 to do this same thing and detector
the intermediate write on the high word?

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

* Re: [PATCH 0/2] arm64: Only call into preempt_schedule() if need_resched()
  2018-11-28  8:56 ` [PATCH 0/2] arm64: Only call into preempt_schedule() if need_resched() Peter Zijlstra
@ 2018-11-28  9:01   ` Peter Zijlstra
  2018-11-28 12:04     ` Will Deacon
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2018-11-28  9:01 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-arm-kernel, linux-kernel, ard.biesheuvel, catalin.marinas,
	rml, tglx, schwidefsky

On Wed, Nov 28, 2018 at 09:56:40AM +0100, Peter Zijlstra wrote:
> On Tue, Nov 27, 2018 at 07:45:00PM +0000, Will Deacon wrote:
> > Hi all,
> > 
> > This pair of patches improves our preempt_enable() implementation slightly
> > on arm64 by making the resulting call to preempt_schedule() conditional
> > on need_resched(), which is tracked in bit 32 of the preempt count. The
> > logic is inverted so that we can detect the preempt count going to zero
> > and need_resched being set with a single CBZ instruction.
> 
> >   40:   a9bf7bfd        stp     x29, x30, [sp, #-16]!
> >   44:   910003fd        mov     x29, sp
> >   48:   d5384101        mrs     x1, sp_el0
> >   4c:   f9400820        ldr     x0, [x1, #16]
> 
> We load x0 which is a u64, right?
> 
> >   50:   d1000400        sub     x0, x0, #0x1
> >   54:   b9001020        str     w0, [x1, #16]
> 
> But we store w0, which is the low u32, such as to not touch the high
> word which contains the preempt bit.
> 
> >   58:   b4000060        cbz     x0, 64 <will+0x24>
> >   5c:   a8c17bfd        ldp     x29, x30, [sp], #16
> >   60:   d65f03c0        ret
> >   64:   94000000        bl      0 <preempt_schedule>
> >   68:   a8c17bfd        ldp     x29, x30, [sp], #16
> >   6c:   d65f03c0        ret
> 
> Why not?
> 
>    58:   b4000060        cbnz    x0, 60 <will+0x24>
>    5c:   94000000        bl      0 <preempt_schedule>
>    60:   a8c17bfd        ldp     x29, x30, [sp], #16
>    64:   d65f03c0        ret
> 
> which seems shorter.
> 
> 
> So it's still early, and I haven't finished (or really even started) my
> pot 'o tea, but what about:
> 
> 
> 	ldr x0, [x1, #16]	// seees the high bit set -- no preempt needed
> 	sub x0, x0, #1
> 
> 	<interrupt>
> 	  ...
> 	  resched_curr()
> 	    set_tsk_need_resched();
> 	    set_preempt_need_resched();
> 	</interrupt> // sees preempt_count != 0, does not preempt
> 
> 	str w0, [x1, #16] // stores preempt_count == 0
> 	cbnz x0, 1f // taken, we still observe the high word from before
> 
> 1:	ret
> 
> 
> Which then ends with preempt_count==0, need_resched==0 and no actual
> preemption afaict.
> 
> Can you use mis-matched ll x0 / sc w0 to do this same thing and detector
> the intermediate write on the high word?

That is, something along these here lines:

1:	ldxr x0, [x1, #16]
	sub  x0, x0, #1
	stxr w1, w0, [x1, #16]
	cbnz w1, 1b

	cbnz x0, 2f
	bl   preempt_schedule
2:	ret

can that work?

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

* Re: [PATCH 0/2] arm64: Only call into preempt_schedule() if need_resched()
  2018-11-28  9:01   ` Peter Zijlstra
@ 2018-11-28 12:04     ` Will Deacon
  0 siblings, 0 replies; 9+ messages in thread
From: Will Deacon @ 2018-11-28 12:04 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-arm-kernel, linux-kernel, ard.biesheuvel, catalin.marinas,
	rml, tglx, schwidefsky

On Wed, Nov 28, 2018 at 10:01:46AM +0100, Peter Zijlstra wrote:
> On Wed, Nov 28, 2018 at 09:56:40AM +0100, Peter Zijlstra wrote:
> > On Tue, Nov 27, 2018 at 07:45:00PM +0000, Will Deacon wrote:
> > > This pair of patches improves our preempt_enable() implementation slightly
> > > on arm64 by making the resulting call to preempt_schedule() conditional
> > > on need_resched(), which is tracked in bit 32 of the preempt count. The
> > > logic is inverted so that we can detect the preempt count going to zero
> > > and need_resched being set with a single CBZ instruction.
> > 
> > >   40:   a9bf7bfd        stp     x29, x30, [sp, #-16]!
> > >   44:   910003fd        mov     x29, sp
> > >   48:   d5384101        mrs     x1, sp_el0
> > >   4c:   f9400820        ldr     x0, [x1, #16]
> > 
> > We load x0 which is a u64, right?
> > 
> > >   50:   d1000400        sub     x0, x0, #0x1
> > >   54:   b9001020        str     w0, [x1, #16]
> > 
> > But we store w0, which is the low u32, such as to not touch the high
> > word which contains the preempt bit.
> > 
> > >   58:   b4000060        cbz     x0, 64 <will+0x24>
> > >   5c:   a8c17bfd        ldp     x29, x30, [sp], #16
> > >   60:   d65f03c0        ret
> > >   64:   94000000        bl      0 <preempt_schedule>
> > >   68:   a8c17bfd        ldp     x29, x30, [sp], #16
> > >   6c:   d65f03c0        ret
> > 
> > Why not?
> > 
> >    58:   b4000060        cbnz    x0, 60 <will+0x24>
> >    5c:   94000000        bl      0 <preempt_schedule>
> >    60:   a8c17bfd        ldp     x29, x30, [sp], #16
> >    64:   d65f03c0        ret
> > 
> > which seems shorter.
> > 
> > 
> > So it's still early, and I haven't finished (or really even started) my
> > pot 'o tea, but what about:
> > 
> > 
> > 	ldr x0, [x1, #16]	// seees the high bit set -- no preempt needed
> > 	sub x0, x0, #1
> > 
> > 	<interrupt>
> > 	  ...
> > 	  resched_curr()
> > 	    set_tsk_need_resched();
> > 	    set_preempt_need_resched();
> > 	</interrupt> // sees preempt_count != 0, does not preempt
> > 
> > 	str w0, [x1, #16] // stores preempt_count == 0
> > 	cbnz x0, 1f // taken, we still observe the high word from before
> > 
> > 1:	ret
> > 
> > 
> > Which then ends with preempt_count==0, need_resched==0 and no actual
> > preemption afaict.
> > 
> > Can you use mis-matched ll x0 / sc w0 to do this same thing and detector
> > the intermediate write on the high word?
> 
> That is, something along these here lines:
> 
> 1:	ldxr x0, [x1, #16]
> 	sub  x0, x0, #1
> 	stxr w1, w0, [x1, #16]

^^ This guy needs a different encoding but, to be honest, I reckon we're
better off just reloading the need_resched flag in the case where the count
has hit zero. I'll have a play. The assembly I posted is all generated by
GCC, so I can't comment on why it didn't chose your shorter sequence :)

Will

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

* Re: [PATCH 2/2] arm64: preempt: Provide our own implementation of asm/preempt.h
  2018-11-27 19:45 ` [PATCH 2/2] arm64: preempt: Provide our own implementation of asm/preempt.h Will Deacon
@ 2018-11-28 15:35   ` Ard Biesheuvel
  2018-11-28 16:40     ` Peter Zijlstra
  0 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2018-11-28 15:35 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-arm-kernel, Linux Kernel Mailing List, Catalin Marinas,
	rml, Thomas Gleixner, Peter Zijlstra, Martin Schwidefsky

On Tue, 27 Nov 2018 at 20:44, Will Deacon <will.deacon@arm.com> wrote:
>
> The asm-generic/preempt.h implementation doesn't make use of the
> PREEMPT_NEED_RESCHED flag, since this can interact badly with load/store
> architectures which rely on the preempt_count word being unchanged across
> an interrupt.
>
> However, since we're a 64-bit architecture and the preempt count is
> only 32 bits wide, we can simply pack it next to the resched flag and
> load the whole thing in one go, so that a dec-and-test operation doesn't
> need to load twice.
>

Since the actual preempt count is a lot narrower than 32 bits, x86
just uses bit 31.

So what is the reason for using two different words?


> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/include/asm/Kbuild        |  1 -
>  arch/arm64/include/asm/preempt.h     | 78 ++++++++++++++++++++++++++++++++++++
>  arch/arm64/include/asm/thread_info.h | 13 +++++-
>  3 files changed, 90 insertions(+), 2 deletions(-)
>  create mode 100644 arch/arm64/include/asm/preempt.h
>
> diff --git a/arch/arm64/include/asm/Kbuild b/arch/arm64/include/asm/Kbuild
> index 6cd5d77b6b44..33498f900390 100644
> --- a/arch/arm64/include/asm/Kbuild
> +++ b/arch/arm64/include/asm/Kbuild
> @@ -14,7 +14,6 @@ generic-y += local64.h
>  generic-y += mcs_spinlock.h
>  generic-y += mm-arch-hooks.h
>  generic-y += msi.h
> -generic-y += preempt.h
>  generic-y += qrwlock.h
>  generic-y += qspinlock.h
>  generic-y += rwsem.h
> diff --git a/arch/arm64/include/asm/preempt.h b/arch/arm64/include/asm/preempt.h
> new file mode 100644
> index 000000000000..832227d5ebc0
> --- /dev/null
> +++ b/arch/arm64/include/asm/preempt.h
> @@ -0,0 +1,78 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_PREEMPT_H
> +#define __ASM_PREEMPT_H
> +
> +#include <linux/thread_info.h>
> +
> +#define PREEMPT_NEED_RESCHED   BIT(32)
> +#define PREEMPT_ENABLED        (PREEMPT_NEED_RESCHED)
> +
> +static inline int preempt_count(void)
> +{
> +       return READ_ONCE(current_thread_info()->preempt.count);
> +}
> +
> +static inline void preempt_count_set(u64 pc)
> +{
> +       /* Preserve existing value of PREEMPT_NEED_RESCHED */
> +       WRITE_ONCE(current_thread_info()->preempt.count, pc);
> +}
> +
> +#define init_task_preempt_count(p) do { \
> +       task_thread_info(p)->preempt_count = FORK_PREEMPT_COUNT; \
> +} while (0)
> +
> +#define init_idle_preempt_count(p, cpu) do { \
> +       task_thread_info(p)->preempt_count = PREEMPT_ENABLED; \
> +} while (0)
> +
> +static inline void set_preempt_need_resched(void)
> +{
> +       current_thread_info()->preempt.need_resched = 0;
> +}
> +
> +static inline void clear_preempt_need_resched(void)
> +{
> +       current_thread_info()->preempt.need_resched = 1;
> +}
> +
> +static inline bool test_preempt_need_resched(void)
> +{
> +       return !current_thread_info()->preempt.need_resched;
> +}
> +
> +static inline void __preempt_count_add(int val)
> +{
> +       u32 pc = READ_ONCE(current_thread_info()->preempt.count);
> +       pc += val;
> +       WRITE_ONCE(current_thread_info()->preempt.count, pc);
> +}
> +
> +static inline void __preempt_count_sub(int val)
> +{
> +       u32 pc = READ_ONCE(current_thread_info()->preempt.count);
> +       pc -= val;
> +       WRITE_ONCE(current_thread_info()->preempt.count, pc);
> +}
> +
> +static inline bool __preempt_count_dec_and_test(void)
> +{
> +       u64 pc = READ_ONCE(current_thread_info()->preempt_count);
> +       WRITE_ONCE(current_thread_info()->preempt.count, --pc);
> +       return !pc;
> +}
> +
> +static inline bool should_resched(int preempt_offset)
> +{
> +       u64 pc = READ_ONCE(current_thread_info()->preempt_count);
> +       return pc == preempt_offset;
> +}
> +
> +#ifdef CONFIG_PREEMPT
> +void preempt_schedule(void);
> +#define __preempt_schedule() preempt_schedule()
> +void preempt_schedule_notrace(void);
> +#define __preempt_schedule_notrace() preempt_schedule_notrace()
> +#endif /* CONFIG_PREEMPT */
> +
> +#endif /* __ASM_PREEMPT_H */
> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index cb2c10a8f0a8..bbca68b54732 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -42,7 +42,18 @@ struct thread_info {
>  #ifdef CONFIG_ARM64_SW_TTBR0_PAN
>         u64                     ttbr0;          /* saved TTBR0_EL1 */
>  #endif
> -       int                     preempt_count;  /* 0 => preemptable, <0 => bug */
> +       union {
> +               u64             preempt_count;  /* 0 => preemptible, <0 => bug */
> +               struct {
> +#ifdef CONFIG_CPU_BIG_ENDIAN
> +                       u32     need_resched;
> +                       u32     count;
> +#else
> +                       u32     count;
> +                       u32     need_resched;
> +#endif
> +               } preempt;
> +       };
>  };
>
>  #define thread_saved_pc(tsk)   \
> --
> 2.1.4
>

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

* Re: [PATCH 2/2] arm64: preempt: Provide our own implementation of asm/preempt.h
  2018-11-28 15:35   ` Ard Biesheuvel
@ 2018-11-28 16:40     ` Peter Zijlstra
  2018-11-28 16:45       ` Ard Biesheuvel
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2018-11-28 16:40 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Will Deacon, linux-arm-kernel, Linux Kernel Mailing List,
	Catalin Marinas, rml, Thomas Gleixner, Martin Schwidefsky

On Wed, Nov 28, 2018 at 04:35:42PM +0100, Ard Biesheuvel wrote:
> On Tue, 27 Nov 2018 at 20:44, Will Deacon <will.deacon@arm.com> wrote:
> >
> > The asm-generic/preempt.h implementation doesn't make use of the
> > PREEMPT_NEED_RESCHED flag, since this can interact badly with load/store
> > architectures which rely on the preempt_count word being unchanged across
> > an interrupt.
> >
> > However, since we're a 64-bit architecture and the preempt count is
> > only 32 bits wide, we can simply pack it next to the resched flag and
> > load the whole thing in one go, so that a dec-and-test operation doesn't
> > need to load twice.
> >
> 
> Since the actual preempt count is a lot narrower than 32 bits, x86
> just uses bit 31.
> 
> So what is the reason for using two different words?

See commit:

  ba1f14fbe709 ("sched: Remove PREEMPT_NEED_RESCHED from generic code")

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

* Re: [PATCH 2/2] arm64: preempt: Provide our own implementation of asm/preempt.h
  2018-11-28 16:40     ` Peter Zijlstra
@ 2018-11-28 16:45       ` Ard Biesheuvel
  0 siblings, 0 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2018-11-28 16:45 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Will Deacon, linux-arm-kernel, Linux Kernel Mailing List,
	Catalin Marinas, rml, Thomas Gleixner, Martin Schwidefsky

On Wed, 28 Nov 2018 at 17:41, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Nov 28, 2018 at 04:35:42PM +0100, Ard Biesheuvel wrote:
> > On Tue, 27 Nov 2018 at 20:44, Will Deacon <will.deacon@arm.com> wrote:
> > >
> > > The asm-generic/preempt.h implementation doesn't make use of the
> > > PREEMPT_NEED_RESCHED flag, since this can interact badly with load/store
> > > architectures which rely on the preempt_count word being unchanged across
> > > an interrupt.
> > >
> > > However, since we're a 64-bit architecture and the preempt count is
> > > only 32 bits wide, we can simply pack it next to the resched flag and
> > > load the whole thing in one go, so that a dec-and-test operation doesn't
> > > need to load twice.
> > >
> >
> > Since the actual preempt count is a lot narrower than 32 bits, x86
> > just uses bit 31.
> >
> > So what is the reason for using two different words?
>
> See commit:
>
>   ba1f14fbe709 ("sched: Remove PREEMPT_NEED_RESCHED from generic code")

Yup, that clears it up - thanks.

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

end of thread, other threads:[~2018-11-28 16:45 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-27 19:45 [PATCH 0/2] arm64: Only call into preempt_schedule() if need_resched() Will Deacon
2018-11-27 19:45 ` [PATCH 1/2] preempt: Move PREEMPT_NEED_RESCHED definition into arch code Will Deacon
2018-11-27 19:45 ` [PATCH 2/2] arm64: preempt: Provide our own implementation of asm/preempt.h Will Deacon
2018-11-28 15:35   ` Ard Biesheuvel
2018-11-28 16:40     ` Peter Zijlstra
2018-11-28 16:45       ` Ard Biesheuvel
2018-11-28  8:56 ` [PATCH 0/2] arm64: Only call into preempt_schedule() if need_resched() Peter Zijlstra
2018-11-28  9:01   ` Peter Zijlstra
2018-11-28 12:04     ` Will Deacon

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).