Thread (21 messages) flat view 21 messages, 3 authors, 2012-08-13

RFC: mutex: hung tasks on SMP platforms with asm-generic/mutex-xchg.h

From: Will Deacon <hidden>
Date: 2012-08-09 17:50:40
Also in: lkml

On Thu, Aug 09, 2012 at 05:57:33PM +0100, Nicolas Pitre wrote:
quoted hunk ↗ jump to hunk
On Thu, 9 Aug 2012, Nicolas Pitre wrote:
quoted
Yes, that looks fine.  I'd remove that if (prev < 0) entirely though.  
We'll just swap a 0 for a 0 if the count wasn't < 0, or a 0 for a 1 if 
the mutex just got unlocked which is also fine.  This is especially 
beneficial when a native xchg processor instruction is used.
In fact, this suggestion isn't entirely correct either. The inner xchg 
in this case should be -1 and not 'count'.  If 'count' is 0 and the 
mutex becomes contended in the small window between the two xchg's then 
the contention mark would be lost again.

In other words, I think this should look like this:
diff --git a/include/asm-generic/mutex-xchg.h b/include/asm-generic/mutex-xchg.h
index 580a6d35c7..44a66c99c8 100644
--- a/include/asm-generic/mutex-xchg.h
+++ b/include/asm-generic/mutex-xchg.h
@@ -25,8 +25,11 @@
 static inline void
 __mutex_fastpath_lock(atomic_t *count, void (*fail_fn)(atomic_t *))
 {
-	if (unlikely(atomic_xchg(count, 0) != 1))
-		fail_fn(count);
+	if (unlikely(atomic_xchg(count, 0) != 1)) {
+		/* Mark lock contention explicitly */
+		if (likely(atomic_xchg(count, -1) != 1))
+			fail_fn(count);
+	}
 }
 
 /**
Doesn't this mean that we're no longer just swapping 0 for a 0 if the lock
was taken, therefore needlessly sending the current owner down the slowpath
on unlock? If we have the explicit test, that doesn't happen and the
disassembly isn't much different (an additional cmp/conditional branch).

Will
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help