From c8e91d5798d041767b50af2055d7b25af30a3263 Mon Sep 17 00:00:00 2001
From: Adam Langley <agl@chromium.org>
Date: Fri, 20 Jun 2014 12:00:00 -0700
Subject: [PATCH] bignum: allow concurrent BN_MONT_CTX_set_locked()

The lazy-initialisation of BN_MONT_CTX was serialising all threads, as noted by
Daniel Sands and co at Sandia. This was to handle the case that 2 or more
threads race to lazy-init the same context, but stunted all scalability in the
case where 2 or more threads are doing unrelated things! We favour the latter
case by punishing the former. The init work gets done by each thread that finds
the context to be uninitialised, and we then lock the "set" logic after that
work is done - the winning thread's work gets used, the losing threads throw
away what they've done.

(Imported from upstream's bf43446835bfd3f9abf1898a99ae20f2285320f3)
---
 crypto/bn/montgomery.c | 50 ++++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/crypto/bn/montgomery.c b/crypto/bn/montgomery.c
index 4c803d2d..ddd2b26c 100644
--- a/crypto/bn/montgomery.c
+++ b/crypto/bn/montgomery.c
@@ -292,33 +292,41 @@ err:
 
 BN_MONT_CTX *BN_MONT_CTX_set_locked(BN_MONT_CTX **pmont, int lock,
                                     const BIGNUM *mod, BN_CTX *ctx) {
-  int got_write_lock = 0;
   BN_MONT_CTX *ret;
 
   CRYPTO_r_lock(lock);
-  if (!*pmont) {
-    CRYPTO_r_unlock(lock);
-    CRYPTO_w_lock(lock);
-    got_write_lock = 1;
-
-    if (!*pmont) {
-      ret = BN_MONT_CTX_new();
-      if (ret && !BN_MONT_CTX_set(ret, mod, ctx)) {
-        BN_MONT_CTX_free(ret);
-      } else {
-        *pmont = ret;
-      }
-    }
-  }
-
   ret = *pmont;
-
-  if (got_write_lock) {
-    CRYPTO_w_unlock(lock);
-  } else {
-    CRYPTO_r_unlock(lock);
+  CRYPTO_r_unlock(lock);
+  if (ret) {
+    return ret;
   }
 
+  /* We don't want to serialise globally while doing our lazy-init math in
+   * BN_MONT_CTX_set. That punishes threads that are doing independent
+   * things. Instead, punish the case where more than one thread tries to
+   * lazy-init the same 'pmont', by having each do the lazy-init math work
+   * independently and only use the one from the thread that wins the race
+   * (the losers throw away the work they've done). */
+  ret = BN_MONT_CTX_new();
+  if (!ret) {
+    return NULL;
+  }
+  if (!BN_MONT_CTX_set(ret, mod, ctx)) {
+    BN_MONT_CTX_free(ret);
+    return NULL;
+  }
+
+  /* The locked compare-and-set, after the local work is done. */
+  CRYPTO_w_lock(lock);
+  if (*pmont) {
+    BN_MONT_CTX_free(ret);
+    ret = *pmont;
+  } else {
+    *pmont = ret;
+  }
+
+  CRYPTO_w_unlock(lock);
+
   return ret;
 }