2020-05-05 14:27:22

by Thomas Gleixner

[permalink] [raw]
Subject: [patch V4 part 2 05/18] x86/entry: Move irq tracing on syscall entry to C-code

Now that the C entry points are safe, move the irq flags tracing code into
the entry helper:

- Invoke lockdep before calling into context tracking

- Use the safe trace_hardirqs_on_prepare() trace function after context
tracking established state and RCU is watching.

Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/entry/common.c | 21 +++++++++++++++++++--
arch/x86/entry/entry_32.S | 12 ------------
arch/x86/entry/entry_64.S | 2 --
arch/x86/entry/entry_64_compat.S | 18 ------------------
4 files changed, 19 insertions(+), 34 deletions(-)

--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -40,19 +40,36 @@
#include <trace/events/syscalls.h>

#ifdef CONFIG_CONTEXT_TRACKING
-/* Called on entry from user mode with IRQs off. */
+/**
+ * enter_from_user_mode - Establish state when coming from user mode
+ *
+ * Syscall entry disables interrupts, but user mode is traced as interrupts
+ * enabled. Also with NO_HZ_FULL RCU might be idle.
+ *
+ * 1) Tell lockdep that interrupts are disabled
+ * 2) Invoke context tracking if enabled to reactivate RCU
+ * 3) Trace interrupts off state
+ */
__visible noinstr void enter_from_user_mode(void)
{
enum ctx_state state = ct_state();

+ lockdep_hardirqs_off(CALLER_ADDR0);
user_exit_irqoff();

instr_begin();
CT_WARN_ON(state != CONTEXT_USER);
+ trace_hardirqs_off_prepare();
instr_end();
}
#else
-static inline void enter_from_user_mode(void) {}
+static __always_inline void enter_from_user_mode(void)
+{
+ lockdep_hardirqs_off(CALLER_ADDR0);
+ instr_begin();
+ trace_hardirqs_off_prepare();
+ instr_end();
+}
#endif

static noinstr void exit_to_user_mode(void)
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -967,12 +967,6 @@ SYM_FUNC_START(entry_SYSENTER_32)
jnz .Lsysenter_fix_flags
.Lsysenter_flags_fixed:

- /*
- * User mode is traced as though IRQs are on, and SYSENTER
- * turned them off.
- */
- TRACE_IRQS_OFF
-
movl %esp, %eax
call do_fast_syscall_32
/* XEN PV guests always use IRET path */
@@ -1082,12 +1076,6 @@ SYM_FUNC_START(entry_INT80_32)

SAVE_ALL pt_regs_ax=$-ENOSYS switch_stacks=1 /* save rest */

- /*
- * User mode is traced as though IRQs are on, and the interrupt gate
- * turned them off.
- */
- TRACE_IRQS_OFF
-
movl %esp, %eax
call do_int80_syscall_32
.Lsyscall_32_done:
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -167,8 +167,6 @@ SYM_INNER_LABEL(entry_SYSCALL_64_after_h

PUSH_AND_CLEAR_REGS rax=$-ENOSYS

- TRACE_IRQS_OFF
-
/* IRQs are off. */
movq %rax, %rdi
movq %rsp, %rsi
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -129,12 +129,6 @@ SYM_FUNC_START(entry_SYSENTER_compat)
jnz .Lsysenter_fix_flags
.Lsysenter_flags_fixed:

- /*
- * User mode is traced as though IRQs are on, and SYSENTER
- * turned them off.
- */
- TRACE_IRQS_OFF
-
movq %rsp, %rdi
call do_fast_syscall_32
/* XEN PV guests always use IRET path */
@@ -247,12 +241,6 @@ SYM_INNER_LABEL(entry_SYSCALL_compat_aft
pushq $0 /* pt_regs->r15 = 0 */
xorl %r15d, %r15d /* nospec r15 */

- /*
- * User mode is traced as though IRQs are on, and SYSENTER
- * turned them off.
- */
- TRACE_IRQS_OFF
-
movq %rsp, %rdi
call do_fast_syscall_32
/* XEN PV guests always use IRET path */
@@ -403,12 +391,6 @@ SYM_CODE_START(entry_INT80_compat)
xorl %r15d, %r15d /* nospec r15 */
cld

- /*
- * User mode is traced as though IRQs are on, and the interrupt
- * gate turned them off.
- */
- TRACE_IRQS_OFF
-
movq %rsp, %rdi
call do_int80_syscall_32
.Lsyscall_32_done:


2020-05-07 14:02:29

by Alexandre Chartre

[permalink] [raw]
Subject: Re: [patch V4 part 2 05/18] x86/entry: Move irq tracing on syscall entry to C-code


On 5/5/20 3:41 PM, Thomas Gleixner wrote:
> Now that the C entry points are safe, move the irq flags tracing code into
> the entry helper:
>
> - Invoke lockdep before calling into context tracking
>
> - Use the safe trace_hardirqs_on_prepare() trace function after context
> tracking established state and RCU is watching.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> arch/x86/entry/common.c | 21 +++++++++++++++++++--
> arch/x86/entry/entry_32.S | 12 ------------
> arch/x86/entry/entry_64.S | 2 --
> arch/x86/entry/entry_64_compat.S | 18 ------------------
> 4 files changed, 19 insertions(+), 34 deletions(-)
>
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -40,19 +40,36 @@
> #include <trace/events/syscalls.h>
>
> #ifdef CONFIG_CONTEXT_TRACKING
> -/* Called on entry from user mode with IRQs off. */
> +/**
> + * enter_from_user_mode - Establish state when coming from user mode
> + *
> + * Syscall entry disables interrupts, but user mode is traced as interrupts
> + * enabled. Also with NO_HZ_FULL RCU might be idle.
> + *
> + * 1) Tell lockdep that interrupts are disabled
> + * 2) Invoke context tracking if enabled to reactivate RCU
> + * 3) Trace interrupts off state
> + */
> __visible noinstr void enter_from_user_mode(void)
> {
> enum ctx_state state = ct_state();
>
> + lockdep_hardirqs_off(CALLER_ADDR0);
> user_exit_irqoff();
>
> instr_begin();
> CT_WARN_ON(state != CONTEXT_USER);
> + trace_hardirqs_off_prepare();
> instr_end();
> }
> #else
> -static inline void enter_from_user_mode(void) {}
> +static __always_inline void enter_from_user_mode(void)
> +{
> + lockdep_hardirqs_off(CALLER_ADDR0);
> + instr_begin();
> + trace_hardirqs_off_prepare();
> + instr_end();
> +}
> #endif
>
> static noinstr void exit_to_user_mode(void)
> --- a/arch/x86/entry/entry_32.S
> +++ b/arch/x86/entry/entry_32.S
> @@ -967,12 +967,6 @@ SYM_FUNC_START(entry_SYSENTER_32)
> jnz .Lsysenter_fix_flags
> .Lsysenter_flags_fixed:
>
> - /*
> - * User mode is traced as though IRQs are on, and SYSENTER
> - * turned them off.
> - */
> - TRACE_IRQS_OFF
> -
> movl %esp, %eax
> call do_fast_syscall_32
> /* XEN PV guests always use IRET path */
> @@ -1082,12 +1076,6 @@ SYM_FUNC_START(entry_INT80_32)
>
> SAVE_ALL pt_regs_ax=$-ENOSYS switch_stacks=1 /* save rest */
>
> - /*
> - * User mode is traced as though IRQs are on, and the interrupt gate
> - * turned them off.
> - */
> - TRACE_IRQS_OFF
> -
> movl %esp, %eax
> call do_int80_syscall_32
> .Lsyscall_32_done:
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -167,8 +167,6 @@ SYM_INNER_LABEL(entry_SYSCALL_64_after_h
>
> PUSH_AND_CLEAR_REGS rax=$-ENOSYS
>
> - TRACE_IRQS_OFF
> -
> /* IRQs are off. */
> movq %rax, %rdi
> movq %rsp, %rsi
> --- a/arch/x86/entry/entry_64_compat.S
> +++ b/arch/x86/entry/entry_64_compat.S
> @@ -129,12 +129,6 @@ SYM_FUNC_START(entry_SYSENTER_compat)
> jnz .Lsysenter_fix_flags
> .Lsysenter_flags_fixed:
>
> - /*
> - * User mode is traced as though IRQs are on, and SYSENTER
> - * turned them off.
> - */
> - TRACE_IRQS_OFF
> -
> movq %rsp, %rdi
> call do_fast_syscall_32
> /* XEN PV guests always use IRET path */
> @@ -247,12 +241,6 @@ SYM_INNER_LABEL(entry_SYSCALL_compat_aft
> pushq $0 /* pt_regs->r15 = 0 */
> xorl %r15d, %r15d /* nospec r15 */
>
> - /*
> - * User mode is traced as though IRQs are on, and SYSENTER
> - * turned them off.
> - */
> - TRACE_IRQS_OFF
> -
> movq %rsp, %rdi
> call do_fast_syscall_32
> /* XEN PV guests always use IRET path */
> @@ -403,12 +391,6 @@ SYM_CODE_START(entry_INT80_compat)
> xorl %r15d, %r15d /* nospec r15 */
> cld
>
> - /*
> - * User mode is traced as though IRQs are on, and the interrupt
> - * gate turned them off.
> - */
> - TRACE_IRQS_OFF
> -
> movq %rsp, %rdi
> call do_int80_syscall_32
> .Lsyscall_32_done:
>

enter_from_user_mode() is also called with the CALL_enter_from_user_mode macro,
which is used in interrupt_entry() and identry. Don't you need to also remove
the TRACE_IRQS_OFF there now?

alex.

2020-05-07 14:13:33

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch V4 part 2 05/18] x86/entry: Move irq tracing on syscall entry to C-code

Alexandre Chartre <[email protected]> writes:
> On 5/5/20 3:41 PM, Thomas Gleixner wrote:
>> - /*
>> - * User mode is traced as though IRQs are on, and the interrupt
>> - * gate turned them off.
>> - */
>> - TRACE_IRQS_OFF
>> -
>> movq %rsp, %rdi
>> call do_int80_syscall_32
>> .Lsyscall_32_done:
>>
>
> enter_from_user_mode() is also called with the CALL_enter_from_user_mode macro,
> which is used in interrupt_entry() and identry. Don't you need to also remove
> the TRACE_IRQS_OFF there now?

Hrm. right. OTOH, it's just redundant and should be no harm, but let me have a
look at that again.

Thanks,

tglx

2020-05-07 15:06:07

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch V4 part 2 05/18] x86/entry: Move irq tracing on syscall entry to C-code

Thomas Gleixner <[email protected]> writes:
> Alexandre Chartre <[email protected]> writes:
>> On 5/5/20 3:41 PM, Thomas Gleixner wrote:
>>> - /*
>>> - * User mode is traced as though IRQs are on, and the interrupt
>>> - * gate turned them off.
>>> - */
>>> - TRACE_IRQS_OFF
>>> -
>>> movq %rsp, %rdi
>>> call do_int80_syscall_32
>>> .Lsyscall_32_done:
>>>
>>
>> enter_from_user_mode() is also called with the CALL_enter_from_user_mode macro,
>> which is used in interrupt_entry() and identry. Don't you need to also remove
>> the TRACE_IRQS_OFF there now?
>
> Hrm. right. OTOH, it's just redundant and should be no harm, but let me have a
> look at that again.

Grr, no. It'll trigger the warnon when context tracking is enabled. /me
scratches head and goes to fix.

2020-05-07 17:08:45

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch V4 part 2 05/18] x86/entry: Move irq tracing on syscall entry to C-code

Thomas Gleixner <[email protected]> writes:
> Thomas Gleixner <[email protected]> writes:
>> Alexandre Chartre <[email protected]> writes:
>>> On 5/5/20 3:41 PM, Thomas Gleixner wrote:
>>>> - /*
>>>> - * User mode is traced as though IRQs are on, and the interrupt
>>>> - * gate turned them off.
>>>> - */
>>>> - TRACE_IRQS_OFF
>>>> -
>>>> movq %rsp, %rdi
>>>> call do_int80_syscall_32
>>>> .Lsyscall_32_done:
>>>>
>>>
>>> enter_from_user_mode() is also called with the CALL_enter_from_user_mode macro,
>>> which is used in interrupt_entry() and identry. Don't you need to also remove
>>> the TRACE_IRQS_OFF there now?
>>
>> Hrm. right. OTOH, it's just redundant and should be no harm, but let me have a
>> look at that again.
>
> Grr, no. It'll trigger the warnon when context tracking is enabled. /me
> scratches head and goes to fix.

Scratch that. After unfrying my brain by walking the dogs for an hour,
it's really just redundant calls into lockdep and tracing and both are
happy about it.

I could do a temporary function for that or just mention it in the
changelog.

Thanks,

tglx

Subject: [tip: x86/entry] x86/entry: Move irq tracing on syscall entry to C-code

The following commit has been merged into the x86/entry branch of tip:

Commit-ID: f0fd87b82db7b0102ba98991fa36c2318d2e2894
Gitweb: https://git.kernel.org/tip/f0fd87b82db7b0102ba98991fa36c2318d2e2894
Author: Thomas Gleixner <[email protected]>
AuthorDate: Tue, 25 Feb 2020 23:08:05 +01:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Tue, 19 May 2020 16:03:51 +02:00

x86/entry: Move irq tracing on syscall entry to C-code

Now that the C entry points are safe, move the irq flags tracing code into
the entry helper:

- Invoke lockdep before calling into context tracking

- Use the safe trace_hardirqs_on_prepare() trace function after context
tracking established state and RCU is watching.

enter_from_user_mode() is also still invoked from the exception/interrupt
entry code which still contains the ASM irq flags tracing. So this is just
a redundant and harmless invocation of tracing / lockdep until these are
removed as well.

Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Alexandre Chartre <[email protected]>
Acked-by: Peter Zijlstra <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]


---
arch/x86/entry/common.c | 21 +++++++++++++++++++--
arch/x86/entry/entry_32.S | 12 ------------
arch/x86/entry/entry_64.S | 2 --
arch/x86/entry/entry_64_compat.S | 18 ------------------
4 files changed, 19 insertions(+), 34 deletions(-)

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 9892fb7..7473c12 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -40,19 +40,36 @@
#include <trace/events/syscalls.h>

#ifdef CONFIG_CONTEXT_TRACKING
-/* Called on entry from user mode with IRQs off. */
+/**
+ * enter_from_user_mode - Establish state when coming from user mode
+ *
+ * Syscall entry disables interrupts, but user mode is traced as interrupts
+ * enabled. Also with NO_HZ_FULL RCU might be idle.
+ *
+ * 1) Tell lockdep that interrupts are disabled
+ * 2) Invoke context tracking if enabled to reactivate RCU
+ * 3) Trace interrupts off state
+ */
__visible noinstr void enter_from_user_mode(void)
{
enum ctx_state state = ct_state();

+ lockdep_hardirqs_off(CALLER_ADDR0);
user_exit_irqoff();

instrumentation_begin();
CT_WARN_ON(state != CONTEXT_USER);
+ trace_hardirqs_off_prepare();
instrumentation_end();
}
#else
-static inline void enter_from_user_mode(void) {}
+static __always_inline void enter_from_user_mode(void)
+{
+ lockdep_hardirqs_off(CALLER_ADDR0);
+ instrumentation_begin();
+ trace_hardirqs_off_prepare();
+ instrumentation_end();
+}
#endif

static noinstr void exit_to_user_mode(void)
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index bf0082b..65704e0 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -967,12 +967,6 @@ SYM_FUNC_START(entry_SYSENTER_32)
jnz .Lsysenter_fix_flags
.Lsysenter_flags_fixed:

- /*
- * User mode is traced as though IRQs are on, and SYSENTER
- * turned them off.
- */
- TRACE_IRQS_OFF
-
movl %esp, %eax
call do_fast_syscall_32
/* XEN PV guests always use IRET path */
@@ -1082,12 +1076,6 @@ SYM_FUNC_START(entry_INT80_32)

SAVE_ALL pt_regs_ax=$-ENOSYS switch_stacks=1 /* save rest */

- /*
- * User mode is traced as though IRQs are on, and the interrupt gate
- * turned them off.
- */
- TRACE_IRQS_OFF
-
movl %esp, %eax
call do_int80_syscall_32
.Lsyscall_32_done:
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index b199f43..9e34fe8 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -167,8 +167,6 @@ SYM_INNER_LABEL(entry_SYSCALL_64_after_hwframe, SYM_L_GLOBAL)

PUSH_AND_CLEAR_REGS rax=$-ENOSYS

- TRACE_IRQS_OFF
-
/* IRQs are off. */
movq %rax, %rdi
movq %rsp, %rsi
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index f1d3cca..e2e8bd7 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -129,12 +129,6 @@ SYM_FUNC_START(entry_SYSENTER_compat)
jnz .Lsysenter_fix_flags
.Lsysenter_flags_fixed:

- /*
- * User mode is traced as though IRQs are on, and SYSENTER
- * turned them off.
- */
- TRACE_IRQS_OFF
-
movq %rsp, %rdi
call do_fast_syscall_32
/* XEN PV guests always use IRET path */
@@ -247,12 +241,6 @@ SYM_INNER_LABEL(entry_SYSCALL_compat_after_hwframe, SYM_L_GLOBAL)
pushq $0 /* pt_regs->r15 = 0 */
xorl %r15d, %r15d /* nospec r15 */

- /*
- * User mode is traced as though IRQs are on, and SYSENTER
- * turned them off.
- */
- TRACE_IRQS_OFF
-
movq %rsp, %rdi
call do_fast_syscall_32
/* XEN PV guests always use IRET path */
@@ -403,12 +391,6 @@ SYM_CODE_START(entry_INT80_compat)
xorl %r15d, %r15d /* nospec r15 */
cld

- /*
- * User mode is traced as though IRQs are on, and the interrupt
- * gate turned them off.
- */
- TRACE_IRQS_OFF
-
movq %rsp, %rdi
call do_int80_syscall_32
.Lsyscall_32_done: