On Tue, Dec 01, 2020 at 07:15:06PM +0100, Peter Zijlstra wrote: > On Tue, Dec 01, 2020 at 03:55:19PM +0100, Peter Zijlstra wrote: > > On Tue, Dec 01, 2020 at 06:46:44AM -0800, Paul E. McKenney wrote: > > > > > > So after having talked to Sven a bit, the thing that is happening, is > > > > that this is the one place where we take interrupts with RCU being > > > > disabled. Normally RCU is watching and all is well, except during idle. > > > > > > Isn't interrupt entry supposed to invoke rcu_irq_enter() at some point? > > > Or did this fall victim to recent optimizations? > > > > It does, but the problem is that s390 is still using > > I might've been too quick there, I can't actually seem to find where > s390 does rcu_irq_enter()/exit(). > > Also, I'm thinking the below might just about solve the current problem. > The next problem would then be it calling TRACE_IRQS_ON after it did > rcu_irq_exit()... :/
I gave this patch a go under QEMU TCG atop v5.10-rc6 s390 defconfig with PROVE_LOCKING and DEBUG_ATOMIC_SLEEP. It significantly reduces the number of lockdep splats, but IIUC we need to handle the io_int_handler path in addition to the ext_int_handler path, and there's a remaining lockdep splat (below). If this ends up looking like we'll need more point-fixes, I wonder if we should conditionalise the new behaviour of the core idle code under a new CONFIG symbol for now, and opt-in x86 and arm64, then transition the rest once they've had a chance to test. They'll still be broken in the mean time, but no more so than they previously were. Thanks, Mark. [ 3.870912] ------------[ cut here ]------------ [ 3.871225] DEBUG_LOCKS_WARN_ON(current->hardirq_chain_key != current->curr_chain_key) [ 3.872025] WARNING: CPU: 1 PID: 0 at kernel/locking/lockdep.c:4155 lockdep_hardirqs_on+0x150/0x158 [ 3.872249] Modules linked in: [ 3.873096] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.10.0-rc6-dirty #9 [ 3.873262] Hardware name: QEMU 2964 QEMU (KVM/Linux) [ 3.873584] Krnl PSW : 0404e00180000000 0000000000ce51ac (lockdep_hardirqs_on+0x154/0x158) [ 3.873967] R:0 T:1 IO:0 EX:0 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3 [ 3.874214] Krnl GPRS: c0000000ffffefff 00000000011e5218 000000000000004a 00000000001d988a [ 3.874363] 00000000ffffffea 000003e000000001 0000000000000000 0000000000000000 [ 3.874511] 0000000000000000 000000000141e5f0 0000000003278000 0000000000cf354a [ 3.874658] 0000000000000000 0000000000000000 0000000000ce51a8 000003e000073db0 [ 3.876258] Krnl Code: 0000000000ce519c: c0200012432c larl %r2,0000000000f2d7f4 [ 3.876258] 0000000000ce51a2: c0e5ffff83db brasl %r14,0000000000cd5958 [ 3.876258] #0000000000ce51a8: af000000 mc 0,0 [ 3.876258] >0000000000ce51ac: a7f4ffb3 brc 15,0000000000ce5112 [ 3.876258] 0000000000ce51b0: ebeff0880024 stmg %r14,%r15,136(%r15) [ 3.876258] 0000000000ce51b6: b90400ef lgr %r14,%r15 [ 3.876258] 0000000000ce51ba: e3f0ffe0ff71 lay %r15,-32(%r15) [ 3.876258] 0000000000ce51c0: e3e0f0980024 stg %r14,152(%r15) [ 3.877823] Call Trace: [ 3.878073] [<0000000000ce51ac>] lockdep_hardirqs_on+0x154/0x158 [ 3.878260] ([<0000000000ce51a8>] lockdep_hardirqs_on+0x150/0x158) [ 3.878406] [<0000000000cf3596>] default_idle_call+0xb6/0x2a8 [ 3.878549] [<0000000000194f60>] do_idle+0xe0/0x190 [ 3.878682] [<00000000001952ae>] cpu_startup_entry+0x36/0x40 [ 3.878816] [<0000000000118152>] smp_start_secondary+0x82/0x88 [ 3.878975] INFO: lockdep is turned off. [ 3.879119] Last Breaking-Event-Address: [ 3.879273] [<0000000000cf76d0>] __s390_indirect_jump_r14+0x0/0xc [ 3.879456] irq event stamp: 104 [ 3.879619] hardirqs last enabled at (102): [<00000000001a540c>] load_balance+0x28c/0x990 [ 3.879783] hardirqs last disabled at (103): [<0000000000cf7626>] __do_softirq+0x536/0x5c8 [ 3.879945] softirqs last enabled at (104): [<0000000000cf7584>] __do_softirq+0x494/0x5c8 [ 3.880114] softirqs last disabled at (95): [<000000000010df60>] do_softirq_own_stack+0x70/0x80 [ 3.880724] random: get_random_bytes called from __warn+0x12a/0x160 with crng_init=0 [ 3.882060] ---[ end trace ffae6482e242107b ]--- > --- > diff --git a/arch/s390/include/asm/irq.h b/arch/s390/include/asm/irq.h > index 9f75d67b8c20..24d3dd482df7 100644 > --- a/arch/s390/include/asm/irq.h > +++ b/arch/s390/include/asm/irq.h > @@ -113,6 +113,8 @@ void irq_subclass_unregister(enum irq_subclass subclass); > > #define irq_canonicalize(irq) (irq) > > +extern __visible void ext_do_IRQ(struct pt_regs *regs, unsigned long vector); > + > #endif /* __ASSEMBLY__ */ > > #endif /* _ASM_IRQ_H */ > diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S > index 26bb0603c5a1..b8e89b685038 100644 > --- a/arch/s390/kernel/entry.S > +++ b/arch/s390/kernel/entry.S > @@ -976,16 +976,10 @@ ENTRY(ext_int_handler) > xc __PT_FLAGS(8,%r11),__PT_FLAGS(%r11) > TSTMSK __LC_CPU_FLAGS,_CIF_IGNORE_IRQ > jo .Lio_restore > -#if IS_ENABLED(CONFIG_TRACE_IRQFLAGS) > - tmhh %r8,0x300 > jz 1f > - TRACE_IRQS_OFF > -1: > -#endif > xc __SF_BACKCHAIN(8,%r15),__SF_BACKCHAIN(%r15) > lgr %r2,%r11 # pass pointer to pt_regs > lghi %r3,EXT_INTERRUPT > - brasl %r14,do_IRQ > + brasl %r14,ext_do_IRQ > j .Lio_return > ENDPROC(ext_int_handler) > > diff --git a/arch/s390/kernel/irq.c b/arch/s390/kernel/irq.c > index 3514420f0259..f4a29114e9fd 100644 > --- a/arch/s390/kernel/irq.c > +++ b/arch/s390/kernel/irq.c > @@ -329,3 +329,23 @@ void irq_subclass_unregister(enum irq_subclass subclass) > spin_unlock(&irq_subclass_lock); > } > EXPORT_SYMBOL(irq_subclass_unregister); > + > +noinstr void ext_do_IRQ(struct pt_regs *regs, unsigned long vector) > +{ > + bool rcu = false; > + > + lockdep_hardirqs_off(CALLER_ADDR0); > + if (!IS_ENABLED(CONFIG_TINY_RCU) && is_idle_task(current)) { > + rcu_irq_enter(); > + rcu = true; > + } > + /* instrumentation_begin(); */ > + > + trace_hardirqs_off_finish(); > + > + do_IRQ(regs, vector); > + > + /* instrumentation_end(); */ > + if (rcu) > + rcu_irq_exit(); > +}