From 64f52b0168a80f5e6c182ad3fc9f8ade58d2c1fe Mon Sep 17 00:00:00 2001
From: Eugene Loh <eugene.loh@oracle.com>
Date: Fri, 13 Mar 2009 18:11:41 +0000
Subject: [PATCH] Clean up in response to code review on CMR 1825: minor
 changes in comments and edge-case handling.

This commit was SVN r20774.
---
 ompi/mca/btl/sm/btl_sm.c               | 18 ++++++++++++++---
 ompi/mca/btl/sm/btl_sm.h               | 28 ++++++++++++++++++++++++--
 ompi/mca/mpool/sm/mpool_sm_component.c |  3 +++
 3 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/ompi/mca/btl/sm/btl_sm.c b/ompi/mca/btl/sm/btl_sm.c
index 2741162dd7..a14b4767e9 100644
--- a/ompi/mca/btl/sm/btl_sm.c
+++ b/ompi/mca/btl/sm/btl_sm.c
@@ -193,9 +193,21 @@ static int sm_btl_first_time_init(mca_btl_sm_t *sm_btl, int n)
         res.mem_node = (num_mem_nodes == 1) ? -1 : i;
 
         /* determine how much memory to create */
-        res.size = m->nfifos * ( sizeof(sm_fifo_t) + sizeof(void *) * m->fifo_size )
-            + ( 2 * n + m->sm_free_list_inc ) * ( m->eager_limit   + CACHE_LINE_SIZE )
-            +           m->sm_free_list_num   * ( m->max_frag_size + CACHE_LINE_SIZE );
+        /*
+         * This heuristic formula mostly says that we request memory for:
+         * - nfifos FIFOs, each comprising:
+         *   . a sm_fifo_t structure
+         *   . many pointers (fifo_size of them per FIFO)
+         * - eager fragments (2*n of them, allocated in sm_free_list_inc chunks)
+         * - max fragments (sm_free_list_num of them)
+         *
+         * On top of all that, we sprinkle in some number of "CACHE_LINE_SIZE"
+         * additions to account for some padding and edge effects that may lie
+         * in the allocator.
+         */
+        res.size = m->nfifos * ( sizeof(sm_fifo_t) + sizeof(void *) * m->fifo_size + 4 * CACHE_LINE_SIZE )
+            + ( 2 * n + m->sm_free_list_inc ) * ( m->eager_limit   + 2 * CACHE_LINE_SIZE )
+            +           m->sm_free_list_num   * ( m->max_frag_size + 2 * CACHE_LINE_SIZE );
         if ( ((double) res.size) * n > LONG_MAX )
             res.size  = LONG_MAX;
         else
diff --git a/ompi/mca/btl/sm/btl_sm.h b/ompi/mca/btl/sm/btl_sm.h
index 8fcd5fcb8d..c7f7f0cc16 100644
--- a/ompi/mca/btl/sm/btl_sm.h
+++ b/ompi/mca/btl/sm/btl_sm.h
@@ -55,8 +55,34 @@ extern "C" {
 
 /*
  * Shared Memory FIFOs
+ *
+ * The FIFO is implemented as a circular queue with head and tail pointers
+ * (integer indices).  For efficient wraparound indexing, the size of the
+ * queue is constrained to be a power of two and we "&" indices with a "mask".
+ *
+ * More than one process can write to the FIFO head.  Therefore, there is a head
+ * lock.  One cannot write until the head slot is empty, indicated by the special
+ * queue entry SM_FIFO_FREE.
+ *
+ * Only the receiver can read the FIFO tail.  Therefore, the tail lock is
+ * required only in multithreaded applications.  If a tail read returns the
+ * SM_FIFO_FREE value, that means the FIFO is empty.  Once a non-FREE value
+ * has been read, the queue slot is *not* automatically reset to SM_FIFO_FREE.
+ * Rather, read tail slots are reset "lazily" (see "lazy_free" and "num_to_clear")
+ * to reduce the number of memory barriers and improve performance.
+ *
+ * Since the FIFO lives in shared memory that is mapped differently into
+ * each address space, the "queue" pointer is relative (each process must
+ * add its own offset) and the queue_recv pointer is meaningful only in the
+ * receiver's address space.
+ *
+ * Since multiple processes access different parts of the FIFO structure in
+ * different ways, we introduce padding to keep different parts on different
+ * cachelines.
  */
 
+#define SM_FIFO_FREE  (void *) (-2)
+
 struct sm_fifo_t {
     /* This queue pointer is used only by the heads. */
     volatile void **queue;           char pad0[CACHE_LINE_SIZE - sizeof(void **)           ];
@@ -173,8 +199,6 @@ typedef struct btl_sm_pending_send_item_t btl_sm_pending_send_item_t;
 /* ================================================== */
 /* ================================================== */
 
-#define SM_FIFO_FREE  (void *) (-2)
-
 static inline int sm_fifo_init(int fifo_size, mca_mpool_base_module_t *mpool,
                                sm_fifo_t *fifo, int lazy_free)
 {
diff --git a/ompi/mca/mpool/sm/mpool_sm_component.c b/ompi/mca/mpool/sm/mpool_sm_component.c
index ddcd7a347d..64983279f8 100644
--- a/ompi/mca/mpool/sm/mpool_sm_component.c
+++ b/ompi/mca/mpool/sm/mpool_sm_component.c
@@ -184,6 +184,9 @@ static mca_mpool_base_module_t* mca_mpool_sm_init(
     if ( mca_mpool_sm_component.sm_size < min_size )
          mca_mpool_sm_component.sm_size = min_size;
 
+    /* add something for the control structure */
+    mca_mpool_sm_component.sm_size += sizeof(mca_common_sm_mmap_t);
+
     allocator_component = mca_allocator_component_lookup(
         mca_mpool_sm_component.sm_allocator_name);