All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.ibm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: Signed-off-by for the fix-late-interrupts patch?
Date: Tue, 2 Jul 2019 09:49:24 -0700	[thread overview]
Message-ID: <20190702164924.GX26519@linux.ibm.com> (raw)
In-Reply-To: <20190702083632.GZ3419@hirez.programming.kicks-ass.net>

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@infradead.org>
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@infradead.org>
    Cc: Frederic Weisbecker <fweisbec@gmail.com>
    Cc: Thomas Gleixner <tglx@linutronix.de>
    Cc: Ingo Molnar <mingo@kernel.org>
    Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>

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();
 
 		/*

      reply	other threads:[~2019-07-02 16:49 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-01 21:39 Signed-off-by for the fix-late-interrupts patch? Paul E. McKenney
2019-07-02  8:36 ` Peter Zijlstra
2019-07-02 16:49   ` Paul E. McKenney [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190702164924.GX26519@linux.ibm.com \
    --to=paulmck@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.