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, 14 Jun 2017 16:33:22 +0200
From:   Andrea Parri <parri.andrea@...il.com>
To:     "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc:     linux-kernel@...r.kernel.org, priyalee.kushwaha@...el.com,
        drozdziak1@...il.com, arnd@...db.de, ldr709@...il.com,
        tglx@...utronix.de, peterz@...radead.org, josh@...htriplett.org,
        nico@...aro.org, kjlx@...pleofstupid.com, vegard.nossum@...cle.com,
        torvalds@...ux-foundation.org, dcb314@...mail.com,
        fengguang.wu@...el.com, fweisbec@...il.com, riel@...hat.com,
        rostedt@...dmis.org, mingo@...nel.org, stern@...land.harvard.edu
Subject: Re: [GIT PULL rcu/next] RCU commits for 4.13

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%)
> > > 
> > 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ