Treat mutex->owner as volatile

When HAVE_MUTEX_OWNER is defined and we are directly accessing
mutex->owner treat is as volative with the ACCESS_ONCE() helper.
Without this you may get a stale cached value when accessing it
from different cpus.  This can result in incorrect behavior from
mutex_owned() and mutex_owner().  This is not a problem for the
!HAVE_MUTEX_OWNER case because in this case all the accesses
are covered by a spin lock which similarly gaurentees we will
not be accessing stale data.

Secondly, check CONFIG_SMP before allowing access to mutex->owner.
I see that for non-SMP setups the kernel does not track the owner
so we cannot rely on it.

Thirdly, check CONFIG_MUTEX_DEBUG when this is defined and the
HAVE_MUTEX_OWNER is defined surprisingly the mutex->owner will
not be cleared on mutex_exit().  When this is the case the SPL
needs to make sure to do it to ensure MUTEX_HELD() behaves as
expected or you will certainly assert in mutex_destroy().

Finally, improve the mutex regression tests.  For mutex_owned() we
now minimally check that it behaves correctly when checked from the
owner thread or the non-owner thread.  This subtle behaviour has bit
me before and I'd like to catch it early next time if it reappears.

As for mutex_owned() regression test additonally verify that
mutex->owner is always cleared on mutex_exit().

1 files changed, 28 insertions, 10 deletions

diff --git a/include/sys/mutex.h b/include/sys/mutex.h
index b36c7e256..d33694766 100644
--- a/include/sys/mutex.h
+++ b/include/sys/mutex.h
@@ -34,20 +34,29 @@ typedef enum {
         MUTEX_ADAPTIVE = 2
 } kmutex_type_t;
 
-#ifdef HAVE_MUTEX_OWNER
+#if defined(HAVE_MUTEX_OWNER) && defined(CONFIG_SMP)
 
 typedef struct mutex kmutex_t;
 
 static inline kthread_t *
 mutex_owner(kmutex_t *mp)
 {
-        if (mp->owner)
-                return (mp->owner)->task;
+	struct thread_info *owner;
+
+	owner = ACCESS_ONCE(mp->owner);
+        if (owner)
+                return owner->task;
 
         return NULL;
 }
-#define mutex_owned(mp)         (mutex_owner(mp) == current)
-#define MUTEX_HELD(mp)          mutex_owned(mp)
+
+static inline int
+mutex_owned(kmutex_t *mp)
+{
+	return (ACCESS_ONCE(mp->owner) == current_thread_info());
+}
+
+#define MUTEX_HELD(mp)                  mutex_owned(mp)
 #undef mutex_init
 #define mutex_init(mp, name, type, ibc)                                 \
 ({                                                                      \
@@ -60,13 +69,22 @@ mutex_owner(kmutex_t *mp)
 #undef mutex_destroy
 #define mutex_destroy(mp)                                               \
 ({                                                                      \
-        VERIFY(!MUTEX_HELD(mp));                                        \
+	VERIFY3P(mutex_owner(mp), ==, NULL);				\
 })
 
-#define mutex_tryenter(mp)      mutex_trylock(mp)
-#define mutex_enter(mp)         mutex_lock(mp)
-#define mutex_exit(mp)          mutex_unlock(mp)
+#define mutex_tryenter(mp)              mutex_trylock(mp)
+#define mutex_enter(mp)                 mutex_lock(mp)
 
+/* mutex->owner is not cleared when CONFIG_DEBUG_MUTEXES is set */
+#ifdef CONFIG_DEBUG_MUTEXES
+# define mutex_exit(mp)                                                 \
+({                                                                      \
+         (mp)->owner = NULL;                                            \
+	 mutex_unlock(mp);                                              \
+})
+#else
+# define mutex_exit(mp)                 mutex_unlock(mp)
+#endif /* CONFIG_DEBUG_MUTEXES */
 
 #ifdef HAVE_GPL_ONLY_SYMBOLS
 # define mutex_enter_nested(mp, sc)     mutex_lock_nested(mp, sc)
@@ -151,7 +169,7 @@ mutex_owner(kmutex_t *mp)
 #undef mutex_destroy
 #define mutex_destroy(mp)                                               \
 ({                                                                      \
-        VERIFY(!MUTEX_HELD(mp));                                        \
+	VERIFY3P(mutex_owner(mp), ==, NULL);				\
 })
 
 #define mutex_tryenter(mp)                                              \