LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/5] x86/asm/entry/64: simplifications
@ 2015-03-30 18:09 Denys Vlasenko
  2015-03-30 18:09 ` [PATCH 1/5] x86/asm/entry/64: move retint_kernel code block closer to its user Denys Vlasenko
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Denys Vlasenko @ 2015-03-30 18:09 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Denys Vlasenko, Linus Torvalds, Steven Rostedt, Borislav Petkov,
	H. Peter Anvin, Andy Lutomirski, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	x86, linux-kernel

This patchset simplifies jump maze in entry_64.S a bit by moving
"retint_kernel" code block, and follows up with simplifications
which become obvious after the move.

Two labels and one define is gone
One label is no longer .global
One redundand DISABLE_INTERRUPTS is gone
A few jump and test instructions use shorter forms now

   text           data     bss     dec     hex filename
  12706              0       0   12706    31a2 entry_64.o.before
  12658              0       0   12658    3172 entry_64.o

Patchset was run-tested (running it while writing this).

Denys Vlasenko (5):
  x86/asm/entry/64: move retint_kernel code block closer to its user
  x86/asm/entry/64: simplify retint_kernel label usage, make
    retint_restore_args label local
  x86/asm/entry/64: remove redundant DISABLE_INTERRUPTS
  x86/asm/entry/64: do not GET_THREAD_INFO() too early
  x86/asm/entry/64: drop exit_intr label

 arch/x86/kernel/entry_64.S | 40 +++++++++++++++++-----------------------
 1 file changed, 17 insertions(+), 23 deletions(-)
---
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org

-- 
1.8.1.4


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

* [PATCH 1/5] x86/asm/entry/64: move retint_kernel code block closer to its user
  2015-03-30 18:09 [PATCH 0/5] x86/asm/entry/64: simplifications Denys Vlasenko
@ 2015-03-30 18:09 ` Denys Vlasenko
  2015-03-31 12:37   ` [tip:x86/asm] x86/asm/entry/64: Move " tip-bot for Denys Vlasenko
  2015-03-30 18:09 ` [PATCH 2/5] x86/asm/entry/64: simplify retint_kernel label usage, make retint_restore_args label local Denys Vlasenko
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Denys Vlasenko @ 2015-03-30 18:09 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Denys Vlasenko, Linus Torvalds, Steven Rostedt, Borislav Petkov,
	H. Peter Anvin, Andy Lutomirski, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	x86, linux-kernel

"retint_kernel" code block is misplaced. Since its logical continuation
is "retint_restore_args", it is more natural to place it above that label.
This also makes two jumps "short".

This change only moves code block around, without changing logic.

This enables the next simplification: making "retint_restore_args"
label a local numeric one.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/kernel/entry_64.S | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index dbfc8875..34d60c3 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -740,7 +740,19 @@ opportunistic_sysret_failed:
 	SWAPGS
 	jmp restore_args
 
-retint_restore_args:	/* return to kernel space */
+/* Returning to kernel space */
+#ifdef CONFIG_PREEMPT
+	/* Interrupts are off */
+	/* Check if we need preemption */
+ENTRY(retint_kernel)
+	cmpl	$0,PER_CPU_VAR(__preempt_count)
+	jnz	retint_restore_args
+	bt	$9,EFLAGS(%rsp)	/* interrupts were off? */
+	jnc	retint_restore_args
+	call	preempt_schedule_irq
+	jmp	exit_intr
+#endif
+retint_restore_args:
 	DISABLE_INTERRUPTS(CLBR_ANY)
 	/*
 	 * The iretq could re-enable interrupts:
@@ -830,17 +842,6 @@ retint_signal:
 	GET_THREAD_INFO(%rcx)
 	jmp retint_with_reschedule
 
-#ifdef CONFIG_PREEMPT
-	/* Returning to kernel space. Check if we need preemption */
-	/* rcx:	 threadinfo. interrupts off. */
-ENTRY(retint_kernel)
-	cmpl $0,PER_CPU_VAR(__preempt_count)
-	jnz  retint_restore_args
-	bt   $9,EFLAGS(%rsp)	/* interrupts off? */
-	jnc  retint_restore_args
-	call preempt_schedule_irq
-	jmp exit_intr
-#endif
 	CFI_ENDPROC
 END(common_interrupt)
 
-- 
1.8.1.4


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

* [PATCH 2/5] x86/asm/entry/64: simplify retint_kernel label usage, make retint_restore_args label local
  2015-03-30 18:09 [PATCH 0/5] x86/asm/entry/64: simplifications Denys Vlasenko
  2015-03-30 18:09 ` [PATCH 1/5] x86/asm/entry/64: move retint_kernel code block closer to its user Denys Vlasenko
@ 2015-03-30 18:09 ` Denys Vlasenko
  2015-03-30 18:18   ` Linus Torvalds
  2015-03-30 18:09 ` [PATCH 3/5] x86/asm/entry/64: remove redundant DISABLE_INTERRUPTS Denys Vlasenko
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Denys Vlasenko @ 2015-03-30 18:09 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Denys Vlasenko, Linus Torvalds, Steven Rostedt, Borislav Petkov,
	H. Peter Anvin, Andy Lutomirski, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	x86, linux-kernel

Get rid of #define obfuscation of retint_kernel in CONFIG_PREEMPT case
by defining retint_kernel label always, not only for CONFIG_PREEMPT.

Strip retint_kernel of .global-ness (ENTRY macro) - it has no users
outside of this file.

This looks like cosmetics, but it is not:
"je LABEL" can be optimized to short jump by assember
only if LABEL is not global, for global labels jump is always
a near one with relocation.

Convert retint_restore_args to a local numeric label, making it clearer
that it is not used elsewhere in the file.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/kernel/entry_64.S | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 34d60c3..4d723ed 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -57,10 +57,6 @@
 	.section .entry.text, "ax"
 
 
-#ifndef CONFIG_PREEMPT
-#define retint_kernel retint_restore_args
-#endif
-
 #ifdef CONFIG_PARAVIRT
 ENTRY(native_usergs_sysret64)
 	swapgs
@@ -741,18 +737,18 @@ opportunistic_sysret_failed:
 	jmp restore_args
 
 /* Returning to kernel space */
+retint_kernel:
 #ifdef CONFIG_PREEMPT
 	/* Interrupts are off */
 	/* Check if we need preemption */
-ENTRY(retint_kernel)
 	cmpl	$0,PER_CPU_VAR(__preempt_count)
-	jnz	retint_restore_args
+	jnz	1f
 	bt	$9,EFLAGS(%rsp)	/* interrupts were off? */
-	jnc	retint_restore_args
+	jnc	1f
 	call	preempt_schedule_irq
 	jmp	exit_intr
+1:
 #endif
-retint_restore_args:
 	DISABLE_INTERRUPTS(CLBR_ANY)
 	/*
 	 * The iretq could re-enable interrupts:
-- 
1.8.1.4


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

* [PATCH 3/5] x86/asm/entry/64: remove redundant DISABLE_INTERRUPTS
  2015-03-30 18:09 [PATCH 0/5] x86/asm/entry/64: simplifications Denys Vlasenko
  2015-03-30 18:09 ` [PATCH 1/5] x86/asm/entry/64: move retint_kernel code block closer to its user Denys Vlasenko
  2015-03-30 18:09 ` [PATCH 2/5] x86/asm/entry/64: simplify retint_kernel label usage, make retint_restore_args label local Denys Vlasenko
@ 2015-03-30 18:09 ` Denys Vlasenko
  2015-03-30 18:09 ` [PATCH 4/5] x86/asm/entry/64: do not GET_THREAD_INFO() too early Denys Vlasenko
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Denys Vlasenko @ 2015-03-30 18:09 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Denys Vlasenko, Linus Torvalds, Steven Rostedt, Borislav Petkov,
	H. Peter Anvin, Andy Lutomirski, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	x86, linux-kernel

At this location, we already have interrupts off, always.
To be more specific, we already disabled them here:

    ret_from_intr:
	    DISABLE_INTERRUPTS(CLBR_NONE)

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/kernel/entry_64.S | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 4d723ed..290ecb3 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -749,7 +749,6 @@ retint_kernel:
 	jmp	exit_intr
 1:
 #endif
-	DISABLE_INTERRUPTS(CLBR_ANY)
 	/*
 	 * The iretq could re-enable interrupts:
 	 */
-- 
1.8.1.4


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

* [PATCH 4/5] x86/asm/entry/64: do not GET_THREAD_INFO() too early
  2015-03-30 18:09 [PATCH 0/5] x86/asm/entry/64: simplifications Denys Vlasenko
                   ` (2 preceding siblings ...)
  2015-03-30 18:09 ` [PATCH 3/5] x86/asm/entry/64: remove redundant DISABLE_INTERRUPTS Denys Vlasenko
@ 2015-03-30 18:09 ` Denys Vlasenko
  2015-03-31 12:37   ` [tip:x86/asm] x86/asm/entry/64: Do " tip-bot for Denys Vlasenko
  2015-03-30 18:09 ` [PATCH 5/5] x86/asm/entry/64: drop exit_intr label Denys Vlasenko
  2015-03-31 15:47 ` [PATCH 0/5] x86/asm/entry/64: simplifications Ingo Molnar
  5 siblings, 1 reply; 13+ messages in thread
From: Denys Vlasenko @ 2015-03-30 18:09 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Denys Vlasenko, Linus Torvalds, Steven Rostedt, Borislav Petkov,
	H. Peter Anvin, Andy Lutomirski, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	x86, linux-kernel

At exit_intr, we GET_THREAD_INFO(%rcx) and then jump to retint_kernel
if saved CS was from kernel. But code at retint_kernel doesn't need %rcx.

Move GET_THREAD_INFO(%rcx) down, after CS check and branch.

While at it, remove "has a correct top of stack" comment.
After recent changes which eliminated FIXUP_TOP_OF_STACK,
we always have correct pt_regs layout.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/kernel/entry_64.S | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 290ecb3..16bf357 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -654,13 +654,12 @@ ret_from_intr:
 	CFI_ADJUST_CFA_OFFSET	RBP
 
 exit_intr:
-	GET_THREAD_INFO(%rcx)
 	testl $3,CS(%rsp)
 	je retint_kernel
-
 	/* Interrupt came from user space */
+
+	GET_THREAD_INFO(%rcx)
 	/*
-	 * Has a correct top of stack.
 	 * %rcx: thread info. Interrupts off.
 	 */
 retint_with_reschedule:
-- 
1.8.1.4


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

* [PATCH 5/5] x86/asm/entry/64: drop exit_intr label
  2015-03-30 18:09 [PATCH 0/5] x86/asm/entry/64: simplifications Denys Vlasenko
                   ` (3 preceding siblings ...)
  2015-03-30 18:09 ` [PATCH 4/5] x86/asm/entry/64: do not GET_THREAD_INFO() too early Denys Vlasenko
@ 2015-03-30 18:09 ` Denys Vlasenko
  2015-03-30 18:22   ` Linus Torvalds
  2015-03-31 10:33   ` [PATCH 5/5 v2] x86/asm/entry/64: simplify looping around preempt_schedule_irq Denys Vlasenko
  2015-03-31 15:47 ` [PATCH 0/5] x86/asm/entry/64: simplifications Ingo Molnar
  5 siblings, 2 replies; 13+ messages in thread
From: Denys Vlasenko @ 2015-03-30 18:09 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Denys Vlasenko, Linus Torvalds, Steven Rostedt, Borislav Petkov,
	H. Peter Anvin, Andy Lutomirski, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	x86, linux-kernel

At this label, we test whether interrupt/exception was in kernel.
If it did, we jump to preemption check. If preemption does happen
(IOW if we call preempt_schedule_irq), we go back to exit_intr.

But it's pointless, we already know that test succeeded last time,
preemption doesn't change the fact that interrupt/exception
was in kernel. We can go back directly to checking
PER_CPU_VAR(__preempt_count) instead.

This makes exit_intr label unused. Dropping it.

While at it, tidy up TEST insn width to use shorter insn form,
use logically correct JZ mnemonic instead of JE (this doesn't change code),
use X86_EFLAGS_IF_BIT instead of literal 9.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/kernel/entry_64.S | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 16bf357..bc90f63 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -653,9 +653,8 @@ ret_from_intr:
 	CFI_DEF_CFA_REGISTER	rsp
 	CFI_ADJUST_CFA_OFFSET	RBP
 
-exit_intr:
-	testl $3,CS(%rsp)
-	je retint_kernel
+	testb $3,CS(%rsp)
+	jz retint_kernel
 	/* Interrupt came from user space */
 
 	GET_THREAD_INFO(%rcx)
@@ -740,12 +739,12 @@ retint_kernel:
 #ifdef CONFIG_PREEMPT
 	/* Interrupts are off */
 	/* Check if we need preemption */
-	cmpl	$0,PER_CPU_VAR(__preempt_count)
-	jnz	1f
-	bt	$9,EFLAGS(%rsp)	/* interrupts were off? */
+	bt	$X86_EFLAGS_IF_BIT,EFLAGS(%rsp)	/* interrupts were off? */
 	jnc	1f
+0:	cmpl	$0,PER_CPU_VAR(__preempt_count)
+	jnz	1f
 	call	preempt_schedule_irq
-	jmp	exit_intr
+	jmp	0b
 1:
 #endif
 	/*
-- 
1.8.1.4

 

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

* Re: [PATCH 2/5] x86/asm/entry/64: simplify retint_kernel label usage, make retint_restore_args label local
  2015-03-30 18:09 ` [PATCH 2/5] x86/asm/entry/64: simplify retint_kernel label usage, make retint_restore_args label local Denys Vlasenko
@ 2015-03-30 18:18   ` Linus Torvalds
  0 siblings, 0 replies; 13+ messages in thread
From: Linus Torvalds @ 2015-03-30 18:18 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Ingo Molnar, Steven Rostedt, Borislav Petkov, H. Peter Anvin,
	Andy Lutomirski, Oleg Nesterov, Frederic Weisbecker,
	Alexei Starovoitov, Will Drewry, Kees Cook,
	the arch/x86 maintainers, Linux Kernel Mailing List

On Mon, Mar 30, 2015 at 11:09 AM, Denys Vlasenko <dvlasenk@redhat.com> wrote:
>
> Strip retint_kernel of .global-ness (ENTRY macro) - it has no users
> outside of this file.

The reason for the ENTRY is to get it aligned, not because it's
globally visible. It doesn't have any fallthroughs, and some cpu's
care more than others.

See commit b06babac45e1 ("Add proper alignment to ENTRY").

Not that I'm sure the alignment matters all that much, but I thought
I'd point it out. Removing the ENTRY() does a lot more than just
remote the .globl.

                     Linus

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

* Re: [PATCH 5/5] x86/asm/entry/64: drop exit_intr label
  2015-03-30 18:09 ` [PATCH 5/5] x86/asm/entry/64: drop exit_intr label Denys Vlasenko
@ 2015-03-30 18:22   ` Linus Torvalds
  2015-03-31 10:33   ` [PATCH 5/5 v2] x86/asm/entry/64: simplify looping around preempt_schedule_irq Denys Vlasenko
  1 sibling, 0 replies; 13+ messages in thread
From: Linus Torvalds @ 2015-03-30 18:22 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Ingo Molnar, Steven Rostedt, Borislav Petkov, H. Peter Anvin,
	Andy Lutomirski, Oleg Nesterov, Frederic Weisbecker,
	Alexei Starovoitov, Will Drewry, Kees Cook,
	the arch/x86 maintainers, Linux Kernel Mailing List

On Mon, Mar 30, 2015 at 11:09 AM, Denys Vlasenko <dvlasenk@redhat.com> wrote:
> -       cmpl    $0,PER_CPU_VAR(__preempt_count)
> -       jnz     1f
> -       bt      $9,EFLAGS(%rsp) /* interrupts were off? */
> +       bt      $X86_EFLAGS_IF_BIT,EFLAGS(%rsp) /* interrupts were off? */

Since you're changing this anyway, just change it to use "testb" the
way you did the other place.

Yeah, yeah, it might make it less readable, but

        testb $2,EFLAGS+1(%rsp) /* interrupts were off? */

should be smaller and faster than "bt", and the point of this commit
was to make code smaller and faster, no?

                      Linus

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

* [PATCH 5/5 v2] x86/asm/entry/64: simplify looping around preempt_schedule_irq
  2015-03-30 18:09 ` [PATCH 5/5] x86/asm/entry/64: drop exit_intr label Denys Vlasenko
  2015-03-30 18:22   ` Linus Torvalds
@ 2015-03-31 10:33   ` Denys Vlasenko
  1 sibling, 0 replies; 13+ messages in thread
From: Denys Vlasenko @ 2015-03-31 10:33 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Denys Vlasenko, Linus Torvalds, Steven Rostedt, Borislav Petkov,
	H. Peter Anvin, Andy Lutomirski, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	x86, linux-kernel

At this label, we test whether interrupt/exception was in kernel.
If it did, we jump to preemption check. If preemption does happen
(IOW if we call preempt_schedule_irq), we go back to exit_intr.

But it's pointless, we already know that test succeeded last time,
preemption doesn't change the fact that interrupt/exception
was in kernel. We can go back directly to checking
PER_CPU_VAR(__preempt_count) instead.

This makes exit_intr label unused. Dropping it.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---

Changes in v2:
* split insn cleanups into a separate patch
* improve commit log message

 arch/x86/kernel/entry_64.S | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 16bf357..356266c 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -653,7 +653,6 @@ ret_from_intr:
 	CFI_DEF_CFA_REGISTER	rsp
 	CFI_ADJUST_CFA_OFFSET	RBP
 
-exit_intr:
 	testl $3,CS(%rsp)
 	je retint_kernel
 	/* Interrupt came from user space */
@@ -740,12 +739,12 @@ retint_kernel:
 #ifdef CONFIG_PREEMPT
 	/* Interrupts are off */
 	/* Check if we need preemption */
-	cmpl	$0,PER_CPU_VAR(__preempt_count)
-	jnz	1f
 	bt	$9,EFLAGS(%rsp)	/* interrupts were off? */
 	jnc	1f
+0:	cmpl	$0,PER_CPU_VAR(__preempt_count)
+	jnz	1f
 	call	preempt_schedule_irq
-	jmp	exit_intr
+	jmp	0b
 1:
 #endif
 	/*
-- 
1.8.1.4


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

* [tip:x86/asm] x86/asm/entry/64: Move retint_kernel code block closer to its user
  2015-03-30 18:09 ` [PATCH 1/5] x86/asm/entry/64: move retint_kernel code block closer to its user Denys Vlasenko
@ 2015-03-31 12:37   ` tip-bot for Denys Vlasenko
  0 siblings, 0 replies; 13+ messages in thread
From: tip-bot for Denys Vlasenko @ 2015-03-31 12:37 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: fweisbec, tglx, wad, rostedt, ast, oleg, luto, hpa, bp, dvlasenk,
	torvalds, mingo, keescook, linux-kernel

Commit-ID:  627276cb55f33e2dc56eab5b973937c128b7704c
Gitweb:     http://git.kernel.org/tip/627276cb55f33e2dc56eab5b973937c128b7704c
Author:     Denys Vlasenko <dvlasenk@redhat.com>
AuthorDate: Mon, 30 Mar 2015 20:09:31 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 31 Mar 2015 09:31:11 +0200

x86/asm/entry/64: Move retint_kernel code block closer to its user

The "retint_kernel" code block is misplaced. Since its logical
continuation is "retint_restore_args", it is more natural to
place it above that label. This also makes two jumps "short".

This change only moves code block around, without changing
logic.

This enables the next simplification: making
"retint_restore_args" label a local numeric one.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Drewry <wad@chromium.org>
Link: http://lkml.kernel.org/r/1427738975-7391-2-git-send-email-dvlasenk@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/entry_64.S | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index dbfc8875..34d60c3 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -740,7 +740,19 @@ opportunistic_sysret_failed:
 	SWAPGS
 	jmp restore_args
 
-retint_restore_args:	/* return to kernel space */
+/* Returning to kernel space */
+#ifdef CONFIG_PREEMPT
+	/* Interrupts are off */
+	/* Check if we need preemption */
+ENTRY(retint_kernel)
+	cmpl	$0,PER_CPU_VAR(__preempt_count)
+	jnz	retint_restore_args
+	bt	$9,EFLAGS(%rsp)	/* interrupts were off? */
+	jnc	retint_restore_args
+	call	preempt_schedule_irq
+	jmp	exit_intr
+#endif
+retint_restore_args:
 	DISABLE_INTERRUPTS(CLBR_ANY)
 	/*
 	 * The iretq could re-enable interrupts:
@@ -830,17 +842,6 @@ retint_signal:
 	GET_THREAD_INFO(%rcx)
 	jmp retint_with_reschedule
 
-#ifdef CONFIG_PREEMPT
-	/* Returning to kernel space. Check if we need preemption */
-	/* rcx:	 threadinfo. interrupts off. */
-ENTRY(retint_kernel)
-	cmpl $0,PER_CPU_VAR(__preempt_count)
-	jnz  retint_restore_args
-	bt   $9,EFLAGS(%rsp)	/* interrupts off? */
-	jnc  retint_restore_args
-	call preempt_schedule_irq
-	jmp exit_intr
-#endif
 	CFI_ENDPROC
 END(common_interrupt)
 

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

* [tip:x86/asm] x86/asm/entry/64: Do not GET_THREAD_INFO() too early
  2015-03-30 18:09 ` [PATCH 4/5] x86/asm/entry/64: do not GET_THREAD_INFO() too early Denys Vlasenko
@ 2015-03-31 12:37   ` tip-bot for Denys Vlasenko
  0 siblings, 0 replies; 13+ messages in thread
From: tip-bot for Denys Vlasenko @ 2015-03-31 12:37 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: ast, wad, dvlasenk, torvalds, hpa, keescook, bp, fweisbec, luto,
	tglx, linux-kernel, rostedt, oleg, mingo

Commit-ID:  a3675b32aac81c2c4733568844f8276527a37423
Gitweb:     http://git.kernel.org/tip/a3675b32aac81c2c4733568844f8276527a37423
Author:     Denys Vlasenko <dvlasenk@redhat.com>
AuthorDate: Mon, 30 Mar 2015 20:09:34 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 31 Mar 2015 09:31:11 +0200

x86/asm/entry/64: Do not GET_THREAD_INFO() too early

At exit_intr, we GET_THREAD_INFO(%rcx) and then jump to
retint_kernel if saved CS was from kernel. But the code at
retint_kernel doesn't need %rcx.

Move GET_THREAD_INFO(%rcx) down, after CS check and branch.

While at it, remove "has a correct top of stack" comment.
After recent changes which eliminated FIXUP_TOP_OF_STACK,
we always have a correct pt_regs layout.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Drewry <wad@chromium.org>
Link: http://lkml.kernel.org/r/1427738975-7391-5-git-send-email-dvlasenk@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/entry_64.S | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 34d60c3..6f251a5 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -658,13 +658,12 @@ ret_from_intr:
 	CFI_ADJUST_CFA_OFFSET	RBP
 
 exit_intr:
-	GET_THREAD_INFO(%rcx)
 	testl $3,CS(%rsp)
 	je retint_kernel
-
 	/* Interrupt came from user space */
+
+	GET_THREAD_INFO(%rcx)
 	/*
-	 * Has a correct top of stack.
 	 * %rcx: thread info. Interrupts off.
 	 */
 retint_with_reschedule:

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

* Re: [PATCH 0/5] x86/asm/entry/64: simplifications
  2015-03-30 18:09 [PATCH 0/5] x86/asm/entry/64: simplifications Denys Vlasenko
                   ` (4 preceding siblings ...)
  2015-03-30 18:09 ` [PATCH 5/5] x86/asm/entry/64: drop exit_intr label Denys Vlasenko
@ 2015-03-31 15:47 ` Ingo Molnar
  2015-03-31 16:18   ` Denys Vlasenko
  5 siblings, 1 reply; 13+ messages in thread
From: Ingo Molnar @ 2015-03-31 15:47 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Linus Torvalds, Steven Rostedt, Borislav Petkov, H. Peter Anvin,
	Andy Lutomirski, Oleg Nesterov, Frederic Weisbecker,
	Alexei Starovoitov, Will Drewry, Kees Cook, x86, linux-kernel


* Denys Vlasenko <dvlasenk@redhat.com> wrote:

> This patchset simplifies jump maze in entry_64.S a bit by moving
> "retint_kernel" code block, and follows up with simplifications
> which become obvious after the move.

So I got conflicts with latest tip:master with 3 of the 5 patches, so 
I probably missed one of your older patches.

I pushed out the other two - mind merging all your pending patches on 
top of latest tip:master and sending out all of them as a single 
series, so that I can pick up all the uncontroversial bits?

Thanks,

	Ingo

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

* Re: [PATCH 0/5] x86/asm/entry/64: simplifications
  2015-03-31 15:47 ` [PATCH 0/5] x86/asm/entry/64: simplifications Ingo Molnar
@ 2015-03-31 16:18   ` Denys Vlasenko
  0 siblings, 0 replies; 13+ messages in thread
From: Denys Vlasenko @ 2015-03-31 16:18 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Linus Torvalds, Steven Rostedt, Borislav Petkov, H. Peter Anvin,
	Andy Lutomirski, Oleg Nesterov, Frederic Weisbecker,
	Alexei Starovoitov, Will Drewry, Kees Cook, x86, linux-kernel

On 03/31/2015 05:47 PM, Ingo Molnar wrote:
> 
> * Denys Vlasenko <dvlasenk@redhat.com> wrote:
> 
>> This patchset simplifies jump maze in entry_64.S a bit by moving
>> "retint_kernel" code block, and follows up with simplifications
>> which become obvious after the move.
> 
> So I got conflicts with latest tip:master with 3 of the 5 patches, so 
> I probably missed one of your older patches.
> 
> I pushed out the other two - mind merging all your pending patches on 
> top of latest tip:master and sending out all of them as a single 
> series, so that I can pick up all the uncontroversial bits?

Will do in ~15 minutes.

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

end of thread, other threads:[~2015-03-31 16:19 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-30 18:09 [PATCH 0/5] x86/asm/entry/64: simplifications Denys Vlasenko
2015-03-30 18:09 ` [PATCH 1/5] x86/asm/entry/64: move retint_kernel code block closer to its user Denys Vlasenko
2015-03-31 12:37   ` [tip:x86/asm] x86/asm/entry/64: Move " tip-bot for Denys Vlasenko
2015-03-30 18:09 ` [PATCH 2/5] x86/asm/entry/64: simplify retint_kernel label usage, make retint_restore_args label local Denys Vlasenko
2015-03-30 18:18   ` Linus Torvalds
2015-03-30 18:09 ` [PATCH 3/5] x86/asm/entry/64: remove redundant DISABLE_INTERRUPTS Denys Vlasenko
2015-03-30 18:09 ` [PATCH 4/5] x86/asm/entry/64: do not GET_THREAD_INFO() too early Denys Vlasenko
2015-03-31 12:37   ` [tip:x86/asm] x86/asm/entry/64: Do " tip-bot for Denys Vlasenko
2015-03-30 18:09 ` [PATCH 5/5] x86/asm/entry/64: drop exit_intr label Denys Vlasenko
2015-03-30 18:22   ` Linus Torvalds
2015-03-31 10:33   ` [PATCH 5/5 v2] x86/asm/entry/64: simplify looping around preempt_schedule_irq Denys Vlasenko
2015-03-31 15:47 ` [PATCH 0/5] x86/asm/entry/64: simplifications Ingo Molnar
2015-03-31 16:18   ` Denys Vlasenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).