linux-kernel - Re: [PATCH v2 tip/core/rcu 0/13] Miscellaneous fixes for 4.12

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 19 Apr 2017 07:47:30 -0700
From:   "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Christian Borntraeger <borntraeger@...ibm.com>,
        linux-kernel@...r.kernel.org, mingo@...nel.org,
        jiangshanlai@...il.com, dipankar@...ibm.com,
        akpm@...ux-foundation.org, mathieu.desnoyers@...icios.com,
        josh@...htriplett.org, tglx@...utronix.de, rostedt@...dmis.org,
        dhowells@...hat.com, edumazet@...gle.com, dvhart@...ux.intel.com,
        fweisbec@...il.com, oleg@...hat.com, bobby.prani@...il.com,
        marc.zyngier@....com
Subject: Re: [PATCH v2 tip/core/rcu 0/13] Miscellaneous fixes for 4.12

On Wed, Apr 19, 2017 at 02:08:47PM +0200, Peter Zijlstra wrote:
> On Wed, Apr 19, 2017 at 01:48:08PM +0200, Christian Borntraeger wrote:
> > On 04/19/2017 01:28 PM, Peter Zijlstra wrote:
> > > 
> > > So the thing Maz complained about is because KVM assumes
> > > synchronize_srcu() is 'free' when there is no srcu_read_lock() activity.
> > > This series 'breaks' that.
> > 
> > Why is such a behaviour change not mentioned in the cover letter?
> > I could not find anything in the patch descriptions that would
> > indicate a slowdown. How much slower did it get?
> > 
> > But indeed, there are several places at KVM startup which have been
> > reworked to srcu since normal rcu was too slow for several usecases.
> > (Mostly registering devices and related data structures at startup,
> > basically the qemu/kvm coldplug interaction)
> 
> I suspect Paul is not considering this a 'normal' RCU feature, and
> therefore didn't think about changing this.
> 
> I know I was fairly surprised by this requirement when I ran into it;
> and only accidentally remembered it now that maz complained.

Indeed -- the natural thing to have done back when KVM's scalability was
first being worked on would have been to simply change synchronize_rcu()
to synchronize_rcu_expedited().  However, at that time, these things
did try_stop_cpus() and the like, which was really bad for latency.
Moving to SRCU avoided this problem.  Of course, now that KVM uses
SRCU, why change unless there is a problem?  Besides, I vaguely recall
some KVM cases where srcu_read_lock() is used from CPUs that look to
be idle or offline from RCU's perspective, and that sort of thing only
works for SRCU.

Which reminds me...

The RCU expedited primitives have been completely rewritten since then,
and no longer use try_stop_cpus(), no longer disturb idle CPUs, and no
longer disturb nohz_full CPUs running in userspace.  In addition, there
is the rcupdate.rcu_normal kernel boot paramter for those who want to
completely avoid RCU expedited primitives.

So it seems to me to be time for the patch below.  Thoughts?

							Thanx, Paul

------------------------------------------------------------------------

commit 333d383fad42b4bdef3d27d91e940a6eafed3f91
Author: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
Date:   Wed Apr 19 07:37:45 2017 -0700

    checkpatch: Remove checks for expedited grace periods
    
    There was a time when the expedited grace-period primitives
    (synchronize_rcu_expedited(), synchronize_rcu_bh_expedited(), and
    synchronize_sched_expedited()) used rather antisocial kernel
    facilities like try_stop_cpus().  However, they have since been
    housebroken to use only single-CPU IPIs, and typically cause less
    disturbance than a scheduling-clock interrupt.  Furthermore, this
    disturbance can be eliminated entirely using NO_HZ_FULL on the
    one hand or the rcupdate.rcu_normal boot parameter on the other.
    
    This commit therefore removes checkpatch's complaints about use
    of the expedited RCU primitives.
    
    Signed-off-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index baa3c7be04ad..64bf2a091368 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5511,23 +5511,6 @@ sub process {
 			}
 		}
 
-# Check for expedited grace periods that interrupt non-idle non-nohz
-# online CPUs.  These expedited can therefore degrade real-time response
-# if used carelessly, and should be avoided where not absolutely
-# needed.  It is always OK to use synchronize_rcu_expedited() and
-# synchronize_sched_expedited() at boot time (before real-time applications
-# start) and in error situations where real-time response is compromised in
-# any case.  Note that synchronize_srcu_expedited() does -not- interrupt
-# other CPUs, so don't warn on uses of synchronize_srcu_expedited().
-# Of course, nothing comes for free, and srcu_read_lock() and
-# srcu_read_unlock() do contain full memory barriers in payment for
-# synchronize_srcu_expedited() non-interruption properties.
-		if ($line =~ /\b(synchronize_rcu_expedited|synchronize_sched_expedited)\(/) {
-			WARN("EXPEDITED_RCU_GRACE_PERIOD",
-			     "expedited RCU grace periods should be avoided where they can degrade real-time response\n" . $herecurr);
-
-		}
-
 # check of hardware specific defines
 		if ($line =~ m@^.\s*\#\s*if.*\b(__i386__|__powerpc64__|__sun__|__s390x__)\b@ && $realfile !~ m@...lude/asm-@) {
 			CHK("ARCH_DEFINES",