Messages in this thread | | | Date | Thu, 17 Aug 2017 05:30:38 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH v5 tip/core/rcu 4/9] completion: Replace spin_unlock_wait() with lock/unlock pair |
| |
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
| |