Messages in this thread Patch in this message | | | Date | Wed, 19 Apr 2017 07:47:30 -0700 | From | "Paul E. McKenney" <> | 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@linux.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@linux.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@include/asm-@) { CHK("ARCH_DEFINES",
| |