On Thu, Apr 13, 2017 at 07:51:36PM +0200, Peter Zijlstra wrote:
> On Thu, Apr 13, 2017 at 10:39:51AM -0700, Paul E. McKenney wrote:
>
> > Well, if there are no objections, I will fix up the smp_mb__before_atomic()
> > and smp_mb__after_atomic() pieces.
>
> Feel free.
How about if I add this in the atomic_ops.txt description of these
two primitives?
Preceding a non-value-returning read-modify-write atomic
operation with smp_mb__before_atomic() and following it with
smp_mb__after_atomic() provides the same full ordering that is
provided by value-returning read-modify-write atomic operations.
> > I suppose that one alternative is the new variant of kerneldoc, though
> > very few of these functions have comment headers, let alone kerneldoc
> > headers. Which reminds me, the question of spin_unlock_wait() and
> > spin_is_locked() semantics came up a bit ago. Here is what I believe
> > to be the case. Does this match others' expectations?
> >
> > o spin_unlock_wait() semantics:
> >
> > 1. Any access in any critical section prior to the
> > spin_unlock_wait() is visible to all code following
> > (in program order) the spin_unlock_wait().
> >
> > 2. Any access prior (in program order) to the
> > spin_unlock_wait() is visible to any critical
> > section following the spin_unlock_wait().
> >
> > o spin_is_locked() semantics: Half of spin_unlock_wait(),
> > but only if it returns false:
> >
> > 1. Any access in any critical section prior to the
> > spin_unlock_wait() is visible to all code following
> > (in program order) the spin_unlock_wait().
>
> Urgh.. yes those are pain. The best advise is to not use them.
>
> 055ce0fd1b86 ("locking/qspinlock: Add comments")
Ah, I must confess that I missed that one. Would you be OK with the
following patch, which adds a docbook header comment for both of them?
Thanx, Paul
------------------------------------------------------------------------
commit 5789953adc360b4d3685dc89513655e6bfb83980
Author: Paul E. McKenney <[email protected]>
Date: Wed Apr 19 16:20:07 2017 -0700
atomics: Add header comment so spin_unlock_wait() and spin_is_locked()
There is material describing the ordering guarantees provided by
spin_unlock_wait() and spin_is_locked(), but it is not necessarily
easy to find. This commit therefore adds a docbook header comment
to both functions informally describing their semantics.
Signed-off-by: Paul E. McKenney <[email protected]>
diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
index 59248dcc6ef3..2647dc7f3ea9 100644
--- a/include/linux/spinlock.h
+++ b/include/linux/spinlock.h
@@ -369,11 +369,49 @@ static __always_inline int spin_trylock_irq(spinlock_t
*lock)
raw_spin_trylock_irqsave(spinlock_check(lock), flags); \
})
+/**
+ * spin_unlock_wait - Interpose between successive critical sections
+ * @lock: the spinlock whose critical sections are to be interposed.
+ *
+ * Semantically this is equivalent to a spin_lock() immediately
+ * followed by a spin_unlock(). However, most architectures have
+ * more efficient implementations in which the spin_unlock_wait()
+ * cannot block concurrent lock acquisition, and in some cases
+ * where spin_unlock_wait() does not write to the lock variable.
+ * Nevertheless, spin_unlock_wait() can have high overhead, so if
+ * you feel the need to use it, please check to see if there is
+ * a better way to get your job done.
+ *
+ * The ordering guarantees provided by spin_unlock_wait() are:
+ *
+ * 1. All accesses preceding the spin_unlock_wait() happen before
+ * any accesses in later critical sections for this same lock.
+ * 2. All accesses following the spin_unlock_wait() happen after
+ * any accesses in earlier critical sections for this same lock.
+ */
static __always_inline void spin_unlock_wait(spinlock_t *lock)
{
raw_spin_unlock_wait(&lock->rlock);
}
+/**
+ * spin_is_locked - Conditionally interpose after prior critical sections
+ * @lock: the spinlock whose critical sections are to be interposed.
+ *
+ * Semantically this is equivalent to a spin_trylock(), and, if
+ * the spin_trylock() succeeds, immediately followed by a (mythical)
+ * spin_unlock_relaxed(). The return value from spin_trylock() is returned
+ * by spin_is_locked(). Note that all current architectures have extremely
+ * efficient implementations in which the spin_is_locked() does not even
+ * write to the lock variable.
+ *
+ * A successful spin_is_locked() primitive in some sense "takes its place"
+ * after some critical section for the lock in question. Any accesses
+ * following a successful spin_is_locked() call will therefore happen
+ * after any accesses by any of the preceding critical section for that
+ * same lock. Note however, that spin_is_locked() provides absolutely no
+ * ordering guarantees for code preceding the call to that spin_is_locked().
+ */
static __always_inline int spin_is_locked(spinlock_t *lock)
{
return raw_spin_is_locked(&lock->rlock);
