Messages in this thread Patch in this message | | | Date | Wed, 20 May 2020 16:25:50 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [patch V6 12/37] x86/entry: Provide idtentry_entry/exit_cond_rcu() |
| |
On Wed, May 20, 2020 at 03:15:31PM -0700, Paul E. McKenney wrote: > On Wed, May 20, 2020 at 09:49:18PM +0200, Thomas Gleixner wrote: > > "Paul E. McKenney" <paulmck@kernel.org> writes: > > > On Wed, May 20, 2020 at 09:51:17AM -0700, Andy Lutomirski wrote: > > >> Paul, the major change here is that if an IRQ hits normal kernel code > > >> (i.e. code where RCU is watching and we're not in an EQS), the IRQ > > >> won't call rcu_irq_enter() and rcu_irq_exit(). Instead it will call > > >> rcu_tickle() on entry and nothing on exit. Does that cover all the > > >> bases? > > > > > > From an RCU viewpoint, yes, give or take my concerns about someone > > > putting rcu_tickle() on entry and rcu_irq_exit() on exit. Perhaps > > > I can bring some lockdep trickery to bear. > > > > An surplus rcu_irq_exit() should already trigger alarms today. > > Fair point! > > > > But I must defer to Thomas and Peter on the non-RCU/non-NO_HZ_FULL > > > portions of this. > > > > I don't see a problem. Let me write that into actual testable patch > > form. > > Here is the RCU part, with my current best guess for the commit log. > > Please note that this is on top of my -rcu stack, so some adjustment > will likely be needed to pull it underneath Joel's series that removes > the special-purpose bits at the bottom of the ->dynticks counter. > > But a starting point, anyway.
Same patch, but with updated commit log based on IRC discussion with Andy.
Thanx, Paul
------------------------------------------------------------------------
commit 1771ea9fac5748d1424d9214c51b2f79cc1176b6 Author: Paul E. McKenney <paulmck@kernel.org> Date: Wed May 20 15:03:07 2020 -0700
rcu: Abstract out tickle_nohz_for_rcu() from rcu_nmi_enter() There will likely be exception handlers that can sleep, which rules out the usual approach of invoking rcu_nmi_enter() on entry and also rcu_nmi_exit() on all exit paths. However, the alternative approach of just not calling anything can prevent RCU from coaxing quiescent states from nohz_full CPUs that are looping in the kernel: RCU must instead IPI them explicitly. It would be better to enable the scheduler tick on such CPUs to interact with RCU in a lighter-weight manner, and this enabling is one of the things that rcu_nmi_enter() currently does. What is needed is something that helps RCU coax quiescent states while not preventing subsequent sleeps. This commit therefore splits out the nohz_full scheduler-tick enabling from the rest of the rcu_nmi_enter() logic into a new function named tickle_nohz_for_rcu(). Suggested-by: Andy Lutomirski <luto@kernel.org> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
diff --git a/include/linux/hardirq.h b/include/linux/hardirq.h index 621556e..d4be42a 100644 --- a/include/linux/hardirq.h +++ b/include/linux/hardirq.h @@ -14,6 +14,10 @@ extern bool synchronize_hardirq(unsigned int irq); #if defined(CONFIG_TINY_RCU) +static inline void tickle_nohz_for_rcu(void) +{ +} + static inline void rcu_nmi_enter(void) { } @@ -23,6 +27,7 @@ static inline void rcu_nmi_exit(void) } #else +extern void tickle_nohz_for_rcu(void); extern void rcu_nmi_enter(void); extern void rcu_nmi_exit(void); #endif diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index 7812574..0a3cad4 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -806,6 +806,67 @@ void noinstr rcu_user_exit(void) #endif /* CONFIG_NO_HZ_FULL */ /** + * tickle_nohz_for_rcu - Enable scheduler tick on CPU if RCU needs it. + * + * The scheduler tick is not normally enabled when CPUs enter the kernel + * from nohz_full userspace execution. After all, nohz_full userspace + * execution is an RCU quiescent state and the time executing in the kernel + * is quite short. Except of course when it isn't. And it is not hard to + * cause a large system to spend tens of seconds or even minutes looping + * in the kernel, which can cause a number of problems, include RCU CPU + * stall warnings. + * + * Therefore, if a nohz_full CPU fails to report a quiescent state + * in a timely manner, the RCU grace-period kthread sets that CPU's + * ->rcu_urgent_qs flag with the expectation that the next interrupt or + * exception will invoke this function, which will turn on the scheduler + * tick, which will enable RCU to detect that CPU's quiescent states, + * for example, due to cond_resched() calls in CONFIG_PREEMPT=n kernels. + * The tick will be disabled once a quiescent state is reported for + * this CPU. + * + * Of course, in carefully tuned systems, there might never be an + * interrupt or exception. In that case, the RCU grace-period kthread + * will eventually cause one to happen. However, in less carefully + * controlled environments, this function allows RCU to get what it + * needs without creating otherwise useless interruptions. + */ +noinstr void tickle_nohz_for_rcu(void) +{ + struct rcu_data *rdp = this_cpu_ptr(&rcu_data); + + if (in_nmi()) + return; // Enabling tick is unsafe in NMI handlers. + RCU_LOCKDEP_WARN(rcu_dynticks_curr_cpu_in_eqs(), + "Illegal tickle_nohz_for_rcu from extended quiescent state"); + instrumentation_begin(); + if (!tick_nohz_full_cpu(rdp->cpu) || + !READ_ONCE(rdp->rcu_urgent_qs) || + READ_ONCE(rdp->rcu_forced_tick)) { + // RCU doesn't need nohz_full help from this CPU, or it is + // already getting that help. + instrumentation_end(); + return; + } + + // We get here only when not in an extended quiescent state and + // from interrupts (as opposed to NMIs). Therefore, (1) RCU is + // already watching and (2) The fact that we are in an interrupt + // handler and that the rcu_node lock is an irq-disabled lock + // prevents self-deadlock. So we can safely recheck under the lock. + // Note that the nohz_full state currently cannot change. + raw_spin_lock_rcu_node(rdp->mynode); + if (rdp->rcu_urgent_qs && !rdp->rcu_forced_tick) { + // A nohz_full CPU is in the kernel and RCU needs a + // quiescent state. Turn on the tick! + WRITE_ONCE(rdp->rcu_forced_tick, true); + tick_dep_set_cpu(rdp->cpu, TICK_DEP_BIT_RCU); + } + raw_spin_unlock_rcu_node(rdp->mynode); + instrumentation_end(); +} + +/** * rcu_nmi_enter - inform RCU of entry to NMI context * @irq: Is this call from rcu_irq_enter? * @@ -835,7 +896,9 @@ noinstr void rcu_nmi_enter(void) * is if the interrupt arrived in kernel mode; in this case we would * be the outermost interrupt but still increment by 2 which is Ok. */ - if (rcu_dynticks_curr_cpu_in_eqs()) { + if (!rcu_dynticks_curr_cpu_in_eqs()) { + tickle_nohz_for_rcu(); + } else { if (!in_nmi()) rcu_dynticks_task_exit(); @@ -851,28 +914,6 @@ noinstr void rcu_nmi_enter(void) } incby = 1; - } else if (!in_nmi()) { - instrumentation_begin(); - if (tick_nohz_full_cpu(rdp->cpu) && - READ_ONCE(rdp->rcu_urgent_qs) && - !READ_ONCE(rdp->rcu_forced_tick)) { - // We get here only if we had already exited the - // extended quiescent state and this was an - // interrupt (not an NMI). Therefore, (1) RCU is - // already watching and (2) The fact that we are in - // an interrupt handler and that the rcu_node lock - // is an irq-disabled lock prevents self-deadlock. - // So we can safely recheck under the lock. - raw_spin_lock_rcu_node(rdp->mynode); - if (rdp->rcu_urgent_qs && !rdp->rcu_forced_tick) { - // A nohz_full CPU is in the kernel and RCU - // needs a quiescent state. Turn on the tick! - WRITE_ONCE(rdp->rcu_forced_tick, true); - tick_dep_set_cpu(rdp->cpu, TICK_DEP_BIT_RCU); - } - raw_spin_unlock_rcu_node(rdp->mynode); - } - instrumentation_end(); } instrumentation_begin(); trace_rcu_dyntick(incby == 1 ? TPS("End") : TPS("StillNonIdle"),
| |