Skip to contentSkip to conversation

x86 entry stuff, maybe for 4.4

PATCH Superseded Andy Lutomirski 2 participants 6 messages Nov 11, 2015 – Nov 12, 2015

Subsystems (4)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Andy Lutomirski

author maintainer
#0 lore raw

v2 cover x86 entry stuff, maybe for 4.4

The first patch is a bit ugly, but it fixes a bug that could affect lockdep. That bug is very minor and may not be observable at all, but I don't really want to bet on it.

The other three are intended to fix a performance regression in the entry rework that Frédéric objected to. They're much later than I'd like to have sent them for 4.4, but they're kind-of sort-of regression fixes, so maybe they're still okay. They would certainly need careful review, though.

I don't have a great benchmark for them. The biggest impact is likely to be to user page fault latency on CONFIG_CONTEXT_TRACKING=y kernels (i.e. distro kernels) that don't use context tracking (i.e. most users).

Changes from v1:
- CALL_ENTER_FROM_USER_MODE is now CALL_enter_from_user_mode (Ingo)
- STATIC_JUMP_IF_{TRUE,FALSE} now cannot be (mis-)used on non-jump-label kernels (Thomas)
- Comments are better (Borislav)

This doesn't really address Thomas' objections to the HAVE_JUMP_LABEL stuff, but it's more robust now, and maybe that's good enough.

Andy Lutomirski (4):
context_tracking: Switch to new static_branch API x86/asm: Error out if asm/jump_label.h is included inappropriately x86/asm: Add asm macros for static keys/jump labels x86/entry/64: Bypass enter_from_user_mode on non-context-tracking
boots

 arch/x86/entry/calling.h               | 15 ++++++++
 arch/x86/entry/entry_64.S              |  8 ++---
 arch/x86/include/asm/jump_label.h      | 63 ++++++++++++++++++++++++++++++----
 include/linux/context_tracking_state.h |  4 +--
 kernel/context_tracking.c              |  4 +--
 5 files changed, 77 insertions(+), 17 deletions(-)
Signature · 1 line
2.5.0
author maintainer
#1 lore raw

PATCH v2 1/4 context_tracking: Switch to new static_branch API

This is much less error-prone than the old code.

---
 include/linux/context_tracking_state.h | 4 ++--
 kernel/context_tracking.c              | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
include/linux/context_tracking_state.h +2 -2
index ee956c528fab..1d34fe68f48a 100644
@@ -22,12 +22,12 @@ struct context_tracking {
 };
 
 #ifdef CONFIG_CONTEXT_TRACKING
-extern struct static_key context_tracking_enabled;
+extern struct static_key_false context_tracking_enabled;
 DECLARE_PER_CPU(struct context_tracking, context_tracking);
 
 static inline bool context_tracking_is_enabled(void)
 {
-	return static_key_false(&context_tracking_enabled);
+	return static_branch_unlikely(&context_tracking_enabled);
 }
 
 static inline bool context_tracking_cpu_is_enabled(void)
kernel/context_tracking.c +2 -2
index 0a495ab35bc7..d0fb09e40784 100644
@@ -24,7 +24,7 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/context_tracking.h>
 
-struct static_key context_tracking_enabled = STATIC_KEY_INIT_FALSE;
+DEFINE_STATIC_KEY_FALSE(context_tracking_enabled);
 EXPORT_SYMBOL_GPL(context_tracking_enabled);
 
 DEFINE_PER_CPU(struct context_tracking, context_tracking);
@@ -191,7 +191,7 @@ void __init context_tracking_cpu_set(int cpu)
 
 	if (!per_cpu(context_tracking.active, cpu)) {
 		per_cpu(context_tracking.active, cpu) = true;
-		static_key_slow_inc(&context_tracking_enabled);
+		static_branch_inc(&context_tracking_enabled);
 	}
 
 	if (initialized)
Signature · 1 line
2.5.0
author maintainer
#2 lore raw

PATCH v2 2/4 x86/asm: Error out if asm/jump_label.h is included inappropriately

In reply to Andy Lutomirski

Rather than potentially generating incorrect code on a non-HAVE_JUMP_LABEL kernel if someone includes asm/jump_label.h, error out.

---
 arch/x86/include/asm/jump_label.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)
arch/x86/include/asm/jump_label.h +13 -0
index 5daeca3d0f9e..96872dc96597 100644
@@ -1,6 +1,19 @@
 #ifndef _ASM_X86_JUMP_LABEL_H
 #define _ASM_X86_JUMP_LABEL_H
 
+#ifndef HAVE_JUMP_LABEL
+/*
+ * For better or for worse, if jump labels (the gcc extension) are missing,
+ * then the entire static branch patching infrastructure is compiled out.
+ * If that happens, the code in here will malfunction.  Raise a compiler
+ * error instead.
+ *
+ * In theory, jump labels and the static branch patching infrastructure
+ * could be decoupled to fix this.
+ */
+#error asm/jump_label.h included on a non-jump-label kernel
+#endif
+
 #ifndef __ASSEMBLY__
 
 #include <linux/stringify.h>
Signature · 1 line
2.5.0
author maintainer
#3 lore raw

PATCH v2 3/4 x86/asm: Add asm macros for static keys/jump labels

In reply to Andy Lutomirski

Unfortunately, we can only do this if HAVE_JUMP_LABEL. In principle, we could do some serious surgery on the core jump label infrastructure to keep the patch infrastructure available on x86 on all builds, but that's probably not worth it.

Implementing the macros using a conditional branch as a fallback seems like a bad idea: we'd have to clobber flags.

This limitation can't cause silent failures -- trying to include asm/jump_label.h at all on a non-HAVE_JUMP_LABEL kernel will error out. The macro's users are responsible for handling this issue themselves.

---
 arch/x86/include/asm/jump_label.h | 52 +++++++++++++++++++++++++++++++++------
 1 file changed, 44 insertions(+), 8 deletions(-)
arch/x86/include/asm/jump_label.h +44 -8
index 96872dc96597..adc54c12cbd1 100644
@@ -14,13 +14,6 @@
 #error asm/jump_label.h included on a non-jump-label kernel
 #endif
 
-#ifndef __ASSEMBLY__
-
-#include <linux/stringify.h>
-#include <linux/types.h>
-#include <asm/nops.h>
-#include <asm/asm.h>
-
 #define JUMP_LABEL_NOP_SIZE 5
 
 #ifdef CONFIG_X86_64
@@ -29,6 +22,14 @@
 # define STATIC_KEY_INIT_NOP GENERIC_NOP5_ATOMIC
 #endif
 
+#include <asm/asm.h>
+#include <asm/nops.h>
+
+#ifndef __ASSEMBLY__
+
+#include <linux/stringify.h>
+#include <linux/types.h>
+
 static __always_inline bool arch_static_branch(struct static_key *key, bool branch)
 {
 	asm_volatile_goto("1:"
@@ -72,5 +73,40 @@ struct jump_entry {
 	jump_label_t key;
 };
 
-#endif  /* __ASSEMBLY__ */
+#else	/* __ASSEMBLY__ */
+
+.macro STATIC_JUMP_IF_TRUE target, key, def
+.Lstatic_jump_\@:
+	.if \def
+	/* Equivalent to "jmp.d32 \target" */
+	.byte		0xe9
+	.long		\target - .Lstatic_jump_after_\@
+.Lstatic_jump_after_\@:
+	.else
+	.byte		STATIC_KEY_INIT_NOP
+	.endif
+	.pushsection __jump_table, "aw"
+	_ASM_ALIGN
+	_ASM_PTR	.Lstatic_jump_\@, \target, \key
+	.popsection
+.endm
+
+.macro STATIC_JUMP_IF_FALSE target, key, def
+.Lstatic_jump_\@:
+	.if \def
+	.byte		STATIC_KEY_INIT_NOP
+	.else
+	/* Equivalent to "jmp.d32 \target" */
+	.byte		0xe9
+	.long		\target - .Lstatic_jump_after_\@
+.Lstatic_jump_after_\@:
+	.endif
+	.pushsection __jump_table, "aw"
+	_ASM_ALIGN
+	_ASM_PTR	.Lstatic_jump_\@, \target, \key + 1
+	.popsection
+.endm
+
+#endif	/* __ASSEMBLY__ */
+
 #endif
Signature · 1 line
2.5.0
author maintainer
#4 lore raw

PATCH v2 4/4 x86/entry/64: Bypass enter_from_user_mode on non-context-tracking boots

In reply to Andy Lutomirski

On CONFIG_CONTEXT_TRACKING kernels that have context tracking disabled at runtime (which includes most distro kernels), we still have the overhead of a call to enter_from_user_mode in interrupt and exception entries.

If jump labels are available, this uses the jump label infrastructure to skip the call.

---
 arch/x86/entry/calling.h  | 15 +++++++++++++++
 arch/x86/entry/entry_64.S |  8 ++------
 2 files changed, 17 insertions(+), 6 deletions(-)
arch/x86/entry/calling.h +15 -0
index 3c71dd947c7b..e32206e09868 100644
@@ -1,3 +1,5 @@
+#include <linux/jump_label.h>
+
 /*
 
  x86 function call convention, 64-bit:
@@ -232,3 +234,16 @@ For 32-bit we have the following conventions - kernel is built with
 
 #endif /* CONFIG_X86_64 */
 
+/*
+ * This does 'call enter_from_user_mode' unless we can avoid it based on
+ * kernel config or using the static jump infrastructure.
+ */
+.macro CALL_enter_from_user_mode
+#ifdef CONFIG_CONTEXT_TRACKING
+#ifdef HAVE_JUMP_LABEL
+	STATIC_JUMP_IF_FALSE .Lafter_call_\@, context_tracking_enabled, def=0
+#endif
+	call enter_from_user_mode
+.Lafter_call_\@:
+#endif
+.endm
arch/x86/entry/entry_64.S +2 -6
index a55697d19824..9d34d3cfceb6 100644
@@ -520,9 +520,7 @@ END(irq_entries_start)
 	 */
 	TRACE_IRQS_OFF
 
-#ifdef CONFIG_CONTEXT_TRACKING
-	call enter_from_user_mode
-#endif
+	CALL_enter_from_user_mode
 
 1:
 	/*
@@ -1066,9 +1064,7 @@ ENTRY(error_entry)
 	 * (which can take locks).
 	 */
 	TRACE_IRQS_OFF
-#ifdef CONFIG_CONTEXT_TRACKING
-	call enter_from_user_mode
-#endif
+	CALL_enter_from_user_mode
 	ret
 
 .Lerror_entry_done:
Signature · 1 line
2.5.0
#5 lore raw

Re: [PATCH v2 4/4] x86/entry/64: Bypass enter_from_user_mode on non-context-tracking boots

Quoted 70 lines · Andy Lutomirski

On CONFIG_CONTEXT_TRACKING kernels that have context tracking disabled at runtime (which includes most distro kernels), we still have the overhead of a call to enter_from_user_mode in interrupt and exception entries.

If jump labels are available, this uses the jump label infrastructure to skip the call.

---
 arch/x86/entry/calling.h  | 15 +++++++++++++++
 arch/x86/entry/entry_64.S |  8 ++------
 2 files changed, 17 insertions(+), 6 deletions(-)
arch/x86/entry/calling.h +15 -0
index 3c71dd947c7b..e32206e09868 100644
@@ -1,3 +1,5 @@
+#include <linux/jump_label.h>
+
 /*
 
  x86 function call convention, 64-bit:
@@ -232,3 +234,16 @@ For 32-bit we have the following conventions - kernel is built with
 
 #endif /* CONFIG_X86_64 */
 
+/*
+ * This does 'call enter_from_user_mode' unless we can avoid it based on
+ * kernel config or using the static jump infrastructure.
+ */
+.macro CALL_enter_from_user_mode
+#ifdef CONFIG_CONTEXT_TRACKING
+#ifdef HAVE_JUMP_LABEL
+	STATIC_JUMP_IF_FALSE .Lafter_call_\@, context_tracking_enabled, def=0
+#endif
+	call enter_from_user_mode
+.Lafter_call_\@:
+#endif
+.endm
arch/x86/entry/entry_64.S +2 -6
index a55697d19824..9d34d3cfceb6 100644
@@ -520,9 +520,7 @@ END(irq_entries_start)
 	 */
 	TRACE_IRQS_OFF
 
-#ifdef CONFIG_CONTEXT_TRACKING
-	call enter_from_user_mode
-#endif
+	CALL_enter_from_user_mode
 
 1:
 	/*
@@ -1066,9 +1064,7 @@ ENTRY(error_entry)
 	 * (which can take locks).
 	 */
 	TRACE_IRQS_OFF
-#ifdef CONFIG_CONTEXT_TRACKING
-	call enter_from_user_mode
-#endif
+	CALL_enter_from_user_mode
 	ret
 
 .Lerror_entry_done:
Signature · 1 line
2.5.0

hm, this patch does not apply to latest -tip:

patching file arch/x86/entry/entry_64.S
Hunk #1 FAILED at 520.
Hunk #2 FAILED at 1066.
2 out of 2 hunks FAILED -- rejects in file arch/x86/entry/entry_64.S

am I missing some dependency, or are they against an older tree?

Thanks,

Ingo