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

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

[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 v3: Actually send entire series (patch 1 was missing)

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 (5):
  x86/entry/64: Fix irqflag tracing wrt context tracking
  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              | 27 +++++++++++----
 arch/x86/include/asm/jump_label.h      | 63 ++++++++++++++++++++++++++++++----
 include/linux/context_tracking_state.h |  4 +--
 kernel/context_tracking.c              |  4 +--
 5 files changed, 95 insertions(+), 18 deletions(-)

-- 
2.5.0

[PATCH v3 1/5] x86/entry/64: Fix irqflag tracing wrt context tracking

[Andy Lutomirski]

Paolo pointed out that enter_from_user_mode could be called while
irqflags were traced as though IRQs were on.

In principle, this could confuse lockdep.  It doesn't cause any
problems that I've seen in any configuration, but if I build with
CONFIG_DEBUG_LOCKDEP=y, enable a nohz_full CPU, and add code like:

	if (irqs_disabled()) {
		spin_lock(&something);
		spin_unlock(&something);
	}

to the top of enter_from_user_mode, then lockdep will complain
without this fix.  It seems that lockdep's irqflags sanity checks
are too weak to detect this bug without forcing the issue.

This patch adds one byte to normal kernels, and it's IMO a bit ugly.
I haven't spotted a better way to do this yet, though.  The issue is
that we can't do TRACE_IRQS_OFF until after SWAPGS (if needed), but
we're also supposed to do it before calling C code.

An alternative approach would be to call trace_hardirqs_off in
enter_from_user_mode.  That would be less code and would not bloat
normal kernels at all, but it would be harder to see how the code
worked.

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/entry/entry_64.S | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 53616ca03244..a55697d19824 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -509,6 +509,17 @@ END(irq_entries_start)
 	 * tracking that we're in kernel mode.
 	 */
 	SWAPGS
+
+	/*
+	 * We need to tell lockdep that IRQs are off.  We can't do this until
+	 * we fix gsbase, and we should do it before enter_from_user_mode
+	 * (which can take locks).  Since TRACE_IRQS_OFF idempotent,
+	 * the simplest way to handle it is to just call it twice if
+	 * we enter from user mode.  There's no reason to optimize this since
+	 * TRACE_IRQS_OFF is a no-op if lockdep is off.
+	 */
+	TRACE_IRQS_OFF
+
 #ifdef CONFIG_CONTEXT_TRACKING
 	call enter_from_user_mode
 #endif
@@ -1049,12 +1060,18 @@ ENTRY(error_entry)
 	SWAPGS
 
 .Lerror_entry_from_usermode_after_swapgs:
+	/*
+	 * We need to tell lockdep that IRQs are off.  We can't do this until
+	 * we fix gsbase, and we should do it before enter_from_user_mode
+	 * (which can take locks).
+	 */
+	TRACE_IRQS_OFF
 #ifdef CONFIG_CONTEXT_TRACKING
 	call enter_from_user_mode
 #endif
+	ret
 
 .Lerror_entry_done:
-
 	TRACE_IRQS_OFF
 	ret
 
-- 
2.5.0

[PATCH v3 2/5] context_tracking: Switch to new static_branch API

[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

[PATCH v3 3/5] x86/asm: Error out if asm/jump_label.h is included inappropriately

[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

[PATCH v3 4/5] x86/asm: Add asm macros for static keys/jump labels

[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

[PATCH v3 5/5] x86/entry/64: Bypass enter_from_user_mode on non-context-tracking boots

[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

Re: [PATCH v3 3/5] x86/asm: Error out if asm/jump_label.h is included inappropriately

[Thomas Gleixner]

On Thu, 12 Nov 2015, Andy Lutomirski wrote:

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

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

Re: [PATCH v3 4/5] x86/asm: Add asm macros for static keys/jump labels

[Thomas Gleixner]

On Thu, 12 Nov 2015, Andy Lutomirski wrote:

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

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

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

[Thomas Gleixner]

On Thu, 12 Nov 2015, Andy Lutomirski 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>

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

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

[Frederic Weisbecker]

On Thu, Nov 12, 2015 at 12:59:04PM -0800, Andy Lutomirski 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.

Looks good. But why are we still calling context tracking code on IRQs at all?

Thanks.

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

[Andy Lutomirski]

On Nov 13, 2015 7:26 AM, "Frederic Weisbecker" <fweisbec@gmail.com> wrote:
>
> On Thu, Nov 12, 2015 at 12:59:04PM -0800, Andy Lutomirski 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.
>
> Looks good. But why are we still calling context tracking code on IRQs at all?

Same reasons as before:

1. This way the IRQ exit path is almost completely shared with all the
other exit paths.

2. It combines the checks for which context we were in with what CPL
we entered from.

Part 2 should be complete across the whole x86 kernel soon once the
64-bit syscall code gets fixed up.

We should get rid of the duplication in the irq entry hooks.  Want to
help with that?  Presumably we should do the massive remote polling
speedup to the nohz code, and we should also teach
enter_from_user_mode to transition directly to IRQ state as
appropriate.  Then irq_enter can be much faster.

--Andy

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

[Frederic Weisbecker]

On Mon, Nov 16, 2015 at 11:10:55AM -0800, Andy Lutomirski wrote:
> On Nov 13, 2015 7:26 AM, "Frederic Weisbecker" <fweisbec@gmail.com> wrote:
> >
> > On Thu, Nov 12, 2015 at 12:59:04PM -0800, Andy Lutomirski 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.
> >
> > Looks good. But why are we still calling context tracking code on IRQs at all?
> 
> Same reasons as before:
> 
> 1. This way the IRQ exit path is almost completely shared with all the
> other exit paths.

I'm all for consolidation in general. Unless it brings bad middle states.

If I knew before that I would have to argue endlessly in order to protest against
these context tracking changes, I would have NACK'ed the x86 consolidation rework in
the state it was while it got merged.

> 
> 2. It combines the checks for which context we were in with what CPL
> we entered from.
> 
> Part 2 should be complete across the whole x86 kernel soon once the
> 64-bit syscall code gets fixed up.
> 
> We should get rid of the duplication in the irq entry hooks.  Want to
> help with that?

Which one? The duplication against irq_enter() and irq_exit()?

I think that irq_exit() should be moved to the IRQ very end and perform the
final signal/schedule/preempt_schedule_irq() loop. But it requires a bit of
rework on all archs in order to do that. This could be done iteratively though.

> Presumably we should do the massive remote polling speedup to the nohz code,

Hmm, I don't get what you mean here.

> and we should also teach enter_from_user_mode to transition directly to IRQ state as
> appropriate.  Then irq_enter can be much faster.

I don't get what you mean here either. You mean calling irq_enter() from enter_from_user_mode()?

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

[Andy Lutomirski]

On Mon, Nov 16, 2015 at 2:50 PM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> On Mon, Nov 16, 2015 at 11:10:55AM -0800, Andy Lutomirski wrote:
>> On Nov 13, 2015 7:26 AM, "Frederic Weisbecker" <fweisbec@gmail.com> wrote:
>> >
>> > On Thu, Nov 12, 2015 at 12:59:04PM -0800, Andy Lutomirski 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.
>> >
>> > Looks good. But why are we still calling context tracking code on IRQs at all?
>>
>> Same reasons as before:
>>
>> 1. This way the IRQ exit path is almost completely shared with all the
>> other exit paths.
>
> I'm all for consolidation in general. Unless it brings bad middle states.

The middle state works fine, though.  With these patches, the middle
state should have essentially no performance hit compared to the
previous state in default configurations.

>
> If I knew before that I would have to argue endlessly in order to protest against
> these context tracking changes, I would have NACK'ed the x86 consolidation rework in
> the state it was while it got merged.
>
>>
>> 2. It combines the checks for which context we were in with what CPL
>> we entered from.
>>
>> Part 2 should be complete across the whole x86 kernel soon once the
>> 64-bit syscall code gets fixed up.
>>
>> We should get rid of the duplication in the irq entry hooks.  Want to
>> help with that?
>
> Which one? The duplication against irq_enter() and irq_exit()?

Yes.

>
> I think that irq_exit() should be moved to the IRQ very end and perform the
> final signal/schedule/preempt_schedule_irq() loop. But it requires a bit of
> rework on all archs in order to do that. This could be done iteratively though.
>
>> Presumably we should do the massive remote polling speedup to the nohz code,
>
> Hmm, I don't get what you mean here.
>

Currently (4.4-rc1), when an IRQ hits user mode, here's roughly what we do:

 - Tell context tracking that we're in the kernel
    - Switch ct state
    - Wake up RCU
    - Adjust vtime
 - irq_enter
    - Adjust preempt count
    - Wake up RCU
    - Tell vtime accounting that we're in an IRQ

All of the initial stuff should be, in the long term, just a write to
some variable and a possible barrier.  Whatever CPU is doing
housekeeping can poll to keep track of user vs system time.  The
irq_enter stuff, in turn, could either set some variable telling the
housekeeper that we're in an IRQ or it could continue to directly
adjust time accounting.

In any event, all of this should be extremely fast, which it currently isn't.

>> and we should also teach enter_from_user_mode to transition directly to IRQ state as
>> appropriate.  Then irq_enter can be much faster.
>
> I don't get what you mean here either. You mean calling irq_enter() from enter_from_user_mode()?
>

No, I mean teaching irq_enter that, on x86 at least, we promise that
irq_enter is only ever called from CONTEXT_KERNEL so it can do less
redundant work.

Or, even better, we could fold the irq_enter and user->kernel hooks
into a single context tracking call.

--Andy

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

[Frederic Weisbecker]

On Mon, Nov 16, 2015 at 03:57:05PM -0800, Andy Lutomirski wrote:
> On Mon, Nov 16, 2015 at 2:50 PM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > On Mon, Nov 16, 2015 at 11:10:55AM -0800, Andy Lutomirski wrote:
> >> On Nov 13, 2015 7:26 AM, "Frederic Weisbecker" <fweisbec@gmail.com> wrote:
> >> >
> >> > On Thu, Nov 12, 2015 at 12:59:04PM -0800, Andy Lutomirski 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.
> >> >
> >> > Looks good. But why are we still calling context tracking code on IRQs at all?
> >>
> >> Same reasons as before:
> >>
> >> 1. This way the IRQ exit path is almost completely shared with all the
> >> other exit paths.
> >
> > I'm all for consolidation in general. Unless it brings bad middle states.
> 
> The middle state works fine, though.  With these patches, the middle
> state should have essentially no performance hit compared to the
> previous state in default configurations.

The current middle state works but introduced a performance hit on IRQs.
But yeah this patchset doesn't make the situation worse.

> 
> >
> > If I knew before that I would have to argue endlessly in order to protest against
> > these context tracking changes, I would have NACK'ed the x86 consolidation rework in
> > the state it was while it got merged.
> >
> >>
> >> 2. It combines the checks for which context we were in with what CPL
> >> we entered from.
> >>
> >> Part 2 should be complete across the whole x86 kernel soon once the
> >> 64-bit syscall code gets fixed up.
> >>
> >> We should get rid of the duplication in the irq entry hooks.  Want to
> >> help with that?
> >
> > Which one? The duplication against irq_enter() and irq_exit()?
> 
> Yes.

The duplication you introduced then :-)

> 
> >
> > I think that irq_exit() should be moved to the IRQ very end and perform the
> > final signal/schedule/preempt_schedule_irq() loop. But it requires a bit of
> > rework on all archs in order to do that. This could be done iteratively though.
> >
> >> Presumably we should do the massive remote polling speedup to the nohz code,
> >
> > Hmm, I don't get what you mean here.
> >
> 
> Currently (4.4-rc1), when an IRQ hits user mode, here's roughly what we do:
> 
>  - Tell context tracking that we're in the kernel
>     - Switch ct state
>     - Wake up RCU
>     - Adjust vtime
>  - irq_enter
>     - Adjust preempt count
>     - Wake up RCU
>     - Tell vtime accounting that we're in an IRQ
> 
> All of the initial stuff should be, in the long term, just a write to
> some variable and a possible barrier.

I think that eventually we may indeed want to merge context tracking and
preempt count concerning kernel and irq tracking. Eventually we can
indeed just write IRQ & KERNEL to context tracking at once on irq entry
and clear that on exit with a special treatment in case of IRQ exit
rescheduling.

The case if RCU is special though, I'm not sure we can merge it with the rest.

Also it means irq_exit() will have to take care of irq exit rescheduling and signals.
I think this is something we can do.

> Whatever CPU is doing
> housekeeping can poll to keep track of user vs system time.

If you're assuming the case where housekeeping will poll on tickless CPUs cputime,
I'm not sure we are going to take that path. This is going to be a lot of overhead.

> The irq_enter stuff, in turn, could either set some variable telling the
> housekeeper that we're in an IRQ or it could continue to directly
> adjust time accounting.
> 
> In any event, all of this should be extremely fast, which it currently isn't.

Definetly.

> 
> >> and we should also teach enter_from_user_mode to transition directly to IRQ state as
> >> appropriate.  Then irq_enter can be much faster.
> >
> > I don't get what you mean here either. You mean calling irq_enter() from enter_from_user_mode()?
> >
> 
> No, I mean teaching irq_enter that, on x86 at least, we promise that
> irq_enter is only ever called from CONTEXT_KERNEL so it can do less
> redundant work.

I don't get how making sure that irq_enter() is only called with CONTEXT_KERNEL
is going to imply less redundant work. But doing the irq tracking from context tracking
is certainly going to factorize things.

> Or, even better, we could fold the irq_enter and user->kernel hooks
> into a single context tracking call.

Yeah that makes sense.

Now this all sounds like a lot of core rework just for a feature (nohz/context tracking)
that is used by few users. Lets hope we can agree on a solution that is an improvement for
everyone. For example moving the irq end work on core code sounds like something that would
be desired for everyone.

If we happen to agree on a solution, I'll definetly help and do the improvements on context
tracking subsystem.

Thanks.

[tip:x86/asm] x86/entry/64: Fix irqflag tracing wrt context tracking

[tip-bot for Andy Lutomirski]

Commit-ID:  f10750536fa783cafb2653f6fa349d6e62337e42
Gitweb:     http://git.kernel.org/tip/f10750536fa783cafb2653f6fa349d6e62337e42
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Thu, 12 Nov 2015 12:59:00 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 24 Nov 2015 09:55:02 +0100

x86/entry/64: Fix irqflag tracing wrt context tracking

Paolo pointed out that enter_from_user_mode could be called
while irqflags were traced as though IRQs were on.

In principle, this could confuse lockdep.  It doesn't cause any
problems that I've seen in any configuration, but if I build
with CONFIG_DEBUG_LOCKDEP=y, enable a nohz_full CPU, and add
code like:

	if (irqs_disabled()) {
		spin_lock(&something);
		spin_unlock(&something);
	}

to the top of enter_from_user_mode, then lockdep will complain
without this fix.  It seems that lockdep's irqflags sanity
checks are too weak to detect this bug without forcing the
issue.

This patch adds one byte to normal kernels, and it's IMO a bit
ugly. I haven't spotted a better way to do this yet, though.
The issue is that we can't do TRACE_IRQS_OFF until after SWAPGS
(if needed), but we're also supposed to do it before calling C
code.

An alternative approach would be to call trace_hardirqs_off in
enter_from_user_mode.  That would be less code and would not
bloat normal kernels at all, but it would be harder to see how
the code worked.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/86237e362390dfa6fec12de4d75a238acb0ae787.1447361906.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/entry/entry_64.S | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 53616ca..a55697d 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -509,6 +509,17 @@ END(irq_entries_start)
 	 * tracking that we're in kernel mode.
 	 */
 	SWAPGS
+
+	/*
+	 * We need to tell lockdep that IRQs are off.  We can't do this until
+	 * we fix gsbase, and we should do it before enter_from_user_mode
+	 * (which can take locks).  Since TRACE_IRQS_OFF idempotent,
+	 * the simplest way to handle it is to just call it twice if
+	 * we enter from user mode.  There's no reason to optimize this since
+	 * TRACE_IRQS_OFF is a no-op if lockdep is off.
+	 */
+	TRACE_IRQS_OFF
+
 #ifdef CONFIG_CONTEXT_TRACKING
 	call enter_from_user_mode
 #endif
@@ -1049,12 +1060,18 @@ ENTRY(error_entry)
 	SWAPGS
 
 .Lerror_entry_from_usermode_after_swapgs:
+	/*
+	 * We need to tell lockdep that IRQs are off.  We can't do this until
+	 * we fix gsbase, and we should do it before enter_from_user_mode
+	 * (which can take locks).
+	 */
+	TRACE_IRQS_OFF
 #ifdef CONFIG_CONTEXT_TRACKING
 	call enter_from_user_mode
 #endif
+	ret
 
 .Lerror_entry_done:
-
 	TRACE_IRQS_OFF
 	ret
 

[tip:x86/asm] context_tracking: Switch to new static_branch API

[tip-bot for Andy Lutomirski]

Commit-ID:  ed11a7f1b3bd482bd7d6ef7bc2859c41fb43b9ee
Gitweb:     http://git.kernel.org/tip/ed11a7f1b3bd482bd7d6ef7bc2859c41fb43b9ee
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Thu, 12 Nov 2015 12:59:01 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 24 Nov 2015 09:56:43 +0100

context_tracking: Switch to new static_branch API

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

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/812df7e64f120c5c7c08481f36a8caa9f53b2199.1447361906.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@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 ee956c5..1d34fe6 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 d8560ee..9ad37b9 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)

[tip:x86/asm] x86/asm: Error out if asm/ jump_label.h is included inappropriately

[tip-bot for Andy Lutomirski]

Commit-ID:  c28454332fe0b65e22c3a2717e5bf05b5b47ca20
Gitweb:     http://git.kernel.org/tip/c28454332fe0b65e22c3a2717e5bf05b5b47ca20
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Thu, 12 Nov 2015 12:59:02 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 24 Nov 2015 09:56:43 +0100

x86/asm: Error out if asm/jump_label.h is included inappropriately

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>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/99407f0ac7fa3ab03a3d31ce076d47b5c2f44795.1447361906.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@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 5daeca3..96872dc 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>

[tip:x86/asm] x86/asm: Add asm macros for static keys/jump labels

[tip-bot for Andy Lutomirski]

Commit-ID:  2671c3e4fe2a34bd9bf2eecdf5d1149d4b55dbdf
Gitweb:     http://git.kernel.org/tip/2671c3e4fe2a34bd9bf2eecdf5d1149d4b55dbdf
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Thu, 12 Nov 2015 12:59:03 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 24 Nov 2015 09:56:44 +0100

x86/asm: Add asm macros for static keys/jump labels

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>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/63aa45c4b692e8469e1876d6ccbb5da707972990.1447361906.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@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 96872dc..adc54c1 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

[tip:x86/asm] x86/entry/64: Bypass enter_from_user_mode on non-context-tracking boots

[tip-bot for Andy Lutomirski]

Commit-ID:  478dc89cf316697e8029411a64ea2b30c528434d
Gitweb:     http://git.kernel.org/tip/478dc89cf316697e8029411a64ea2b30c528434d
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Thu, 12 Nov 2015 12:59:04 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 24 Nov 2015 09:56:44 +0100

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>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/73ee804fff48cd8c66b65b724f9f728a11a8c686.1447361906.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@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 3c71dd9..e32206e 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 a55697d..9d34d3c 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: