Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1264644 > unrolled thread

[PATCH 0/4] x86 entry stuff, maybe for 4.4

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4] x86 entry stuff, maybe for 4.4 Andy Lutomirski <luto@kernel.org> - 2015-11-07 00:20 +0100
    [PATCH 4/4] x86/entry/64: Bypass enter_from_user_mode on non-context-tracking boots Andy Lutomirski <luto@kernel.org> - 2015-11-07 00:20 +0100
      Re: [PATCH 4/4] x86/entry/64: Bypass enter_from_user_mode on  non-context-tracking boots Ingo Molnar <mingo@kernel.org> - 2015-11-09 10:00 +0100
    [PATCH 3/4] x86/asm: Add asm macros for static keys/jump labels Andy Lutomirski <luto@kernel.org> - 2015-11-07 00:20 +0100
      Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump  labels Thomas Gleixner <tglx@linutronix.de> - 2015-11-07 12:30 +0100
        Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump  labels Thomas Gleixner <tglx@linutronix.de> - 2015-11-07 18:00 +0100
          Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump  labels Thomas Gleixner <tglx@linutronix.de> - 2015-11-07 18:10 +0100
            Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump labels Andy Lutomirski <luto@amacapital.net> - 2015-11-07 19:20 +0100
              Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump labels Peter Zijlstra <peterz@infradead.org> - 2015-11-09 10:50 +0100
            Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump labels Andy Lutomirski <luto@amacapital.net> - 2015-11-08 17:20 +0100
          Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump labels Andy Lutomirski <luto@amacapital.net> - 2015-11-07 18:10 +0100
        Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump labels Andy Lutomirski <luto@amacapital.net> - 2015-11-07 18:00 +0100

#1264644 — [PATCH 0/4] 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).

Andy Lutomirski (4):
  x86/entry/64: Fix irqflag tracing wrt context tracking
  context_tracking: Switch to new static_branch API
  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              | 19 ++++++++-----
 arch/x86/include/asm/jump_label.h      | 52 ++++++++++++++++++++++++++++------
 include/linux/context_tracking_state.h |  4 +--
 kernel/context_tracking.c              |  4 +--
 5 files changed, 75 insertions(+), 19 deletions(-)

-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1264646 — [PATCH 4/4] x86/entry/64: Bypass enter_from_user_mode on non-context-tracking boots

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..271e30c585bc 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 f585df24ab3d..9b49b56efa29 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -517,9 +517,7 @@ END(irq_entries_start)
 	 */
 	TRACE_IRQS_OFF
 
-#ifdef CONFIG_CONTEXT_TRACKING
-	call enter_from_user_mode
-#endif
+	CALL_ENTER_FROM_USER_MODE
 
 1:
 	/*
@@ -1058,9 +1056,7 @@ ENTRY(error_entry)
 
 .Lerror_entry_from_usermode_after_swapgs:
 	TRACE_IRQS_OFF
-#ifdef CONFIG_CONTEXT_TRACKING
-	call enter_from_user_mode
-#endif
+	CALL_ENTER_FROM_USER_MODE
 	ret
 
 .Lerror_entry_done:
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

* 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..271e30c585bc 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 f585df24ab3d..9b49b56efa29 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -517,9 +517,7 @@ END(irq_entries_start)
>  	 */
>  	TRACE_IRQS_OFF
>  
> -#ifdef CONFIG_CONTEXT_TRACKING
> -	call enter_from_user_mode
> -#endif
> +	CALL_ENTER_FROM_USER_MODE
>  
>  1:
>  	/*
> @@ -1058,9 +1056,7 @@ ENTRY(error_entry)
>  
>  .Lerror_entry_from_usermode_after_swapgs:
>  	TRACE_IRQS_OFF
> -#ifdef CONFIG_CONTEXT_TRACKING
> -	call enter_from_user_mode
> -#endif
> +	CALL_ENTER_FROM_USER_MODE
>  	ret

Yeah, so I only have a really minor syntactic nit abou tthis: it's OK to 
capitalize things when doing macros, but here I don't think capitalizing the 
called function name is good - I think the following would be more obvious:

	CALL_enter_from_user_mode

this makes it really, really obvious (to me!) that this macro is a shortcut for:

	call enter_from_user_mode

with some glue around it, and I could grep for 'enter_from_user_mode' immediately 
- while grepping for ENTER_FROM_USER_MODE would miss the called function.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1264648 — [PATCH 3/4] x86/asm: Add asm macros for static keys/jump labels

Unfortunately, these only work 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.

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 5daeca3d0f9e..da275c1825f5 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -1,13 +1,6 @@
 #ifndef _ASM_X86_JUMP_LABEL_H
 #define _ASM_X86_JUMP_LABEL_H
 
-#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
@@ -16,6 +9,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:"
@@ -59,5 +60,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.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1264812 — Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump labels

On Fri, 6 Nov 2015, Andy Lutomirski wrote:

> Unfortunately, these only work 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.

No, but we can be smarter about it. There is nothing which that asm
entry code needs from linux/jump_label.h and asm/jump_label.h execpt
STATIC_KEY_INIT_NOP. Something like the patch below should do the
trick.

Thanks,

	tglx

Index: tip/arch/x86/entry/calling.h
===================================================================
--- tip.orig/arch/x86/entry/calling.h
+++ tip/arch/x86/entry/calling.h
@@ -232,3 +232,22 @@ For 32-bit we have the following convent
 
 #endif /* CONFIG_X86_64 */
 
+/* ASM jump label support */
+#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL)
+#include <asm/jump_label.h>
+
+	.macro STATIC_CALL_IF_ENABLED fun, key
+	1:
+	jmp.d32		2f
+	call		fun
+	.pushsection __jump_table, "aw"
+	_ASM_ALIGN
+	_ASM_PTR	1b, 2f, \key
+	.popsection
+	2:
+	.endm
+#else
+	.macro STATIC_CALL_IF_ENABLED fun, key
+	call		fun
+	.endm
+#endif
Index: tip/arch/x86/entry/entry_64.S
===================================================================
--- tip.orig/arch/x86/entry/entry_64.S
+++ tip/arch/x86/entry/entry_64.S
@@ -510,7 +510,7 @@ END(irq_entries_start)
 	 */
 	SWAPGS
 #ifdef CONFIG_CONTEXT_TRACKING
-	call enter_from_user_mode
+	STATIC_CALL_IF_ENABLED enter_from_user_mode, context_tracking_enabled
 #endif
 
 1:
@@ -1050,7 +1050,7 @@ ENTRY(error_entry)
 
 .Lerror_entry_from_usermode_after_swapgs:
 #ifdef CONFIG_CONTEXT_TRACKING
-	call enter_from_user_mode
+	STATIC_CALL_IF_ENABLED enter_from_user_mode, context_tracking_enabled
 #endif
 
 .Lerror_entry_done:
Index: tip/arch/x86/include/asm/jump_label.h
===================================================================
--- tip.orig/arch/x86/include/asm/jump_label.h
+++ tip/arch/x86/include/asm/jump_label.h
@@ -1,10 +1,6 @@
 #ifndef _ASM_X86_JUMP_LABEL_H
 #define _ASM_X86_JUMP_LABEL_H
 
-#ifndef __ASSEMBLY__
-
-#include <linux/stringify.h>
-#include <linux/types.h>
 #include <asm/nops.h>
 #include <asm/asm.h>
 
@@ -16,6 +12,11 @@
 # define STATIC_KEY_INIT_NOP GENERIC_NOP5_ATOMIC
 #endif
 
+#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:"


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1264884 — Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump labels

On Sat, 7 Nov 2015, Andy Lutomirski wrote:
> On Nov 7, 2015 3:21 AM, "Thomas Gleixner" <tglx@linutronix.de> wrote:
> > +/* ASM jump label support */
> > +#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL)
> > +#include <asm/jump_label.h>
> > +
> > +       .macro STATIC_CALL_IF_ENABLED fun, key
> 
> I think we still need the "def" parameter.  But maybe the "static
> call" is a good idea.  (By giving 'def' a default value or omitting it
> entirely, it's too easy not to think about it, and that's a huge
> historical source of breakage and it's a good part of the reason for
> the new improved C API.  At least these days I *think* we'll diagnose
> the problem on bootup instead of waiting for random breakage later.)

Ok. That's easy to add :)
 
> > +       1:
> > +       jmp.d32         2f
> 
> Old binutils will choke on this.  But maybe no one has jump labels and
> old binutils.

Good question.
 
> > +       call            fun
> > +       .pushsection __jump_table, "aw"
> > +       _ASM_ALIGN
> > +       _ASM_PTR        1b, 2f, \key
> > +       .popsection
> > +       2:
> > +       .endm
> > +#else
> > +       .macro STATIC_CALL_IF_ENABLED fun, key
> > +       call            fun
> > +       .endm
> 
> This could result in errors down the road, since this macro is really
> "static call if enabled or if the kernel is built without jump
> labels".  enter_from_user_mode is special because it's already a no-op
> if context tracking is off, but I'm not sure that this oddity should
> leak into more generic code.

Hmm. Maybe we need a better name for this macro.
 
> > +#endif
> > Index: tip/arch/x86/entry/entry_64.S
> > ===================================================================
> > --- tip.orig/arch/x86/entry/entry_64.S
> > +++ tip/arch/x86/entry/entry_64.S
> > @@ -510,7 +510,7 @@ END(irq_entries_start)
> >          */
> >         SWAPGS
> >  #ifdef CONFIG_CONTEXT_TRACKING
> > -       call enter_from_user_mode
> > +       STATIC_CALL_IF_ENABLED enter_from_user_mode, context_tracking_enabled
> >  #endif
> 
> This doesn't get rid of the ifdeffery, which I thought was a nice
> feature of my patch.

Right. I was just too lazy to add another macro :)

My main point was to avoid the extra stuff for !HAVE_JUMP_LABEL and
hide this in a header file.

Thanks,

	tglx


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1264888 — Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump labels

On Sat, 7 Nov 2015, Andy Lutomirski wrote:
> On Sat, Nov 7, 2015 at 8:58 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> True.  But I hid it in a header file, too, but it was just a different
> header file -- I had it all hidden away as CALL_ENTER_FROM_USER_MODE.

It probably does not really make a difference :) 

I just got triggered by you saying:

> Unfortunately, these only work if HAVE_JUMP_LABEL ....

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1264905 — Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump labels

On Sat, Nov 7, 2015 at 9:08 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Sat, 7 Nov 2015, Andy Lutomirski wrote:
>> On Sat, Nov 7, 2015 at 8:58 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> True.  But I hid it in a header file, too, but it was just a different
>> header file -- I had it all hidden away as CALL_ENTER_FROM_USER_MODE.
>
> It probably does not really make a difference :)
>
> I just got triggered by you saying:
>
>> Unfortunately, these only work if HAVE_JUMP_LABEL ....

Yeah, I don't really like that either.  I think that the real fix
would be to compile in the runtime jump label bits unconditionally.
Admittedly it would bloat the kernel image a little bit for
!CONFIG_JUMP_LABEL or if build with an older gcc, and that's even less
appropriate for 4.4 at this point.  Peter?

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1265491 — Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump labels

On Sat, Nov 07, 2015 at 10:16:37AM -0800, Andy Lutomirski wrote:

> >> Unfortunately, these only work if HAVE_JUMP_LABEL ....
> 
> Yeah, I don't really like that either.  I think that the real fix
> would be to compile in the runtime jump label bits unconditionally.
> Admittedly it would bloat the kernel image a little bit for
> !CONFIG_JUMP_LABEL or if build with an older gcc, and that's even less
> appropriate for 4.4 at this point.  Peter?

That is a _lot_ of code for just one user. I'll bet the tiny people
won't be happy about that.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1265125 — Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump labels

On Sat, Nov 7, 2015 at 9:08 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Sat, 7 Nov 2015, Andy Lutomirski wrote:
>> On Sat, Nov 7, 2015 at 8:58 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> True.  But I hid it in a header file, too, but it was just a different
>> header file -- I had it all hidden away as CALL_ENTER_FROM_USER_MODE.
>
> It probably does not really make a difference :)
>
> I just got triggered by you saying:
>
>> Unfortunately, these only work if HAVE_JUMP_LABEL ....
>

Looking again, my patch is crappy is one obvious respect: on
non-HAVE_JUMP_LABEL kernels, I'm still generating a macro that will
compile but do the wrong thing.  I'll add better comments and the
correct ifdef in v2.  That will make it much harder to screw up
without causing a compiler error.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1264889 — Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump labels

On Sat, Nov 7, 2015 at 8:58 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Sat, 7 Nov 2015, Andy Lutomirski wrote:
>> On Nov 7, 2015 3:21 AM, "Thomas Gleixner" <tglx@linutronix.de> wrote:
>> > +/* ASM jump label support */
>> > +#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL)
>> > +#include <asm/jump_label.h>
>> > +
>> > +       .macro STATIC_CALL_IF_ENABLED fun, key
>>
>> I think we still need the "def" parameter.  But maybe the "static
>> call" is a good idea.  (By giving 'def' a default value or omitting it
>> entirely, it's too easy not to think about it, and that's a huge
>> historical source of breakage and it's a good part of the reason for
>> the new improved C API.  At least these days I *think* we'll diagnose
>> the problem on bootup instead of waiting for random breakage later.)
>
> Ok. That's easy to add :)
>
>> > +       1:
>> > +       jmp.d32         2f
>>
>> Old binutils will choke on this.  But maybe no one has jump labels and
>> old binutils.
>
> Good question.
>
>> > +       call            fun
>> > +       .pushsection __jump_table, "aw"
>> > +       _ASM_ALIGN
>> > +       _ASM_PTR        1b, 2f, \key
>> > +       .popsection
>> > +       2:
>> > +       .endm
>> > +#else
>> > +       .macro STATIC_CALL_IF_ENABLED fun, key
>> > +       call            fun
>> > +       .endm
>>
>> This could result in errors down the road, since this macro is really
>> "static call if enabled or if the kernel is built without jump
>> labels".  enter_from_user_mode is special because it's already a no-op
>> if context tracking is off, but I'm not sure that this oddity should
>> leak into more generic code.
>
> Hmm. Maybe we need a better name for this macro.
>
>> > +#endif
>> > Index: tip/arch/x86/entry/entry_64.S
>> > ===================================================================
>> > --- tip.orig/arch/x86/entry/entry_64.S
>> > +++ tip/arch/x86/entry/entry_64.S
>> > @@ -510,7 +510,7 @@ END(irq_entries_start)
>> >          */
>> >         SWAPGS
>> >  #ifdef CONFIG_CONTEXT_TRACKING
>> > -       call enter_from_user_mode
>> > +       STATIC_CALL_IF_ENABLED enter_from_user_mode, context_tracking_enabled
>> >  #endif
>>
>> This doesn't get rid of the ifdeffery, which I thought was a nice
>> feature of my patch.
>
> Right. I was just too lazy to add another macro :)
>
> My main point was to avoid the extra stuff for !HAVE_JUMP_LABEL and
> hide this in a header file.
>

True.  But I hid it in a header file, too, but it was just a different
header file -- I had it all hidden away as CALL_ENTER_FROM_USER_MODE.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1264885 — Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump labels

On Nov 7, 2015 3:21 AM, "Thomas Gleixner" <tglx@linutronix.de> wrote:
>
> On Fri, 6 Nov 2015, Andy Lutomirski wrote:
>
> > Unfortunately, these only work 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.
>
> No, but we can be smarter about it. There is nothing which that asm
> entry code needs from linux/jump_label.h and asm/jump_label.h execpt
> STATIC_KEY_INIT_NOP. Something like the patch below should do the
> trick.

I like some bits of this.  See below.

>
> Thanks,
>
>         tglx
>
> Index: tip/arch/x86/entry/calling.h
> ===================================================================
> --- tip.orig/arch/x86/entry/calling.h
> +++ tip/arch/x86/entry/calling.h
> @@ -232,3 +232,22 @@ For 32-bit we have the following convent
>
>  #endif /* CONFIG_X86_64 */
>
> +/* ASM jump label support */
> +#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL)
> +#include <asm/jump_label.h>
> +
> +       .macro STATIC_CALL_IF_ENABLED fun, key

I think we still need the "def" parameter.  But maybe the "static
call" is a good idea.  (By giving 'def' a default value or omitting it
entirely, it's too easy not to think about it, and that's a huge
historical source of breakage and it's a good part of the reason for
the new improved C API.  At least these days I *think* we'll diagnose
the problem on bootup instead of waiting for random breakage later.)

> +       1:
> +       jmp.d32         2f

Old binutils will choke on this.  But maybe no one has jump labels and
old binutils.

> +       call            fun
> +       .pushsection __jump_table, "aw"
> +       _ASM_ALIGN
> +       _ASM_PTR        1b, 2f, \key
> +       .popsection
> +       2:
> +       .endm
> +#else
> +       .macro STATIC_CALL_IF_ENABLED fun, key
> +       call            fun
> +       .endm

This could result in errors down the road, since this macro is really
"static call if enabled or if the kernel is built without jump
labels".  enter_from_user_mode is special because it's already a no-op
if context tracking is off, but I'm not sure that this oddity should
leak into more generic code.

> +#endif
> Index: tip/arch/x86/entry/entry_64.S
> ===================================================================
> --- tip.orig/arch/x86/entry/entry_64.S
> +++ tip/arch/x86/entry/entry_64.S
> @@ -510,7 +510,7 @@ END(irq_entries_start)
>          */
>         SWAPGS
>  #ifdef CONFIG_CONTEXT_TRACKING
> -       call enter_from_user_mode
> +       STATIC_CALL_IF_ENABLED enter_from_user_mode, context_tracking_enabled
>  #endif

This doesn't get rid of the ifdeffery, which I thought was a nice
feature of my patch.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web