linux-kernel - [tip:core/locking] locking/mcs: Micro-optimize the MCS code, add extra comments

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <tip-5faeb8adb956a5ad6579c4e309e8689943ad8294@git.kernel.org>
Date:	Tue, 28 Jan 2014 11:23:07 -0800
From:	tip-bot for Jason Low <tipbot@...or.com>
To:	linux-tip-commits@...r.kernel.org
Cc:	linux-kernel@...r.kernel.org, hpa@...or.com, mingo@...nel.org,
	torvalds@...ux-foundation.org, peterz@...radead.org,
	tim.c.chen@...ux.intel.com, jason.low2@...com,
	paulmck@...ux.vnet.ibm.com, akpm@...ux-foundation.org,
	tglx@...utronix.de
Subject: [tip:core/locking] locking/mcs: Micro-optimize the MCS code,
  add extra comments

Commit-ID:  5faeb8adb956a5ad6579c4e309e8689943ad8294
Gitweb:     http://git.kernel.org/tip/5faeb8adb956a5ad6579c4e309e8689943ad8294
Author:     Jason Low <jason.low2@...com>
AuthorDate: Tue, 21 Jan 2014 15:36:05 -0800
Committer:  Ingo Molnar <mingo@...nel.org>
CommitDate: Tue, 28 Jan 2014 13:13:28 +0100

locking/mcs: Micro-optimize the MCS code, add extra comments

Remove unnecessary operation to assign locked status to 1 if lock is
acquired without contention. Lock status will not be checked by lock
holder again once it is acquired and any lock
contenders will not be looking at the lock holder's lock status.

Make the cmpxchg(lock, node, NULL) == node check in mcs_spin_unlock()
likely() as it is likely that a race did not occur most of the time.

Also add in more comments describing how the local node is used in MCS locks.

Reviewed-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
Reviewed-by: Tim Chen <tim.c.chen@...ux.intel.com>
Signed-off-by: Jason Low <jason.low2@...com>
Signed-off-by: Tim Chen <tim.c.chen@...ux.intel.com>
Signed-off-by: Peter Zijlstra <peterz@...radead.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Link: http://lkml.kernel.org/r/1390347365.3138.64.camel@schen9-DESK
Signed-off-by: Ingo Molnar <mingo@...nel.org>
---
 include/linux/mcs_spinlock.h | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/include/linux/mcs_spinlock.h b/include/linux/mcs_spinlock.h
index 9578ef8..143fa42 100644
--- a/include/linux/mcs_spinlock.h
+++ b/include/linux/mcs_spinlock.h
@@ -25,6 +25,17 @@ struct mcs_spinlock {
  * with mcs_unlock and mcs_lock pair, smp_mb__after_unlock_lock() should be
  * used after mcs_lock.
  */
+
+/*
+ * In order to acquire the lock, the caller should declare a local node and
+ * pass a reference of the node to this function in addition to the lock.
+ * If the lock has already been acquired, then this will proceed to spin
+ * on this node->locked until the previous lock holder sets the node->locked
+ * in mcs_spin_unlock().
+ *
+ * We don't inline mcs_spin_lock() so that perf can correctly account for the
+ * time spent in this lock function.
+ */
 static inline
 void mcs_spin_lock(struct mcs_spinlock **lock, struct mcs_spinlock *node)
 {
@@ -36,8 +47,14 @@ void mcs_spin_lock(struct mcs_spinlock **lock, struct mcs_spinlock *node)
 
 	prev = xchg(lock, node);
 	if (likely(prev == NULL)) {
-		/* Lock acquired */
-		node->locked = 1;
+		/*
+		 * Lock acquired, don't need to set node->locked to 1. Threads
+		 * only spin on its own node->locked value for lock acquisition.
+		 * However, since this thread can immediately acquire the lock
+		 * and does not proceed to spin on its own node->locked, this
+		 * value won't be used. If a debug mode is needed to
+		 * audit lock status, then set node->locked value here.
+		 */
 		return;
 	}
 	ACCESS_ONCE(prev->next) = node;
@@ -50,6 +67,10 @@ void mcs_spin_lock(struct mcs_spinlock **lock, struct mcs_spinlock *node)
 		arch_mutex_cpu_relax();
 }
 
+/*
+ * Releases the lock. The caller should pass in the corresponding node that
+ * was used to acquire the lock.
+ */
 static inline
 void mcs_spin_unlock(struct mcs_spinlock **lock, struct mcs_spinlock *node)
 {
@@ -59,7 +80,7 @@ void mcs_spin_unlock(struct mcs_spinlock **lock, struct mcs_spinlock *node)
 		/*
 		 * Release the lock by setting it to NULL
 		 */
-		if (cmpxchg(lock, node, NULL) == node)
+		if (likely(cmpxchg(lock, node, NULL) == node))
 			return;
 		/* Wait until the next pointer is set */
 		while (!(next = ACCESS_ONCE(node->next)))
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/