From: Manfred Spraul <manfred@colorfullife.com>
To: benh@kernel.crashing.org, paulmck@linux.vnet.ibm.com,
Ingo Molnar <mingo@elte.hu>, Boqun Feng <boqun.feng@gmail.com>,
Peter Zijlstra <peterz@infradead.org>,
Andrew Morton <akpm@linux-foundation.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
1vier1@web.de, Davidlohr Bueso <dave@stgolabs.net>,
Manfred Spraul <manfred@colorfullife.com>
Subject: [PATCH 1/4 v4] spinlock: Document memory barrier rules
Date: Mon, 29 Aug 2016 15:34:26 +0200 [thread overview]
Message-ID: <1472477669-27508-2-git-send-email-manfred@colorfullife.com> (raw)
In-Reply-To: <1472477669-27508-1-git-send-email-manfred@colorfullife.com>
Right now, the spinlock machinery tries to guarantee barriers even for
unorthodox locking cases, which ends up as a constant stream of updates
as the architectures try to support new unorthodox ideas.
The patch proposes to clarify the rules:
spin_lock is ACQUIRE, spin_unlock is RELEASE.
spin_unlock_wait is also ACQUIRE.
Code that needs further guarantees must use appropriate explicit barriers.
Architectures that can implement some barriers for free can define the
barriers as NOPs.
As the initial step, the patch converts ipc/sem.c to the new defines:
- With commit 2c6100227116
("locking/qspinlock: Fix spin_unlock_wait() some more"),
(and the commits for the other archs), spin_unlock_wait() is an
ACQUIRE.
Therefore the smp_rmb() after spin_unlock_wait() can be removed.
- smp_mb__after_spin_lock() instead of a direct smp_mb().
This allows that architectures override it with a less expensive
barrier if this is sufficient for their hardware/spinlock
implementation.
For overriding, the same approach as for smp_mb__before_spin_lock()
is used: If smp_mb__after_spin_lock is already defined, then it is
not changed.
Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
---
Documentation/locking/spinlocks.txt | 5 +++++
include/linux/spinlock.h | 12 ++++++++++++
ipc/sem.c | 16 +---------------
3 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/Documentation/locking/spinlocks.txt b/Documentation/locking/spinlocks.txt
index ff35e40..fc37beb 100644
--- a/Documentation/locking/spinlocks.txt
+++ b/Documentation/locking/spinlocks.txt
@@ -40,6 +40,11 @@ example, internal driver data structures that nobody else ever touches).
touches a shared variable has to agree about the spinlock they want
to use.
+ NOTE! Code that needs stricter memory barriers than ACQUIRE during
+ LOCK and RELEASE during UNLOCK must use appropriate memory barriers
+ such as smp_mb__after_spin_lock().
+ spin_unlock_wait() has ACQUIRE semantics.
+
----
Lesson 2: reader-writer spinlocks.
diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
index 47dd0ce..d79000e 100644
--- a/include/linux/spinlock.h
+++ b/include/linux/spinlock.h
@@ -130,6 +130,18 @@ do { \
#define smp_mb__before_spinlock() smp_wmb()
#endif
+#ifndef smp_mb__after_spin_lock
+/**
+ * smp_mb__after_spin_lock() - Provide smp_mb() after spin_lock
+ *
+ * spin_lock() provides ACQUIRE semantics regarding reading the lock.
+ * There are no guarantees that the lock write is visible before any read
+ * or write operation within the protected area is performed.
+ * If the lock write must happen first, this function is required.
+ */
+#define smp_mb__after_spin_lock() smp_mb()
+#endif
+
/**
* raw_spin_unlock_wait - wait until the spinlock gets unlocked
* @lock: the spinlock in question.
diff --git a/ipc/sem.c b/ipc/sem.c
index 5e318c5..ac15ab2 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -290,14 +290,6 @@ static void complexmode_enter(struct sem_array *sma)
sem = sma->sem_base + i;
spin_unlock_wait(&sem->lock);
}
- /*
- * spin_unlock_wait() is not a memory barriers, it is only a
- * control barrier. The code must pair with spin_unlock(&sem->lock),
- * thus just the control barrier is insufficient.
- *
- * smp_rmb() is sufficient, as writes cannot pass the control barrier.
- */
- smp_rmb();
}
/*
@@ -363,13 +355,7 @@ static inline int sem_lock(struct sem_array *sma, struct sembuf *sops,
*/
spin_lock(&sem->lock);
- /*
- * See 51d7d5205d33
- * ("powerpc: Add smp_mb() to arch_spin_is_locked()"):
- * A full barrier is required: the write of sem->lock
- * must be visible before the read is executed
- */
- smp_mb();
+ smp_mb__after_spin_lock();
if (!smp_load_acquire(&sma->complex_mode)) {
/* fast path successful! */
--
2.5.5
next prev parent reply other threads:[~2016-08-29 13:36 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-29 13:34 [PATCH 0/4 V4] Clarify/standardize memory barriers for lock/unlock Manfred Spraul
2016-08-29 13:34 ` Manfred Spraul [this message]
2016-08-29 17:38 ` [PATCH 1/4 v4] spinlock: Document memory barrier rules Davidlohr Bueso
2016-08-29 13:34 ` [PATCH 2/4 V4] spinlock.h: Move smp_mb__after_unlock_lock to spinlock.h Manfred Spraul
2016-08-29 13:34 ` [PATCH 3/4 V4] net/netfilter/nf_conntrack_core: update memory barriers Manfred Spraul
2016-08-29 13:34 ` [PATCH 4/4 V4] qspinlock for x86: smp_mb__after_spin_lock() is free Manfred Spraul
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1472477669-27508-2-git-send-email-manfred@colorfullife.com \
--to=manfred@colorfullife.com \
--cc=1vier1@web.de \
--cc=akpm@linux-foundation.org \
--cc=benh@kernel.crashing.org \
--cc=boqun.feng@gmail.com \
--cc=dave@stgolabs.net \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=paulmck@linux.vnet.ibm.com \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox