Re: [PATCH v5 tip/core/rcu 4/9] completion: Replace spin_unlock_wait() with lock/unlock pair

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Thu, Aug 17, 2017 at 10:26:16AM +0200, Ingo Molnar wrote:
> 
> * Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> 
> > There is no agreed-upon definition of spin_unlock_wait()'s semantics,
> > and it appears that all callers could do just as well with a lock/unlock
> > pair.  This commit therefore replaces the spin_unlock_wait() call in
> > completion_done() with spin_lock() followed immediately by spin_unlock().
> > This should be safe from a performance perspective because the lock
> > will be held only the wakeup happens really quickly.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Alan Stern <stern@rowland.harvard.edu>
> > Cc: Andrea Parri <parri.andrea@gmail.com>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > [ paulmck: Updated to use irqsave based on 0day Test Robot feedback. ]
> > 
> > diff --git a/kernel/sched/completion.c b/kernel/sched/completion.c
> > index 13fc5ae9bf2f..c9524d2d9316 100644
> > --- a/kernel/sched/completion.c
> > +++ b/kernel/sched/completion.c
> > @@ -300,6 +300,8 @@ EXPORT_SYMBOL(try_wait_for_completion);
> >   */
> >  bool completion_done(struct completion *x)
> >  {
> > +	unsigned long flags;
> > +
> >  	if (!READ_ONCE(x->done))
> >  		return false;
> >  
> > @@ -307,14 +309,9 @@ bool completion_done(struct completion *x)
> >  	 * If ->done, we need to wait for complete() to release ->wait.lock
> >  	 * otherwise we can end up freeing the completion before complete()
> >  	 * is done referencing it.
> > -	 *
> > -	 * The RMB pairs with complete()'s RELEASE of ->wait.lock and orders
> > -	 * the loads of ->done and ->wait.lock such that we cannot observe
> > -	 * the lock before complete() acquires it while observing the ->done
> > -	 * after it's acquired the lock.
> >  	 */
> > -	smp_rmb();
> > -	spin_unlock_wait(&x->wait.lock);
> > +	spin_lock_irqsave(&x->wait.lock, flags);
> > +	spin_unlock_irqrestore(&x->wait.lock, flags);
> >  	return true;
> >  }
> >  EXPORT_SYMBOL(completion_done);
> 
> I'm fine with this patch - as long as there are no performance regression reports. 
> (which I suspect there won't be.)

If there is, 0day Test Robot hasn't spotted it, nor has anyone else
reported it.

> Would you like to carry this in the RCU tree, due to other changes depending on 
> this change - or can I pick this up into the scheduler tree?

Timely question!  ;-)

My current plan is to send you a pull request like the following later
today, Pacific Time (but rebased adding Steve Rostedt's Reviewed-by).
This patch is on one of the branches, currently v4.13-rc2..93d8d7a12090
("arch: Remove spin_unlock_wait() arch-specific definitions") in my
-rcu tree.

Ah, and v4.13-rc2..7391304c4959 ("membarrier: Expedited private command")
is mostly outside of RCU as well.

Since I will be rebasing and remerging anyway, if you would prefer that I
split the spin_unlock_wait() and/or misc branches out, I am happy to do so.
If I don't hear otherwise, though, I will send all seven branches using
my usual approach.

So, if you want something different than my usual approach, please just
let me know!

							Thanx, Paul

PS.	At the moment, there are no code changes to be applied, just
	Steve's Reviewed-by.

------------------------------------------------------------------------
Prototype pull request
------------------------------------------------------------------------

The following changes since commit 520eccdfe187591a51ea9ab4c1a024ae4d0f68d9:

  Linux 4.13-rc2 (2017-07-23 16:15:17 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git 20f2d43c114bf57d16580a758120cc65aa991bea

for you to fetch changes up to 20f2d43c114bf57d16580a758120cc65aa991bea:

  Merge branches 'doc.2017.07.24c', 'fixes.2017.08.09a', 'hotplug.2017.07.25b', 'misc.2017.08.10a', 'spin_unlock_wait_no.2017.08.11a', 'srcu.2017.07.27c' and 'torture.2017.07.24c' into HEAD (2017-08-11 13:15:44 -0700)

----------------------------------------------------------------
Joe Perches (1):
      module: Fix pr_fmt() bug for header use of printk

Luis R. Rodriguez (2):
      swait: add idle variants which don't contribute to load average
      rcu: use idle versions of swait to make idle-hack clear

Manfred Spraul (1):
      net/netfilter/nf_conntrack_core: Fix net_conntrack_lock()

Masami Hiramatsu (1):
      rcu/tracing: Set disable_rcu_irq_enter on rcu_eqs_exit()

Mathieu Desnoyers (1):
      membarrier: Expedited private command

Oleg Nesterov (1):
      task_work: Replace spin_unlock_wait() with lock/unlock pair

Paul E. McKenney (56):
      documentation: Fix relation between nohz_full and rcu_nocbs
      doc: RCU documentation update
      doc: Update memory-barriers.txt for read-to-write dependencies
      doc: Add RCU files to docbook-generation files
      doc: No longer allowed to use rcu_dereference on non-pointers
      doc: Set down RCU's scheduling-clock-interrupt needs
      init_task: Remove redundant INIT_TASK_RCU_TREE_PREEMPT() macro
      srcu: Move rcu_scheduler_starting() from Tiny RCU to Tiny SRCU
      rcutorture: Remove obsolete SRCU-C.boot
      srcu: Make process_srcu() be static
      rcutorture: Move SRCU status printing to SRCU implementations
      rcutorture: Print SRCU lock/unlock totals
      rcu: Remove CONFIG_TASKS_RCU ifdef from rcuperf.c
      rcutorture: Select CONFIG_PROVE_LOCKING for Tiny SRCU scenario
      torture: Add --kconfig argument to kvm.sh
      rcutorture: Don't wait for kernel when all builds fail
      rcutorture: Enable SRCU readers from timer handler
      rcutorture: Place event-traced strings into trace buffer
      rcutorture: Use nr_cpus rather than maxcpus to limit test size
      rcutorture: Add task's CPU for rcutorture writer stalls
      rcutorture: Eliminate unused ts_rem local from rcu_trace_clock_local()
      rcu: Add last-CPU to GP-kthread starvation messages
      rcutorture: Invoke call_rcu() from timer handler
      rcu: Use timer as backstop for NOCB deferred wakeups
      atomics: Revert addition of comment header to spin_unlock_wait()
      rcu: Migrate callbacks earlier in the CPU-offline timeline
      rcu: Make expedited GPs correctly handle hardware CPU insertion
      torture: Fix typo suppressing CPU-hotplug statistics
      rcu: Remove orphan/adopt event-tracing fields
      rcu: Check for NOCB CPUs and empty lists earlier in CB migration
      rcu: Make NOCB CPUs migrate CBs directly from outgoing CPU
      rcu: Advance outgoing CPU's callbacks before migrating them
      rcu: Eliminate rcu_state ->orphan_lock
      rcu: Advance callbacks after migration
      rcu: Localize rcu_state ->orphan_pend and ->orphan_done
      rcu: Remove unused RCU list functions
      rcu: Move callback-list warning to irq-disable region
      srcu: Provide ordering for CPU not involved in grace period
      sched,rcu: Make cond_resched() provide RCU quiescent state
      rcu: Drive TASKS_RCU directly off of PREEMPT
      rcu: Create reasonable API for do_exit() TASKS_RCU processing
      rcu: Add TPS() to event-traced strings
      rcu: Move rcu.h to new trivial-function style
      rcu: Add event tracing to ->gp_tasks update at GP start
      rcu: Add TPS() protection for _rcu_barrier_trace strings
      rcu: Add assertions verifying blocked-tasks list
      rcu: Add warning to rcu_idle_enter() for irqs enabled
      rcu: Remove exports from rcu_idle_exit() and rcu_idle_enter()
      sched: Replace spin_unlock_wait() with lock/unlock pair
      completion: Replace spin_unlock_wait() with lock/unlock pair
      exit: Replace spin_unlock_wait() with lock/unlock pair
      ipc: Replace spin_unlock_wait() with lock/unlock pair
      drivers/ata: Replace spin_unlock_wait() with lock/unlock pair
      locking: Remove spin_unlock_wait() generic definitions
      arch: Remove spin_unlock_wait() arch-specific definitions
      Merge branches 'doc.2017.07.24c', 'fixes.2017.08.09a', 'hotplug.2017.07.25b', 'misc.2017.08.10a', 'spin_unlock_wait_no.2017.08.11a', 'srcu.2017.07.27c' and 'torture.2017.07.24c' into HEAD

Peter Zijlstra (Intel) (1):
      rcu: Make rcu_idle_enter() rely on callers disabling irqs

Tejun Heo (1):
      sched: Allow migrating kthreads into online but inactive CPUs

 .../RCU/Design/Requirements/Requirements.html      | 130 +++++++++++
 Documentation/RCU/checklist.txt                    | 121 +++++++----
 Documentation/RCU/rcu.txt                          |   9 +-
 Documentation/RCU/rcu_dereference.txt              |  61 ++----
 Documentation/RCU/rcubarrier.txt                   |   5 +
 Documentation/RCU/torture.txt                      |  20 +-
 Documentation/RCU/whatisRCU.txt                    |   5 +-
 Documentation/admin-guide/kernel-parameters.txt    |   7 +-
 Documentation/core-api/kernel-api.rst              |  49 +++++
 Documentation/memory-barriers.txt                  |  41 ++--
 MAINTAINERS                                        |   2 +-
 arch/alpha/include/asm/spinlock.h                  |   5 -
 arch/arc/include/asm/spinlock.h                    |   5 -
 arch/arm/include/asm/spinlock.h                    |  16 --
 arch/arm64/include/asm/spinlock.h                  |  58 +----
 arch/arm64/kernel/process.c                        |   2 +
 arch/blackfin/include/asm/spinlock.h               |   5 -
 arch/blackfin/kernel/module.c                      |  39 ++--
 arch/hexagon/include/asm/spinlock.h                |   5 -
 arch/ia64/include/asm/spinlock.h                   |  21 --
 arch/m32r/include/asm/spinlock.h                   |   5 -
 arch/metag/include/asm/spinlock.h                  |   5 -
 arch/mn10300/include/asm/spinlock.h                |   5 -
 arch/parisc/include/asm/spinlock.h                 |   7 -
 arch/powerpc/include/asm/spinlock.h                |  33 ---
 arch/s390/include/asm/spinlock.h                   |   7 -
 arch/sh/include/asm/spinlock-cas.h                 |   5 -
 arch/sh/include/asm/spinlock-llsc.h                |   5 -
 arch/sparc/include/asm/spinlock_32.h               |   5 -
 arch/tile/include/asm/spinlock_32.h                |   2 -
 arch/tile/include/asm/spinlock_64.h                |   2 -
 arch/tile/lib/spinlock_32.c                        |  23 --
 arch/tile/lib/spinlock_64.c                        |  22 --
 arch/xtensa/include/asm/spinlock.h                 |   5 -
 drivers/ata/libata-eh.c                            |   8 +-
 include/asm-generic/qspinlock.h                    |  14 --
 include/linux/init_task.h                          |   8 +-
 include/linux/rcupdate.h                           |  15 +-
 include/linux/rcutiny.h                            |   8 +-
 include/linux/sched.h                              |   8 +-
 include/linux/spinlock.h                           |  31 ---
 include/linux/spinlock_up.h                        |   6 -
 include/linux/srcutiny.h                           |  13 ++
 include/linux/srcutree.h                           |   3 +-
 include/linux/swait.h                              |  55 +++++
 include/trace/events/rcu.h                         |   7 +-
 include/uapi/linux/membarrier.h                    |  23 +-
 ipc/sem.c                                          |   3 +-
 kernel/Makefile                                    |   1 -
 kernel/cpu.c                                       |   1 +
 kernel/exit.c                                      |  10 +-
 kernel/locking/qspinlock.c                         | 117 ----------
 kernel/membarrier.c                                |  70 ------
 kernel/rcu/Kconfig                                 |   3 +-
 kernel/rcu/rcu.h                                   | 128 ++---------
 kernel/rcu/rcu_segcblist.c                         | 108 +++-------
 kernel/rcu/rcu_segcblist.h                         |  28 +--
 kernel/rcu/rcuperf.c                               |  17 +-
 kernel/rcu/rcutorture.c                            |  83 +++----
 kernel/rcu/srcutiny.c                              |   8 +
 kernel/rcu/srcutree.c                              |  50 ++++-
 kernel/rcu/tiny.c                                  |   2 -
 kernel/rcu/tiny_plugin.h                           |  47 ----
 kernel/rcu/tree.c                                  | 238 ++++++++-------------
 kernel/rcu/tree.h                                  |  15 +-
 kernel/rcu/tree_exp.h                              |   2 +-
 kernel/rcu/tree_plugin.h                           | 238 ++++++++++++---------
 kernel/rcu/update.c                                |  18 +-
 kernel/sched/Makefile                              |   1 +
 kernel/sched/completion.c                          |  11 +-
 kernel/sched/core.c                                |  39 +++-
 kernel/sched/membarrier.c                          | 152 +++++++++++++
 kernel/task_work.c                                 |   8 +-
 kernel/torture.c                                   |   2 +-
 net/netfilter/nf_conntrack_core.c                  |  52 +++--
 .../selftests/rcutorture/bin/config_override.sh    |  61 ++++++
 .../testing/selftests/rcutorture/bin/functions.sh  |  27 ++-
 .../testing/selftests/rcutorture/bin/kvm-build.sh  |  11 +-
 .../selftests/rcutorture/bin/kvm-test-1-run.sh     |  58 ++---
 tools/testing/selftests/rcutorture/bin/kvm.sh      |  34 ++-
 .../selftests/rcutorture/configs/rcu/BUSTED.boot   |   2 +-
 .../selftests/rcutorture/configs/rcu/SRCU-C.boot   |   1 -
 .../selftests/rcutorture/configs/rcu/SRCU-u        |   3 +-
 .../selftests/rcutorture/configs/rcu/TREE01.boot   |   2 +-
 .../selftests/rcutorture/doc/TREE_RCU-kconfig.txt  |   2 +-
 85 files changed, 1245 insertions(+), 1344 deletions(-)
 delete mode 100644 kernel/membarrier.c
 delete mode 100644 kernel/rcu/tiny_plugin.h
 create mode 100644 kernel/sched/membarrier.c
 create mode 100755 tools/testing/selftests/rcutorture/bin/config_override.sh
 delete mode 100644 tools/testing/selftests/rcutorture/configs/rcu/SRCU-C.boot