Because:
irq_enter_rcu() includes lockdep_hardirq_enter()
irq_exit_rcu() does *NOT* include lockdep_hardirq_exit()
Which resulted in two 'stray' lockdep_hardirq_exit() calls in
idtentry.h, and me spending a long time trying to find the matching
enter calls.
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
arch/x86/include/asm/idtentry.h | 2 --
kernel/softirq.c | 19 +++++++++++++------
2 files changed, 13 insertions(+), 8 deletions(-)
--- a/arch/x86/include/asm/idtentry.h
+++ b/arch/x86/include/asm/idtentry.h
@@ -206,7 +206,6 @@ __visible noinstr void func(struct pt_re
kvm_set_cpu_l1tf_flush_l1d(); \
__##func (regs, (u8)error_code); \
irq_exit_rcu(); \
- lockdep_hardirq_exit(); \
instrumentation_end(); \
idtentry_exit_cond_rcu(regs, rcu_exit); \
} \
@@ -249,7 +248,6 @@ __visible noinstr void func(struct pt_re
kvm_set_cpu_l1tf_flush_l1d(); \
run_on_irqstack_cond(__##func, regs, regs); \
irq_exit_rcu(); \
- lockdep_hardirq_exit(); \
instrumentation_end(); \
idtentry_exit_cond_rcu(regs, rcu_exit); \
} \
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -404,12 +404,7 @@ static inline void tick_irq_exit(void)
#endif
}
-/**
- * irq_exit_rcu() - Exit an interrupt context without updating RCU
- *
- * Also processes softirqs if needed and possible.
- */
-void irq_exit_rcu(void)
+static inline void __irq_exit_rcu(void)
{
#ifndef __ARCH_IRQ_EXIT_IRQS_DISABLED
local_irq_disable();
@@ -425,6 +420,18 @@ void irq_exit_rcu(void)
}
/**
+ * irq_exit_rcu() - Exit an interrupt context without updating RCU
+ *
+ * Also processes softirqs if needed and possible.
+ */
+void irq_exit_rcu(void)
+{
+ __irq_exit_rcu();
+ /* must be last! */
+ lockdep_hardirq_exit();
+}
+
+/**
* irq_exit - Exit an interrupt context, update RCU and lockdep
*
* Also processes softirqs if needed and possible.
The following commit has been merged into the x86/entry branch of tip:
Commit-ID: b614345f52bcde8299a53132f5e48a9eb5a1f320
Gitweb: https://git.kernel.org/tip/b614345f52bcde8299a53132f5e48a9eb5a1f320
Author: Peter Zijlstra <[email protected]>
AuthorDate: Fri, 29 May 2020 23:27:39 +02:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Sat, 30 May 2020 10:00:10 +02:00
x86/entry: Clarify irq_{enter,exit}_rcu()
Because:
irq_enter_rcu() includes lockdep_hardirq_enter()
irq_exit_rcu() does *NOT* include lockdep_hardirq_exit()
Which resulted in two 'stray' lockdep_hardirq_exit() calls in
idtentry.h, and me spending a long time trying to find the matching
enter calls.
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/include/asm/idtentry.h | 2 --
kernel/softirq.c | 19 +++++++++++++------
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/idtentry.h b/arch/x86/include/asm/idtentry.h
index d214a30..f8e2737 100644
--- a/arch/x86/include/asm/idtentry.h
+++ b/arch/x86/include/asm/idtentry.h
@@ -206,7 +206,6 @@ __visible noinstr void func(struct pt_regs *regs, \
kvm_set_cpu_l1tf_flush_l1d(); \
__##func (regs, (u8)error_code); \
irq_exit_rcu(); \
- lockdep_hardirq_exit(); \
instrumentation_end(); \
idtentry_exit_cond_rcu(regs, rcu_exit); \
} \
@@ -249,7 +248,6 @@ __visible noinstr void func(struct pt_regs *regs) \
kvm_set_cpu_l1tf_flush_l1d(); \
run_on_irqstack_cond(__##func, regs, regs); \
irq_exit_rcu(); \
- lockdep_hardirq_exit(); \
instrumentation_end(); \
idtentry_exit_cond_rcu(regs, rcu_exit); \
} \
diff --git a/kernel/softirq.c b/kernel/softirq.c
index beb8e3a..a3eb6eb 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -404,12 +404,7 @@ static inline void tick_irq_exit(void)
#endif
}
-/**
- * irq_exit_rcu() - Exit an interrupt context without updating RCU
- *
- * Also processes softirqs if needed and possible.
- */
-void irq_exit_rcu(void)
+static inline void __irq_exit_rcu(void)
{
#ifndef __ARCH_IRQ_EXIT_IRQS_DISABLED
local_irq_disable();
@@ -425,6 +420,18 @@ void irq_exit_rcu(void)
}
/**
+ * irq_exit_rcu() - Exit an interrupt context without updating RCU
+ *
+ * Also processes softirqs if needed and possible.
+ */
+void irq_exit_rcu(void)
+{
+ __irq_exit_rcu();
+ /* must be last! */
+ lockdep_hardirq_exit();
+}
+
+/**
* irq_exit - Exit an interrupt context, update RCU and lockdep
*
* Also processes softirqs if needed and possible.
On Fri, May 29, 2020 at 11:27:39PM +0200, Peter Zijlstra wrote:
> Because:
>
> irq_enter_rcu() includes lockdep_hardirq_enter()
> irq_exit_rcu() does *NOT* include lockdep_hardirq_exit()
>
> Which resulted in two 'stray' lockdep_hardirq_exit() calls in
> idtentry.h, and me spending a long time trying to find the matching
> enter calls.
>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> arch/x86/include/asm/idtentry.h | 2 --
> kernel/softirq.c | 19 +++++++++++++------
> 2 files changed, 13 insertions(+), 8 deletions(-)
>
[]
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -404,12 +404,7 @@ static inline void tick_irq_exit(void)
> #endif
> }
>
> -/**
> - * irq_exit_rcu() - Exit an interrupt context without updating RCU
> - *
> - * Also processes softirqs if needed and possible.
> - */
> -void irq_exit_rcu(void)
> +static inline void __irq_exit_rcu(void)
> {
> #ifndef __ARCH_IRQ_EXIT_IRQS_DISABLED
> local_irq_disable();
> @@ -425,6 +420,18 @@ void irq_exit_rcu(void)
> }
>
> /**
> + * irq_exit_rcu() - Exit an interrupt context without updating RCU
> + *
> + * Also processes softirqs if needed and possible.
> + */
> +void irq_exit_rcu(void)
> +{
> + __irq_exit_rcu();
> + /* must be last! */
> + lockdep_hardirq_exit();
> +}
> +
> +/**
> * irq_exit - Exit an interrupt context, update RCU and lockdep
> *
> * Also processes softirqs if needed and possible.
>
>
Reverted this commit fixed the POWER9 boot warning,
[ 0.005196][ T0] clocksource: timebase: mask: 0xffffffffffffffff max_cycles: 0x761537d007, max_idle_ns: 440795202126 ns
[ 0.012502][ T0] clocksource: timebase mult[1f40000] shift[24] registered
[ 0.030273][ T0] ------------[ cut here ]------------
[ 0.034421][ T0] DEBUG_LOCKS_WARN_ON(current->hardirq_context)
[ 0.034433][ T0] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:3680 lockdep_hardirqs_on_prepare+0x29c/0x2d0
[ 0.045874][ T0] Modules linked in:
[ 0.047977][ T0] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.7.0-next-20200602 #1
[ 0.053187][ T0] NIP: c0000000001d2fec LR: c0000000001d2fe8 CTR: c00000000074b0a0
[ 0.057395][ T0] REGS: c00000000130f810 TRAP: 0700 Not tainted (5.7.0-next-20200602)
[ 0.062614][ T0] MSR: 9000000000021033 <SF,HV,ME,IR,DR,RI,LE> CR: 48000422 XER: 20040000
[ 0.069856][ T0] CFAR: c00000000010e448 IRQMASK: 1
[ 0.069856][ T0] GPR00: c0000000001d2fe8 c00000000130faa0 c00000000130aa00 000000000000002d
[ 0.069856][ T0] GPR04: c00000000133c3b0 000000000000000d 000000006e6f635f 72727563284e4f5f
[ 0.069856][ T0] GPR08: 0000000000000002 c000000000dcf230 0000000000000001 c0000000012b0280
[ 0.069856][ T0] GPR12: 0000000000000000 c0000000057b0000 0000000000000000 0000000000000000
[ 0.069856][ T0] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 0.069856][ T0] GPR20: 0000000000000000 0000000000000001 0000000010004d9c 00000000100053ed
[ 0.069856][ T0] GPR24: 0000000010005411 0000000000000001 0000000000000002 0000000000000003
[ 0.069856][ T0] GPR28: 0000000000000000 0000000000000000 0000000000000000 c000000003e3b008
[ 0.117846][ T0] NIP [c0000000001d2fec] lockdep_hardirqs_on_prepare+0x29c/0x2d0
[ 0.123052][ T0] LR [c0000000001d2fe8] lockdep_hardirqs_on_prepare+0x298/0x2d0
[ 0.127248][ T0] Call Trace:
[ 0.129337][ T0] [c00000000130faa0] [c0000000001d2fe8] lockdep_hardirqs_on_prepare+0x298/0x2d0 (unreliable)
[ 0.137613][ T0] [c00000000130fb10] [c0000000002d3834] trace_hardirqs_on+0x94/0x230
trace_hardirqs_on at kernel/trace/trace_preemptirq.c:49
[ 0.141824][ T0] [c00000000130fb60] [c000000000039100] interrupt_exit_kernel_prepare+0x110/0x1f0
interrupt_exit_kernel_prepare at arch/powerpc/kernel/syscall_64.c:337
[ 0.148069][ T0] [c00000000130fbc0] [c00000000000f328] interrupt_return+0x118/0x1c0
[ 0.152281][ T0] --- interrupt: 900 at arch_local_irq_restore+0xc0/0xd0
arch_local_irq_restore at arch/powerpc/kernel/irq.c:367
(inlined by) arch_local_irq_restore at arch/powerpc/kernel/irq.c:318
[ 0.152281][ T0] LR = start_kernel+0x7f0/0x9dc
[ 0.153579][ T0] [c00000000130fec0] [c000000001208fa8] init_on_free+0x0/0x2b0 (unreliable)
[ 0.159810][ T0] [c00000000130fee0] [c000000000c845c8] start_kernel+0x7e4/0x9dc
start_kernel at init/main.c:961 (discriminator 3)
[ 0.165017][ T0] [c00000000130ff90] [c00000000000c890] start_here_common+0x1c/0x8c
[ 0.169224][ T0] Instruction dump:
[ 0.171324][ T0] 0fe00000 e8010080 ebc10060 ebe10068 7c0803a6 4bfffe7c 3c82ff8b 3c62ff8a
[ 0.177558][ T0] 38848808 3863e460 4bf3b3fd 60000000 <0fe00000> e8010080 ebc10060 ebe10068
[ 0.183796][ T0] irq event stamp: 16
[ 0.186904][ T0] hardirqs last enabled at (14): [<c00000000020cf14>] rcu_core+0x9a4/0xbe0
[ 0.191130][ T0] hardirqs last disabled at (15): [<c000000000a39944>] __do_softirq+0x5d4/0x8d8
[ 0.195365][ T0] softirqs last enabled at (16): [<c000000000a399c8>] __do_softirq+0x658/0x8d8
[ 0.201606][ T0] softirqs last disabled at (5): [<c00000000011cbbc>] irq_exit+0x17c/0x1c0
[ 0.206832][ T0] ---[ end trace 339d75c2056bfda1 ]---
[ 0.208990][ T0] printk: console [hvc0] enabled
[ 0.208990][ T0] printk: console [hvc0] enabled
On Tue, Jun 02, 2020 at 10:42:35AM -0400, Qian Cai wrote:
> Reverted this commit fixed the POWER9 boot warning,
ARGH, I'm an idiot. Please try this instead:
diff --git a/kernel/softirq.c b/kernel/softirq.c
index a3eb6eba8c41..c4201b7f42b1 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -438,7 +438,7 @@ void irq_exit_rcu(void)
*/
void irq_exit(void)
{
- irq_exit_rcu();
+ __irq_exit_rcu();
rcu_irq_exit();
/* must be last! */
lockdep_hardirq_exit();
On Tue, Jun 02, 2020 at 05:05:11PM +0200, Peter Zijlstra wrote:
> On Tue, Jun 02, 2020 at 10:42:35AM -0400, Qian Cai wrote:
>
> > Reverted this commit fixed the POWER9 boot warning,
>
> ARGH, I'm an idiot. Please try this instead:
>
>
> diff --git a/kernel/softirq.c b/kernel/softirq.c
> index a3eb6eba8c41..c4201b7f42b1 100644
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -438,7 +438,7 @@ void irq_exit_rcu(void)
> */
> void irq_exit(void)
> {
> - irq_exit_rcu();
> + __irq_exit_rcu();
> rcu_irq_exit();
> /* must be last! */
> lockdep_hardirq_exit();
This works fine.
The following commit has been merged into the x86/entry branch of tip:
Commit-ID: 10396895ab36357e676b894d89f64667ce226150
Gitweb: https://git.kernel.org/tip/10396895ab36357e676b894d89f64667ce226150
Author: Peter Zijlstra <[email protected]>
AuthorDate: Wed, 03 Jun 2020 13:40:15 +02:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Wed, 03 Jun 2020 16:35:36 +02:00
x86/entry: Use __irq_exit_rcu() in irq_exit()
Because if you rename a function, you should also rename the users.
Fixes: b614345f52bc ("x86/entry: Clarify irq_{enter,exit}_rcu()")
Reported-by: Qian Cai <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Qian Cai <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
Link: https://lkml.kernel.org/r/[email protected]
---
kernel/softirq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/softirq.c b/kernel/softirq.c
index a3eb6eb..c4201b7 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -438,7 +438,7 @@ void irq_exit_rcu(void)
*/
void irq_exit(void)
{
- irq_exit_rcu();
+ __irq_exit_rcu();
rcu_irq_exit();
/* must be last! */
lockdep_hardirq_exit();