| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20200526153413.GG2869@paulmck-ThinkPad-P72>
Date: Tue, 26 May 2020 08:34:13 -0700
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Ingo Molnar <mingo@...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()
On Tue, May 26, 2020 at 10:14:56AM +0200, Ingo Molnar wrote:
>
> * 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.
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);
> }
Powered by blists - more mailing lists