Messages in this thread Patch in this message | | | Date | Tue, 26 May 2020 08:34:13 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [patch V9 02/39] rcu: Abstract out rcu_irq_enter_check_tick() from rcu_nmi_enter() |
| |
On Tue, May 26, 2020 at 10:14:56AM +0200, Ingo Molnar wrote: > > * Paul E. McKenney <paulmck@kernel.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.
The "//" comment style does save vertical space. Is it really ugly or just unfamiliar? For purposes of comparison, back in the day, the "/* */" style seemed quite strange compared to my earlier languages' commenting styles.
> ( 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(). )
Some were stylistic rather than wrong, but I have no objection to any of these changes.
Thanx, Paul
> 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); > }
| |