Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752256AbdFNOdg (ORCPT ); Wed, 14 Jun 2017 10:33:36 -0400 Received: from mail-wr0-f193.google.com ([209.85.128.193]:32860 "EHLO mail-wr0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751857AbdFNOde (ORCPT ); Wed, 14 Jun 2017 10:33:34 -0400 Date: Wed, 14 Jun 2017 16:33:22 +0200 From: Andrea Parri To: "Paul E. McKenney" Cc: linux-kernel@vger.kernel.org, priyalee.kushwaha@intel.com, drozdziak1@gmail.com, arnd@arndb.de, ldr709@gmail.com, tglx@linutronix.de, peterz@infradead.org, josh@joshtriplett.org, nico@linaro.org, kjlx@templeofstupid.com, vegard.nossum@oracle.com, torvalds@linux-foundation.org, dcb314@hotmail.com, fengguang.wu@intel.com, fweisbec@gmail.com, riel@redhat.com, rostedt@goodmis.org, mingo@kernel.org, stern@rowland.harvard.edu Subject: Re: [GIT PULL rcu/next] RCU commits for 4.13 Message-ID: <20170614143322.GA3368@andrea> References: <20170612213754.GA7201@linux.vnet.ibm.com> <20170614025404.GA2525@andrea> <20170614043317.GK3721@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170614043317.GK3721@linux.vnet.ibm.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 16602 Lines: 304 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@linux.vnet.ibm.com > > > > 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@vger.kernel.org/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%) > > > > > >