public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] x86 entry stuff, maybe for 4.4
@ 2015-11-11 23:14 Andy Lutomirski
  2015-11-11 23:14 ` [PATCH v2 1/4] context_tracking: Switch to new static_branch API Andy Lutomirski
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Andy Lutomirski @ 2015-11-11 23:14 UTC (permalink / raw)
  To: x86, linux-kernel
  Cc: Brian Gerst, Linus Torvalds, Borislav Petkov,
	Frédéric Weisbecker, Peter Zijlstra, Andy Lutomirski

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

-- 
2.5.0


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

* [PATCH v2 1/4] context_tracking: Switch to new static_branch API
  2015-11-11 23:14 [PATCH v2 0/4] x86 entry stuff, maybe for 4.4 Andy Lutomirski
@ 2015-11-11 23:14 ` Andy Lutomirski
  2015-11-11 23:14 ` [PATCH v2 2/4] x86/asm: Error out if asm/jump_label.h is included inappropriately Andy Lutomirski
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Andy Lutomirski @ 2015-11-11 23:14 UTC (permalink / raw)
  To: x86, linux-kernel
  Cc: Brian Gerst, Linus Torvalds, Borislav Petkov,
	Frédéric Weisbecker, Peter Zijlstra, Andy Lutomirski

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

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 include/linux/context_tracking_state.h | 4 ++--
 kernel/context_tracking.c              | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/context_tracking_state.h b/include/linux/context_tracking_state.h
index ee956c528fab..1d34fe68f48a 100644
--- a/include/linux/context_tracking_state.h
+++ b/include/linux/context_tracking_state.h
@@ -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)
diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
index 0a495ab35bc7..d0fb09e40784 100644
--- a/kernel/context_tracking.c
+++ b/kernel/context_tracking.c
@@ -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)
-- 
2.5.0


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

* [PATCH v2 2/4] x86/asm: Error out if asm/jump_label.h is included inappropriately
  2015-11-11 23:14 [PATCH v2 0/4] x86 entry stuff, maybe for 4.4 Andy Lutomirski
  2015-11-11 23:14 ` [PATCH v2 1/4] context_tracking: Switch to new static_branch API Andy Lutomirski
@ 2015-11-11 23:14 ` Andy Lutomirski
  2015-11-11 23:14 ` [PATCH v2 3/4] x86/asm: Add asm macros for static keys/jump labels Andy Lutomirski
  2015-11-11 23:14 ` [PATCH v2 4/4] x86/entry/64: Bypass enter_from_user_mode on non-context-tracking boots Andy Lutomirski
  3 siblings, 0 replies; 6+ messages in thread
From: Andy Lutomirski @ 2015-11-11 23:14 UTC (permalink / raw)
  To: x86, linux-kernel
  Cc: Brian Gerst, Linus Torvalds, Borislav Petkov,
	Frédéric Weisbecker, Peter Zijlstra, Andy Lutomirski

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

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/jump_label.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
index 5daeca3d0f9e..96872dc96597 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -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>
-- 
2.5.0


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

* [PATCH v2 3/4] x86/asm: Add asm macros for static keys/jump labels
  2015-11-11 23:14 [PATCH v2 0/4] x86 entry stuff, maybe for 4.4 Andy Lutomirski
  2015-11-11 23:14 ` [PATCH v2 1/4] context_tracking: Switch to new static_branch API Andy Lutomirski
  2015-11-11 23:14 ` [PATCH v2 2/4] x86/asm: Error out if asm/jump_label.h is included inappropriately Andy Lutomirski
@ 2015-11-11 23:14 ` Andy Lutomirski
  2015-11-11 23:14 ` [PATCH v2 4/4] x86/entry/64: Bypass enter_from_user_mode on non-context-tracking boots Andy Lutomirski
  3 siblings, 0 replies; 6+ messages in thread
From: Andy Lutomirski @ 2015-11-11 23:14 UTC (permalink / raw)
  To: x86, linux-kernel
  Cc: Brian Gerst, Linus Torvalds, Borislav Petkov,
	Frédéric Weisbecker, Peter Zijlstra, 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.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/jump_label.h | 52 +++++++++++++++++++++++++++++++++------
 1 file changed, 44 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
index 96872dc96597..adc54c12cbd1 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -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
-- 
2.5.0


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

* [PATCH v2 4/4] x86/entry/64: Bypass enter_from_user_mode on non-context-tracking boots
  2015-11-11 23:14 [PATCH v2 0/4] x86 entry stuff, maybe for 4.4 Andy Lutomirski
                   ` (2 preceding siblings ...)
  2015-11-11 23:14 ` [PATCH v2 3/4] x86/asm: Add asm macros for static keys/jump labels Andy Lutomirski
@ 2015-11-11 23:14 ` Andy Lutomirski
  2015-11-12  8:18   ` Ingo Molnar
  3 siblings, 1 reply; 6+ messages in thread
From: Andy Lutomirski @ 2015-11-11 23:14 UTC (permalink / raw)
  To: x86, linux-kernel
  Cc: Brian Gerst, Linus Torvalds, Borislav Petkov,
	Frédéric Weisbecker, Peter Zijlstra, 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.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/entry/calling.h  | 15 +++++++++++++++
 arch/x86/entry/entry_64.S |  8 ++------
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index 3c71dd947c7b..e32206e09868 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -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
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index a55697d19824..9d34d3cfceb6 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -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:
-- 
2.5.0


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

* Re: [PATCH v2 4/4] x86/entry/64: Bypass enter_from_user_mode on non-context-tracking boots
  2015-11-11 23:14 ` [PATCH v2 4/4] x86/entry/64: Bypass enter_from_user_mode on non-context-tracking boots Andy Lutomirski
@ 2015-11-12  8:18   ` Ingo Molnar
  0 siblings, 0 replies; 6+ messages in thread
From: Ingo Molnar @ 2015-11-12  8:18 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: x86, linux-kernel, Brian Gerst, Linus Torvalds, Borislav Petkov,
	Frédéric Weisbecker, Peter Zijlstra


* Andy Lutomirski <luto@kernel.org> wrote:

> 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.
> 
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/x86/entry/calling.h  | 15 +++++++++++++++
>  arch/x86/entry/entry_64.S |  8 ++------
>  2 files changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
> index 3c71dd947c7b..e32206e09868 100644
> --- a/arch/x86/entry/calling.h
> +++ b/arch/x86/entry/calling.h
> @@ -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
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index a55697d19824..9d34d3cfceb6 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -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:
> -- 
> 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

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

end of thread, other threads:[~2015-11-12  8:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-11 23:14 [PATCH v2 0/4] x86 entry stuff, maybe for 4.4 Andy Lutomirski
2015-11-11 23:14 ` [PATCH v2 1/4] context_tracking: Switch to new static_branch API Andy Lutomirski
2015-11-11 23:14 ` [PATCH v2 2/4] x86/asm: Error out if asm/jump_label.h is included inappropriately Andy Lutomirski
2015-11-11 23:14 ` [PATCH v2 3/4] x86/asm: Add asm macros for static keys/jump labels Andy Lutomirski
2015-11-11 23:14 ` [PATCH v2 4/4] x86/entry/64: Bypass enter_from_user_mode on non-context-tracking boots Andy Lutomirski
2015-11-12  8:18   ` Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox