Message-ID: <20200526081456.GA35238@gmail.com>
Date:   Tue, 26 May 2020 10:14:56 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     "Paul E. McKenney" <paulmck@...nel.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Andrew Cooper <andrew.cooper3@...rix.com>,
        X86 ML <x86@...nel.org>,
        Alexandre Chartre <alexandre.chartre@...cle.com>,
        Frederic Weisbecker <frederic@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Petr Mladek <pmladek@...e.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Joel Fernandes <joel@...lfernandes.org>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Juergen Gross <jgross@...e.com>,
        Brian Gerst <brgerst@...il.com>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Will Deacon <will@...nel.org>,
        Tom Lendacky <thomas.lendacky@....com>,
        Wei Liu <wei.liu@...nel.org>,
        Michael Kelley <mikelley@...rosoft.com>,
        Jason Chen CJ <jason.cj.chen@...el.com>,
        Zhao Yakui <yakui.zhao@...el.com>,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>
Subject: Re: [patch V9 02/39] rcu: Abstract out rcu_irq_enter_check_tick()
 from rcu_nmi_enter()


* Paul E. McKenney <paulmck@...nel.org> wrote:

> > +	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.
> > +		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);

BTW., can we please not ever use this weird comment style in the future?

Linus gave an exception to single-line C++ style comments - but I 
don't think that should be extrapolated to a license to uglify the 
kernel with inconsistent muck like this. :-/

I've sanitized it via the patch below.

( I also fixed the whitespace damage and a capitalization typo while 
  at it, and fixed the spelling in the big comment explaining 
  __rcu_irq_enter_check_tick(). )

Thanks,

	Ingo

--- tip.orig/kernel/rcu/tree.c
+++ tip/kernel/rcu/tree.c
@@ -850,14 +850,14 @@ void noinstr rcu_user_exit(void)
 }
 
 /**
- * __rcu_irq_enter_check_tick - Enable scheduler tick on CPU if RCU needs it.
+ * __rcu_irq_enter_check_tick - Enable the scheduler tick on a 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
+ * is quite short.  Except of course when it isn't: 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
+ * in the kernel, which can cause a number of problems, including RCU CPU
  * stall warnings.
  *
  * Therefore, if a nohz_full CPU fails to report a quiescent state
@@ -879,7 +879,7 @@ void __rcu_irq_enter_check_tick(void)
 {
 	struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
 
-	 // Enabling the tick is unsafe in NMI handlers.
+	/* Enabling the tick is unsafe in NMI handlers. */
 	if (WARN_ON_ONCE(in_nmi()))
 		return;
 
@@ -889,21 +889,27 @@ void __rcu_irq_enter_check_tick(void)
 	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.
+		/*
+		 * RCU doesn't need nohz_full help from this CPU, or it is
+		 * already getting that help.
+		 */
 		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.
+	/*
+	 * 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!
+		/*
+		 * 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);
 	}

Powered by blists - more mailing lists