From: Jonas Oreland
Date: March 21 2012 9:26am
Subject: bzr push into mysql-5.1-telco-7.0 branch (jonas.oreland:4895 to 4896)
 Bug#13869781
List-Archive: http://lists.mysql.com/commits/143256
X-Bug: 13869781
Message-Id: <20120321092659.009EC55C8EA@perch.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

 4896 Jonas Oreland	2012-03-21
      ndb - bug#13869781 - fix missing barrier in mt.cpp causing (very) spurious hangs

    modified:
      storage/ndb/src/kernel/vm/mt.cpp
 4895 Jonas Oreland	2012-03-21
      ndb - add forceSend to mt-send-t

    modified:
      storage/ndb/src/kernel/vm/mt-send-t.cpp
=== modified file 'storage/ndb/src/kernel/vm/mt.cpp'
--- a/storage/ndb/src/kernel/vm/mt.cpp	2012-03-20 13:23:29 +0000
+++ b/storage/ndb/src/kernel/vm/mt.cpp	2012-03-21 09:26:18 +0000
@@ -2584,6 +2584,13 @@ pack_send_buffer(thr_data *selfptr, Uint
    * After having locked/unlock m_send_lock
    *   "protocol" dictates that we must check the m_force_send
    */
+
+  /**
+   * We need a memory barrier here to prevent race between clearing lock
+   *   and reading of m_force_send.
+   *   CPU can reorder the load to before the clear of the lock
+   */
+  mb();
   if (sb->m_force_send)
   {
     try_send(selfptr, node);
@@ -2652,7 +2659,13 @@ mt_send_handle::forceSend(NodeId nodeId)
 
   do
   {
+    /**
+     * NOTE: we don't need a memory barrier after clearing
+     *       m_force_send here as we unconditionally lock m_send_lock
+     *       hence there is no way that our data can be "unsent"
+     */
     sb->m_force_send = 0;
+
     lock(&sb->m_send_lock);
     sb->m_send_thread = selfptr->m_thr_no;
     globalTransporterRegistry.performSend(nodeId);
@@ -2664,6 +2677,12 @@ mt_send_handle::forceSend(NodeId nodeId)
      */
     selfptr->m_send_buffer_pool.release_global(rep->m_mm,
                                                RG_TRANSPORTER_BUFFERS);
+    /**
+     * We need a memory barrier here to prevent race between clearing lock
+     *   and reading of m_force_send.
+     *   CPU can reorder the load to before the clear of the lock
+     */
+    mb();
   } while (sb->m_force_send);
 
   return true;
@@ -2686,6 +2705,16 @@ try_send(thr_data * selfptr, Uint32 node
       return;
     }
 
+    /**
+     * Now clear the flag, and start sending all data available to this node.
+     *
+     * Put a memory barrier here, so that if another thread tries to grab
+     * the send lock but fails due to us holding it here, we either
+     * 1) Will see m_force_send[nodeId] set to 1 at the end of the loop, or
+     * 2) We clear here the flag just set by the other thread, but then we
+     * will (thanks to mb()) be able to see and send all of the data already
+     * in the first send iteration.
+     */
     sb->m_force_send = 0;
     mb();
 
@@ -2699,6 +2728,13 @@ try_send(thr_data * selfptr, Uint32 node
      */
     selfptr->m_send_buffer_pool.release_global(rep->m_mm,
                                                RG_TRANSPORTER_BUFFERS);
+
+    /**
+     * We need a memory barrier here to prevent race between clearing lock
+     *   and reading of m_force_send.
+     *   CPU can reorder the load to before the clear of the lock
+     */
+    mb();
   } while (sb->m_force_send);
 }
 
@@ -2831,6 +2867,13 @@ do_send(struct thr_data* selfptr, bool m
       {
         register_pending_send(selfptr, node);
       }
+
+      /**
+       * We need a memory barrier here to prevent race between clearing lock
+       *   and reading of m_force_send.
+       *   CPU can reorder the load to before the clear of the lock
+       */
+      mb();
       if (sb->m_force_send)
       {
         /**

No bundle (reason: useless for push emails).