Re: [GIT PULL rcu/next] RCU commits for 4.13

From: Andrea Parri
Date: Wed Jun 14 2017 - 10:33:41 EST

On Tue, Jun 13, 2017 at 09:33:17PM -0700, Paul E. McKenney wrote:
> On Wed, Jun 14, 2017 at 04:54:04AM +0200, Andrea Parri wrote:
> > On Mon, Jun 12, 2017 at 02:37:55PM -0700, Paul E. McKenney wrote:
> > > Hello, Ingo,
> > >
> > > This pull request is unusual in being a single linear set of commits,
> > > as opposed to my usual topic branches. This is due to the many
> > > large-footprint changes, which means that reasonable topic branches
> > > result in large numbers of merge conflicts. In addition, some commits
> > > depend on other commits that should be on different topic branches.
> > > I will return to the topic-branch style next time.
> > >
> > > The largest feature of this series is shrinking and simplification,
> > > with the following diffstat summary:
> > >
> > > 79 files changed, 1496 insertions(+), 4211 deletions(-)
> > >
> > > In other words, this series represents a net reduction of more than 2700
> > > lines of code.
> > >
> > > These commits were posted to LKML:
> > >
> > > http://lkml.kernel.org/r/20170525215934.GA11578@xxxxxxxxxxxxxxxxxx
> >
> > I did raise some issues (AFAICT, unresolved) concerning...
> >
> >
> > >
> > > Two of these commits (46/88 and 48/88) have been deferred, most likely
> > > to v4.14. All of the remaining commits have been subjected to the 0day
> > > Test Robot and -next testing, and are availiable in teh git repository at:
> > >
> > > git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git for-mingo
> > >
> > > for you to fetch changes up to 6d48152eafde1f0d0a4a9e0584fa7d9ff4fbfdac:
> > >
> > > rcu: Remove RCU CPU stall warnings from Tiny RCU (2017-06-08 18:52:45 -0700)
> > >
> > > ----------------------------------------------------------------
> > > Arnd Bergmann (1):
> > > bcm47xx: Fix build regression
> > >
> > > Paul E. McKenney (83):
> > > rcutorture: Add lockdep to one of the SRCU scenarios
> > > rcutorture: Add three-level tree test for Tree SRCU
> > > rcutorture: Fix bug in reporting Kconfig mis-settings
> > > rcutorture: Add a scenario for Tiny SRCU
> > > rcutorture: Add a scenario for Classic SRCU
> > > rcu: Prevent rcu_barrier() from starting needless grace periods
> > > rcutorture: Correctly handle CONFIG_RCU_TORTURE_TEST_* options
> > > rcutorture: Update test scenarios based on new Kconfig dependencies
> > > srcu: Eliminate possibility of destructive counter overflow
> > > rcu: Complain if blocking in preemptible RCU read-side critical section
> > > rcuperf: Defer expedited/normal check to end of test
> > > rcuperf: Remove conflicting Kconfig options
> > > rcu: Remove obsolete reference to synchronize_kernel()
> > > rcuperf: Add ability to performance-test call_rcu() and friends
> > > rcuperf: Add a Kconfig-fragment file for Classic SRCU
> > > rcu: Make sync_rcu_preempt_exp_done() return bool
> > > checkpatch: Remove checks for expedited grace periods
> > > rcuperf: Add test for dynamically initialized srcu_struct
> > > doc/atomic_ops: Clarify smp_mb__{before,after}_atomic()
> > > atomics: Add header comment so spin_unlock_wait()
> >
> > ... this one: c.f.,
> >
> > http://www.mail-archive.com/linux-kernel@xxxxxxxxxxxxxxx/msg1418503.html
> >
> > Any hints about those?
>
> I suggest being -extremely- clear. This is about ARM, correct?
> If so, lay out the exact situation (code example, which hardware,
> which spin_unlock_wait(), which sequence of events) that could lead to
> the failure.
>
> The problem here is that no one knows which of the 30 CPU families you
> might be talking about, nor do they know exactly what the problem is.
> I didn't worry about it at the time because I figured that you had
> sent private email to Will with the full story.
>
> Yes, the four of us (you, Alan, Luc, and me) discussed it, but we weren't
> sure whether it was a bug in the memory model, the spin_unlock_wait()
> code, or my description of spin_unlock_wait(). Given that Will didn't
> object to my April 13th email (the one that you were not CCed on),
> I figured that he wasn't going to claim that the spin_unlock_wait()
> description was wrong, especially since he went to so much effort some
> years back to make ARM64 meet that description.
>
> So again, I recommend replying to your msg1418503.html email with
> a code fragment demonstrating the problem, exact identification of
> the hardware that might be susceptible (ARM64? ARM32? Which ARM32?),
> exact identification of which spin_unlock_wait() function you suspect,
> and a clear bullet-form sequence of events that shows how you believe
> that the problem can occur.
>
> That makes it easy for people to see what your concern is, easy for
> them to check their code and hardware, and hard for them to ignore you.
>
> Make sense?

My concerns originates from the fact that none of the implementations
(of spin_unlock_wait()) for the architectures touched by:

726328d92a42b6d4b76078e2659f43067f82c4e8
("locking/spinlock, arch: Update and fix spin_unlock_wait() implementations"

currently contain any traces of that RELEASE/spin_unlock() from your:

"Semantically this is equivalent to a spin_lock() immediately followed
by a spin_unlock()."

In fact, the header of that commit states:

"The update is in semantics; where it previously was only a control
dependency, we now upgrade to a full load-acquire [...]"

For an example leveraging this RELEASE, consider:

[initially: X = 0, s UNLOCKED]

P0 P1
X = 1; spin_lock(s);
spin_unlock_wait(s); r0 = X;

According to the "spin_lock(); spin_unlock() semantics" this has one
non-deadlocking execution, and the RELEASE from the spin_unlock_wait()
(paired with the ACQUIRE from the spin_lock() in P1) guarantees that
r0 = 1 in this execution. AFAICT, this same conclusion does not hold
according to the "smp_cond_load_acquire() semantics" (726328d92a42b).

Andrea


>
> Thanx, Paul
>
> > Andrea
> >
> > > rcuperf: Add the ability to test tiny RCU flavors
> > > srcu: Make Classic and Tree SRCU announce themselves at bootup
> > > rcutorture: Reduce CPUs dedicated to testing Classic SRCU
> > > srcu: Shrink Tiny SRCU a bit more
> > > rcuperf: Set more user-friendly defaults
> > > rcuperf: Add writer_holdoff boot parameter
> > > rcutorture: Add "git diff" output to testid.txt file
> > > srcu: Document auto-expediting requirement
> > > doc: Take tail recursion into account in RCU requirements
> > > rcu: Add preemptibility checks in rcu_sched_qs() and rcu_bh_qs()
> > > rcu: Print out rcupdate.c non-default boot-time settings
> > > rcu: Update rcu_bootup_announce_oddness()
> > > srcu: Make exp_holdoff module parameter be static
> > > srcu: Print non-default exp_holdoff values at boot time
> > > rcu: Add lockdep_assert_held() teeth to tree.c
> > > rcu: Add lockdep_assert_held() teeth to tree_plugin.h
> > > srcu: Make SRCU be once again optional
> > > srcu: Shrink Tiny SRCU a bit
> > > srcu: Add DEBUG_OBJECTS_RCU_HEAD functionality
> > > rcu: Make synchronize_rcu_mult() check for duplicates
> > > sched: Rely on synchronize_rcu_mult() de-duplication
> > > rcu: Use RCU_NOCB_WAKE rather than RCU_NOGP_WAKE
> > > rcu: Add memory barriers for NOCB leader wakeup
> > > rcu: Flag need for rcu_node_tree.h and rcu_segcblist.h visibility
> > > rcu: Move docbook comments out of rcupdate.h
> > > rcu: Move rcu_expedited and rcu_normal externs from rcupdate.h
> > > rcu: Move expediting-related access/control out of rcupdate.h
> > > rcu: Move torture-related definitions from rcupdate.h to rcu.h
> > > rcu: Remove UINT_CMP_GE() and UINT_CMP_LT()
> > > rcu: Move rcupdate.h to new empty-function style
> > > rcu: Eliminate the unused __rcu_is_watching() function
> > > rcu: Move the RCU_SCHEDULER_ definitions from rcupdate.h
> > > rcu: Remove linux/debugobjects.h from rcupdate.h
> > > rcu: Improve __call_rcu() debug-objects error message
> > > rcu: Move rcu_is_nocb_cpu() from rcupdate.h to rcu.h
> > > rcu: Move rcu_ftrace_dump() from rcupdate.h to rcu.h
> > > rcu: move rcupdate.h to the new true/false-function style
> > > rcu: Move torture-related functions out of rcutiny.h and rcutree.h
> > > rcu: Move rcu_request_urgent_qs_task() out of rcutiny.h and rcutree.h
> > > rcu: Move rcutiny.h to new empty/true/false-function style
> > > srcu: Prevent sdp->srcu_gp_seq_needed counter wrap
> > > srcu: Shrink srcu.h by moving docbook and private function
> > > srcu: Apply trivial callback lists to shrink Tiny SRCU
> > > lockdep: Use consistent printing primitives
> > > rcu: Refactor #includes from include/linux/rcupdate.h
> > > rcu: Convert rnp->lock wrappers to macros for SRCU use
> > > rcu: Move rnp->lock wrappers for SRCU use
> > > srcu: Use rnp->lock wrappers to replace explicit memory barriers
> > > rcu: Remove *_SLOW_* Kconfig options
> > > rcu: Remove the RCU_KTHREAD_PRIO Kconfig option
> > > rcu: Remove nohz_full full-system-idle state machine
> > > rcu: Remove #ifdef moving rcu_end_inkernel_boot from rcupdate.h
> > > rcu: Remove typecheck() from RCU locking wrapper functions
> > > rcu: Remove the now-obsolete PROVE_RCU_REPEATEDLY Kconfig option
> > > rcu: Remove SPARSE_RCU_POINTER Kconfig option
> > > srcu: Fix rcutorture-statistics typo
> > > srcu: Remove Classic SRCU
> > > rcu: Remove debugfs tracing
> > > rcu: Eliminate NOCBs CPU-state Kconfig options
> > > rcu: Move RCU non-debug Kconfig options to kernel/rcu
> > > rcu: Move RCU debug Kconfig options to kernel/rcu
> > > rcu: Remove event tracing from Tiny RCU
> > > rcu: Remove RCU CPU stall warnings from Tiny RCU
> > >
> > > Priyalee Kushwaha (1):
> > > srcu-cbmc: Use /usr/bin/awk instead of /bin/awk
> > >
> > > Stan Drozd (1):
> > > docs: Fix typo in Documentation/memory-barriers.txt
> > >
> > > Documentation/RCU/00-INDEX | 2 -
> > > .../RCU/Design/Requirements/Requirements.html | 34 +-
> > > Documentation/RCU/checklist.txt | 8 +-
> > > Documentation/RCU/trace.txt | 535 -----------------
> > > Documentation/admin-guide/kernel-parameters.txt | 41 +-
> > > Documentation/core-api/atomic_ops.rst | 5 +
> > > Documentation/dev-tools/sparse.rst | 6 -
> > > Documentation/kernel-per-CPU-kthreads.txt | 31 +-
> > > Documentation/memory-barriers.txt | 2 +-
> > > Documentation/timers/NO_HZ.txt | 29 +-
> > > include/linux/bcm47xx_nvram.h | 1 +
> > > include/linux/compiler.h | 4 -
> > > include/linux/rcu_node_tree.h | 4 +
> > > include/linux/rcu_segcblist.h | 4 +
> > > include/linux/rcupdate.h | 318 +---------
> > > include/linux/rcutiny.h | 167 +-----
> > > include/linux/rcutree.h | 21 +-
> > > include/linux/spinlock.h | 20 +
> > > include/linux/srcu.h | 25 +-
> > > include/linux/srcuclassic.h | 115 ----
> > > include/linux/srcutiny.h | 47 +-
> > > include/linux/srcutree.h | 13 +-
> > > include/trace/events/rcu.h | 1 +
> > > init/Kconfig | 349 +----------
> > > kernel/locking/lockdep.c | 176 +++---
> > > kernel/rcu/Kconfig | 242 ++++++++
> > > kernel/rcu/Kconfig.debug | 82 +++
> > > kernel/rcu/Makefile | 2 -
> > > kernel/rcu/rcu.h | 277 +++++++++
> > > kernel/rcu/rcuperf.c | 129 +++-
> > > kernel/rcu/rcutorture.c | 21 +-
> > > kernel/rcu/srcu.c | 661 ---------------------
> > > kernel/rcu/srcutiny.c | 86 +--
> > > kernel/rcu/srcutree.c | 187 ++++--
> > > kernel/rcu/tiny.c | 54 +-
> > > kernel/rcu/tiny_plugin.h | 123 ----
> > > kernel/rcu/tree.c | 195 +++---
> > > kernel/rcu/tree.h | 109 +---
> > > kernel/rcu/tree_exp.h | 2 +-
> > > kernel/rcu/tree_plugin.h | 573 +++---------------
> > > kernel/rcu/tree_trace.c | 494 ---------------
> > > kernel/rcu/update.c | 77 ++-
> > > kernel/sched/core.c | 8 +-
> > > kernel/time/Kconfig | 50 --
> > > lib/Kconfig.debug | 184 +-----
> > > lib/Makefile | 3 -
> > > scripts/checkpatch.pl | 17 -
> > > .../selftests/rcutorture/bin/configcheck.sh | 2 +-
> > > .../testing/selftests/rcutorture/bin/kvm-build.sh | 2 +-
> > > tools/testing/selftests/rcutorture/bin/kvm.sh | 5 +-
> > > .../selftests/rcutorture/configs/rcu/CFLIST | 2 +
> > > .../selftests/rcutorture/configs/rcu/SRCU-C.boot | 1 +
> > > .../selftests/rcutorture/configs/rcu/SRCU-N | 2 +-
> > > .../selftests/rcutorture/configs/rcu/SRCU-P | 6 +-
> > > .../selftests/rcutorture/configs/rcu/SRCU-t | 10 +
> > > .../selftests/rcutorture/configs/rcu/SRCU-t.boot | 1 +
> > > .../selftests/rcutorture/configs/rcu/SRCU-u | 9 +
> > > .../selftests/rcutorture/configs/rcu/SRCU-u.boot | 1 +
> > > .../selftests/rcutorture/configs/rcu/TINY02 | 5 +-
> > > .../selftests/rcutorture/configs/rcu/TREE01 | 5 +-
> > > .../selftests/rcutorture/configs/rcu/TREE01.boot | 4 +
> > > .../selftests/rcutorture/configs/rcu/TREE02 | 5 +-
> > > .../selftests/rcutorture/configs/rcu/TREE03 | 4 -
> > > .../selftests/rcutorture/configs/rcu/TREE03.boot | 4 +
> > > .../selftests/rcutorture/configs/rcu/TREE04 | 4 -
> > > .../selftests/rcutorture/configs/rcu/TREE05 | 4 -
> > > .../selftests/rcutorture/configs/rcu/TREE05.boot | 3 +
> > > .../selftests/rcutorture/configs/rcu/TREE06 | 4 +-
> > > .../selftests/rcutorture/configs/rcu/TREE06.boot | 3 +
> > > .../selftests/rcutorture/configs/rcu/TREE07 | 6 -
> > > .../selftests/rcutorture/configs/rcu/TREE08 | 1 -
> > > .../selftests/rcutorture/configs/rcu/TREE08-T | 21 -
> > > .../selftests/rcutorture/configs/rcu/TREE08.boot | 1 +
> > > .../configs/{rcu/TREE02-T => rcuperf/TINY} | 19 +-
> > > .../selftests/rcutorture/configs/rcuperf/TREE | 1 -
> > > .../selftests/rcutorture/configs/rcuperf/TREE54 | 1 -
> > > .../testing/selftests/rcutorture/doc/TINY_RCU.txt | 1 -
> > > .../selftests/rcutorture/doc/TREE_RCU-kconfig.txt | 34 +-
> > > .../rcutorture/formal/srcu-cbmc/modify_srcu.awk | 2 +-
> > > 79 files changed, 1496 insertions(+), 4211 deletions(-)
> > > delete mode 100644 Documentation/RCU/trace.txt
> > > delete mode 100644 include/linux/srcuclassic.h
> > > create mode 100644 kernel/rcu/Kconfig
> > > create mode 100644 kernel/rcu/Kconfig.debug
> > > delete mode 100644 kernel/rcu/srcu.c
> > > delete mode 100644 kernel/rcu/tree_trace.c
> > > create mode 100644 tools/testing/selftests/rcutorture/configs/rcu/SRCU-C.boot
> > > create mode 100644 tools/testing/selftests/rcutorture/configs/rcu/SRCU-t
> > > create mode 100644 tools/testing/selftests/rcutorture/configs/rcu/SRCU-t.boot
> > > create mode 100644 tools/testing/selftests/rcutorture/configs/rcu/SRCU-u
> > > create mode 100644 tools/testing/selftests/rcutorture/configs/rcu/SRCU-u.boot
> > > delete mode 100644 tools/testing/selftests/rcutorture/configs/rcu/TREE08-T
> > > rename tools/testing/selftests/rcutorture/configs/{rcu/TREE02-T => rcuperf/TINY} (50%)
> > >
> >
>