Re: Signed-off-by for the fix-late-interrupts patch?

[Paul E. McKenney]

On Tue, Jul 02, 2019 at 10:36:32AM +0200, Peter Zijlstra wrote:
> On Mon, Jul 01, 2019 at 02:39:31PM -0700, Paul E. McKenney wrote:
> > Hello, Peter,
> > 
> > The patch below from your earlier email is doing fine in my testing.
> > May I please add your Signed-of-by and designate you as author?
> 
> Sure, glad it worked :-)

And here is the updated version, fixing a few typos along the way.
If I don't hear otherwise from you, I will push it out via -rcu, but
of course feel free to direct it elsewhere if you prefer.

							Thanx, Paul

------------------------------------------------------------------------

commit 559712d50c90fbaec1a6386d720deb323bb6468e
Author: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Date:   Wed Jun 5 07:46:43 2019 -0700

    idle: Prevent late-arriving interrupts from disrupting offline
    
    Scheduling-clock interrupts can arrive late in the CPU-offline process,
    after idle entry and the subsequent call to cpuhp_report_idle_dead().
    Once execution passes the call to rcu_report_dead(), RCU is ignoring
    the CPU, which results in lockdep complaints when the interrupt handler
    uses RCU:
    
    ------------------------------------------------------------------------
    
    =============================
    WARNING: suspicious RCU usage
    5.2.0-rc1+ #681 Not tainted
    -----------------------------
    kernel/sched/fair.c:9542 suspicious rcu_dereference_check() usage!
    
    other info that might help us debug this:
    
    RCU used illegally from offline CPU!
    rcu_scheduler_active = 2, debug_locks = 1
    no locks held by swapper/5/0.
    
    stack backtrace:
    CPU: 5 PID: 0 Comm: swapper/5 Not tainted 5.2.0-rc1+ #681
    Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS Bochs 01/01/2011
    Call Trace:
     <IRQ>
     dump_stack+0x5e/0x8b
     trigger_load_balance+0xa8/0x390
     ? tick_sched_do_timer+0x60/0x60
     update_process_times+0x3b/0x50
     tick_sched_handle+0x2f/0x40
     tick_sched_timer+0x32/0x70
     __hrtimer_run_queues+0xd3/0x3b0
     hrtimer_interrupt+0x11d/0x270
     ? sched_clock_local+0xc/0x74
     smp_apic_timer_interrupt+0x79/0x200
     apic_timer_interrupt+0xf/0x20
     </IRQ>
    RIP: 0010:delay_tsc+0x22/0x50
    Code: ff 0f 1f 80 00 00 00 00 65 44 8b 05 18 a7 11 48 0f ae e8 0f 31 48 89 d6 48 c1 e6 20 48 09 c6 eb 0e f3 90 65 8b 05 fe a6 11 48 <41> 39 c0 75 18 0f ae e8 0f 31 48 c1 e2 20 48 09 c2 48 89 d0 48 29
    RSP: 0000:ffff8f92c0157ed0 EFLAGS: 00000212 ORIG_RAX: ffffffffffffff13
    RAX: 0000000000000005 RBX: ffff8c861f356400 RCX: ffff8f92c0157e64
    RDX: 000000321214c8cc RSI: 00000032120daa7f RDI: 0000000000260f15
    RBP: 0000000000000005 R08: 0000000000000005 R09: 0000000000000000
    R10: 0000000000000001 R11: 0000000000000001 R12: 0000000000000000
    R13: 0000000000000000 R14: ffff8c861ee18000 R15: ffff8c861ee18000
     cpuhp_report_idle_dead+0x31/0x60
     do_idle+0x1d5/0x200
     ? _raw_spin_unlock_irqrestore+0x2d/0x40
     cpu_startup_entry+0x14/0x20
     start_secondary+0x151/0x170
     secondary_startup_64+0xa4/0xb0
    
    ------------------------------------------------------------------------
    
    This happens rarely, but can be forced by happen more often by
    placing delays in cpuhp_report_idle_dead() following the call to
    rcu_report_dead().  With this in place, the following rcutorture
    scenario reproduces the problem within a few minutes:
    
    tools/testing/selftests/rcutorture/bin/kvm.sh --cpus 8 --duration 5 --kconfig "CONFIG_DEBUG_LOCK_ALLOC=y CONFIG_PROVE_LOCKING=y" --configs "TREE04"
    
    This commit uses the crude but effective expedient of moving the disabling
    of interrupts within the idle loop to precede the cpu_is_offline()
    check.  It also invokes tick_nohz_idle_stop_tick() instead of
    tick_nohz_idle_stop_tick_protected() to shut off the scheduling-clock
    interrupt, and then removes the latter because there are no other callers.
    
    Signed-off-by: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
    Cc: Frederic Weisbecker <fweisbec@xxxxxxxxx>
    Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
    Cc: Ingo Molnar <mingo@xxxxxxxxxx>
    Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxx>

diff --git a/include/linux/tick.h b/include/linux/tick.h
index f92a10b5e112..196a0a7bfc4f 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -134,14 +134,6 @@ extern unsigned long tick_nohz_get_idle_calls(void);
 extern unsigned long tick_nohz_get_idle_calls_cpu(int cpu);
 extern u64 get_cpu_idle_time_us(int cpu, u64 *last_update_time);
 extern u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time);
-
-static inline void tick_nohz_idle_stop_tick_protected(void)
-{
-	local_irq_disable();
-	tick_nohz_idle_stop_tick();
-	local_irq_enable();
-}
-
 #else /* !CONFIG_NO_HZ_COMMON */
 #define tick_nohz_enabled (0)
 static inline int tick_nohz_tick_stopped(void) { return 0; }
@@ -164,8 +156,6 @@ static inline ktime_t tick_nohz_get_sleep_length(ktime_t *delta_next)
 }
 static inline u64 get_cpu_idle_time_us(int cpu, u64 *unused) { return -1; }
 static inline u64 get_cpu_iowait_time_us(int cpu, u64 *unused) { return -1; }
-
-static inline void tick_nohz_idle_stop_tick_protected(void) { }
 #endif /* !CONFIG_NO_HZ_COMMON */
 
 #ifdef CONFIG_NO_HZ_FULL
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index f5516bae0c1b..42270ca90d94 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -240,13 +240,14 @@ static void do_idle(void)
 		check_pgt_cache();
 		rmb();
 
+		local_irq_disable();
+
 		if (cpu_is_offline(cpu)) {
-			tick_nohz_idle_stop_tick_protected();
+			tick_nohz_idle_stop_tick();
 			cpuhp_report_idle_dead();
 			arch_cpu_idle_dead();
 		}
 
-		local_irq_disable();
 		arch_cpu_idle_enter();
 
 		/*