Merge pull request #2436 from hjelmn/v1.3.x_ugni

uct/ugni: fix internal locking

src/uct/ugni/base/ugni_def.h

@@ -1,5 +1,6 @@
 /**
  * Copyright (c) UT-Battelle, LLC. 2017. ALL RIGHTS RESERVED.
+ * Copyright (c) Los Alamos National Security, LLC. 2018. ALL RIGHTS RESERVED.
  * See file LICENSE for terms.
  */
 
@@ -45,23 +46,23 @@ do {\
 
 #if ENABLE_MT
 #define uct_ugni_check_lock_needed(_cdm) UCS_THREAD_MODE_MULTI == (_cdm)->thread_mode
-#define uct_ugni_device_init_lock(_dev) ucs_spinlock_init(&(_dev)->lock)
-#define uct_ugni_device_destroy_lock(_dev) ucs_spinlock_destroy(&(_dev)->lock)
-#define uct_ugni_device_lock(_cdm) \
+#define uct_ugni_cdm_init_lock(_cdm) ucs_spinlock_init(&(_cdm)->lock)
+#define uct_ugni_cdm_destroy_lock(_cdm) ucs_spinlock_destroy(&(_cdm)->lock)
+#define uct_ugni_cdm_lock(_cdm) \
 if (uct_ugni_check_lock_needed(_cdm)) {  \
     ucs_trace_async("Taking lock");      \
-    ucs_spin_lock(&(_cdm)->dev->lock);   \
+    ucs_spin_lock(&(_cdm)->lock);   \
 }
-#define uct_ugni_device_unlock(_cdm) \
+#define uct_ugni_cdm_unlock(_cdm) \
 if (uct_ugni_check_lock_needed(_cdm)) {    \
     ucs_trace_async("Releasing lock");        \
-    ucs_spin_unlock(&(_cdm)->dev->lock);   \
+    ucs_spin_unlock(&(_cdm)->lock);   \
 }
 #else
-#define uct_ugni_device_init_lock(x) UCS_OK
-#define uct_ugni_device_destroy_lock(x) UCS_OK
-#define uct_ugni_device_lock(x)
-#define uct_ugni_device_unlock(x)
+#define uct_ugni_cdm_init_lock(x) UCS_OK
+#define uct_ugni_cdm_destroy_lock(x) UCS_OK
+#define uct_ugni_cdm_lock(x)
+#define uct_ugni_cdm_unlock(x)
 #define uct_ugni_check_lock_needed(x) 0
 #endif
 

src/uct/ugni/base/ugni_device.c

@@ -1,6 +1,7 @@
 /**
  * Copyright (c) UT-Battelle, LLC. 2014-2017. ALL RIGHTS RESERVED.
  * Copyright (C) Mellanox Technologies Ltd. 2001-2014.  ALL RIGHTS RESERVED.
+ * Copyright (c) Los Alamos National Security, LLC. 2018. ALL RIGHTS RESERVED.
  * See file LICENSE for terms.
  */
 
@@ -370,22 +371,11 @@ ucs_status_t uct_ugni_device_create(int dev_id, int index, uct_ugni_device_t *de
     ucs_snprintf_zero(dev_p->fname, sizeof(dev_p->fname), "%s:%d",
                       dev_p->type_name, index);
 
-    status = uct_ugni_device_init_lock(dev_p);
-    if (UCS_OK != status) {
-        ucs_error("Couldn't initalize device lock.");
-        return status;
-    }
     return UCS_OK;
 }
 
 void uct_ugni_device_destroy(uct_ugni_device_t *dev)
 {
-    ucs_status_t status;
-
-    status = uct_ugni_device_destroy_lock(dev);
-    if (UCS_OK != status) {
-        ucs_error("Couldn't destroy device lock.");
-    }
 }
 
 ucs_status_t uct_ugni_iface_get_dev_address(uct_iface_t *tl_iface, uct_device_addr_t *addr)
@@ -413,7 +403,6 @@ ucs_status_t uct_ugni_create_cdm(uct_ugni_cdm_t *cdm, uct_ugni_device_t *device,
 
     cdm->thread_mode = thread_mode;
     cdm->dev = device;
-    uct_ugni_device_lock(cdm);
     cdm->domain_id = job_info->pmi_rank_id + job_info->pmi_num_of_ranks * ucs_atomic_fadd32(&ugni_domain_counter,1);
     ucs_debug("Creating new command domain with id %d (%d + %d * %d)",
               cdm->domain_id, job_info->pmi_rank_id,
@@ -425,8 +414,7 @@ ucs_status_t uct_ugni_create_cdm(uct_ugni_cdm_t *cdm, uct_ugni_device_t *device,
     if (GNI_RC_SUCCESS != ugni_rc) {
         ucs_error("GNI_CdmCreate failed, Error status: %s %d",
                   gni_err_str[ugni_rc], ugni_rc);
-        status = UCS_ERR_NO_DEVICE;
-        goto out_unlock;
+        return UCS_ERR_NO_DEVICE;
     }
 
     ugni_rc = GNI_CdmAttach(cdm->cdm_handle, device->device_id,
@@ -440,8 +428,11 @@ ucs_status_t uct_ugni_create_cdm(uct_ugni_cdm_t *cdm, uct_ugni_device_t *device,
         status = UCS_ERR_NO_DEVICE;
     }
 
-out_unlock:
-    uct_ugni_device_unlock(cdm);
+    status = uct_ugni_cdm_init_lock(cdm);
+    if (UCS_OK != status) {
+        ucs_error("Couldn't initalize CDM lock.");
+    }
+
     if (UCS_OK == status) {
         ucs_debug("Made ugni cdm. nic_addr = %i domain_id = %i", device->address, cdm->domain_id);
     }
@@ -456,11 +447,15 @@ ucs_status_t uct_ugni_create_md_cdm(uct_ugni_cdm_t *cdm)
 ucs_status_t uct_ugni_destroy_cdm(uct_ugni_cdm_t *cdm)
 {
     gni_return_t ugni_rc;
+    ucs_status_t status;
+
+    status = uct_ugni_cdm_destroy_lock(cdm);
+    if (UCS_OK != status) {
+        ucs_error("Couldn't destroy cdm lock.");
+    }
 
     ucs_trace_func("cdm=%p", cdm);
-    uct_ugni_device_lock(cdm);
     ugni_rc = GNI_CdmDestroy(cdm->cdm_handle);
-    uct_ugni_device_unlock(cdm);
     if (GNI_RC_SUCCESS != ugni_rc) {
         ucs_error("GNI_CdmDestroy error status: %s (%d)",
                  gni_err_str[ugni_rc], ugni_rc);
@@ -473,11 +468,9 @@ ucs_status_t uct_ugni_create_cq(gni_cq_handle_t *cq, unsigned cq_size, uct_ugni_
 {
     gni_return_t ugni_rc;
 
-    uct_ugni_device_lock(cdm);
     ugni_rc = GNI_CqCreate(cdm->nic_handle, UCT_UGNI_LOCAL_CQ, 0,
                            GNI_CQ_NOBLOCK,
                            NULL, NULL, cq);
-    uct_ugni_device_unlock(cdm);
     if (GNI_RC_SUCCESS != ugni_rc) {
         ucs_error("GNI_CqCreate failed, Error status: %s %d",
                   gni_err_str[ugni_rc], ugni_rc);
@@ -491,9 +484,7 @@ ucs_status_t uct_ugni_destroy_cq(gni_cq_handle_t cq, uct_ugni_cdm_t *cdm)
 {
     gni_return_t ugni_rc;
 
-    uct_ugni_device_lock(cdm);
     ugni_rc = GNI_CqDestroy(cq);
-    uct_ugni_device_unlock(cdm);
     if (GNI_RC_SUCCESS != ugni_rc) {
         ucs_warn("GNI_CqDestroy failed, Error status: %s %d",
                  gni_err_str[ugni_rc], ugni_rc);

src/uct/ugni/base/ugni_ep.c

@@ -162,13 +162,13 @@ ucs_status_t ugni_connect_ep(uct_ugni_iface_t *iface,
                              uct_ugni_ep_t *ep){
     gni_return_t ugni_rc;
 
-    uct_ugni_device_lock(&iface->cdm);
+    uct_ugni_cdm_lock(&iface->cdm);
     ugni_rc = GNI_EpBind(ep->ep, dev_addr->nic_addr, iface_addr->domain_id);
-    uct_ugni_device_unlock(&iface->cdm);
+    uct_ugni_cdm_unlock(&iface->cdm);
     if (GNI_RC_SUCCESS != ugni_rc) {
-        uct_ugni_device_lock(&iface->cdm);
+        uct_ugni_cdm_lock(&iface->cdm);
         (void)GNI_EpDestroy(ep->ep);
-        uct_ugni_device_unlock(&iface->cdm);
+        uct_ugni_cdm_unlock(&iface->cdm);
         ucs_error("GNI_EpBind failed, Error status: %s %d",
                   gni_err_str[ugni_rc], ugni_rc);
         return UCS_ERR_UNREACHABLE;
@@ -200,9 +200,9 @@ UCS_CLASS_INIT_FUNC(uct_ugni_ep_t, uct_iface_t *tl_iface,
     self->flush_group->flush_comp.func = NULL;
     self->flush_group->parent = NULL;
 #endif
-    uct_ugni_device_lock(&iface->cdm);
+    uct_ugni_cdm_lock(&iface->cdm);
     ugni_rc = GNI_EpCreate(uct_ugni_iface_nic_handle(iface), iface->local_cq, &self->ep);
-    uct_ugni_device_unlock(&iface->cdm);
+    uct_ugni_cdm_unlock(&iface->cdm);
     if (GNI_RC_SUCCESS != ugni_rc) {
         ucs_error("GNI_CdmCreate failed, Error status: %s %d",
                   gni_err_str[ugni_rc], ugni_rc);
@@ -235,9 +235,9 @@ static UCS_CLASS_CLEANUP_FUNC(uct_ugni_ep_t)
 
     ucs_arbiter_group_purge(&iface->arbiter, &self->arb_group,
                             uct_ugni_ep_abriter_purge_cb, NULL);
-    uct_ugni_device_lock(&iface->cdm);
+    uct_ugni_cdm_lock(&iface->cdm);
     ugni_rc = GNI_EpDestroy(self->ep);
-    uct_ugni_device_unlock(&iface->cdm);
+    uct_ugni_cdm_unlock(&iface->cdm);
     if (GNI_RC_SUCCESS != ugni_rc) {
         ucs_warn("GNI_EpDestroy failed, Error status: %s %d",
                   gni_err_str[ugni_rc], ugni_rc);

src/uct/ugni/base/ugni_md.c

@@ -64,12 +64,12 @@ static ucs_status_t uct_ugni_mem_reg(uct_md_h md, void *address, size_t length,
         goto mem_err;
     }
 
-    uct_ugni_device_lock(&ugni_md->cdm);
+    uct_ugni_cdm_lock(&ugni_md->cdm);
     ugni_rc = GNI_MemRegister(ugni_md->cdm.nic_handle, (uint64_t)address,
                               length, NULL,
                               GNI_MEM_READWRITE | GNI_MEM_RELAXED_PI_ORDERING,
                               -1, mem_hndl);
-    uct_ugni_device_unlock(&ugni_md->cdm);
+    uct_ugni_cdm_unlock(&ugni_md->cdm);
     if (GNI_RC_SUCCESS != ugni_rc) {
         ucs_error("GNI_MemRegister failed (addr %p, size %zu), Error status: %s %d",
                  address, length, gni_err_str[ugni_rc], ugni_rc);
@@ -94,9 +94,9 @@ static ucs_status_t uct_ugni_mem_dereg(uct_md_h md, uct_mem_h memh)
     gni_return_t ugni_rc;
     ucs_status_t status = UCS_OK;
 
-    uct_ugni_device_lock(&ugni_md->cdm);
+    uct_ugni_cdm_lock(&ugni_md->cdm);
     ugni_rc = GNI_MemDeregister(ugni_md->cdm.nic_handle, mem_hndl);
-    uct_ugni_device_unlock(&ugni_md->cdm);
+    uct_ugni_cdm_unlock(&ugni_md->cdm);
     if (GNI_RC_SUCCESS != ugni_rc) {
         ucs_error("GNI_MemDeregister failed, Error status: %s %d",
                  gni_err_str[ugni_rc], ugni_rc);

src/uct/ugni/base/ugni_types.h

@@ -1,5 +1,6 @@
 /**
  * Copyright (c) UT-Battelle, LLC. 2014-2017. ALL RIGHTS RESERVED.
+ * Copyright (c) Los Alamos National Security, LLC. 2018. ALL RIGHTS RESERVED.
  * See file LICENSE for terms.
  */
 
@@ -21,9 +22,6 @@ typedef struct uct_ugni_device {
     uint32_t         cpu_id;                    /**< CPU attached directly
                                                   to the device */
     cpu_set_t        cpu_mask;                  /**< CPU mask */
-#if ENABLE_MT
-    ucs_spinlock_t   lock;                      /**< Device lock */
-#endif
     /* TBD - reference counter */
 } uct_ugni_device_t;
 
@@ -34,6 +32,10 @@ typedef struct uct_ugni_cdm {
     ucs_thread_mode_t  thread_mode;
     uint32_t           address; 
     uint16_t           domain_id;
+
+#if ENABLE_MT
+    ucs_spinlock_t   lock;                      /**< Device lock */
+#endif
 } uct_ugni_cdm_t;
 
 /**

src/uct/ugni/rdma/ugni_rdma_ep.c

@@ -94,9 +94,9 @@ static inline ucs_status_t uct_ugni_post_rdma(uct_ugni_rdma_iface_t *iface,
         ucs_mpool_put(rdma);
         return UCS_ERR_NO_RESOURCE;
     }
-    uct_ugni_device_lock(&iface->super.cdm);
+    uct_ugni_cdm_lock(&iface->super.cdm);
     ugni_rc = GNI_PostRdma(ep->ep, &rdma->desc);
-    uct_ugni_device_unlock(&iface->super.cdm);
+    uct_ugni_cdm_unlock(&iface->super.cdm);
     if (ucs_unlikely(GNI_RC_SUCCESS != ugni_rc)) {
         ucs_mpool_put(rdma);
         if(GNI_RC_ERROR_RESOURCE == ugni_rc || GNI_RC_ERROR_NOMEM == ugni_rc) {
@@ -127,9 +127,9 @@ static inline ssize_t uct_ugni_post_fma(uct_ugni_rdma_iface_t *iface,
         ucs_mpool_put(fma);
         return UCS_ERR_NO_RESOURCE;
     }
-    uct_ugni_device_lock(&iface->super.cdm);
+    uct_ugni_cdm_lock(&iface->super.cdm);
     ugni_rc = GNI_PostFma(ep->ep, &fma->desc);
-    uct_ugni_device_unlock(&iface->super.cdm);
+    uct_ugni_cdm_unlock(&iface->super.cdm);
     if (ucs_unlikely(GNI_RC_SUCCESS != ugni_rc)) {
         ucs_mpool_put(fma);
         if(GNI_RC_ERROR_RESOURCE == ugni_rc || GNI_RC_ERROR_NOMEM == ugni_rc) {

src/uct/ugni/rdma/ugni_rdma_iface.c

@@ -114,21 +114,21 @@ unsigned uct_ugni_progress(void *arg)
     unsigned count = 0;
 
     while (1) {
-        uct_ugni_device_lock(&iface->cdm);
+        uct_ugni_cdm_lock(&iface->cdm);
         ugni_rc = GNI_CqGetEvent(iface->local_cq, &event_data);
         if (GNI_RC_NOT_DONE == ugni_rc) {
-            uct_ugni_device_unlock(&iface->cdm);
+            uct_ugni_cdm_unlock(&iface->cdm);
             break;
         }
         if ((GNI_RC_SUCCESS != ugni_rc && !event_data) || GNI_CQ_OVERRUN(event_data)) {
-            uct_ugni_device_unlock(&iface->cdm);
+            uct_ugni_cdm_unlock(&iface->cdm);
             ucs_error("GNI_CqGetEvent falied. Error status %s %d ",
                       gni_err_str[ugni_rc], ugni_rc);
             return count;
         }
 
         ugni_rc = GNI_GetCompleted(iface->local_cq, event_data, &event_post_desc_ptr);
-        uct_ugni_device_unlock(&iface->cdm);
+        uct_ugni_cdm_unlock(&iface->cdm);
         if (GNI_RC_SUCCESS != ugni_rc && GNI_RC_TRANSACTION_ERROR != ugni_rc) {
             ucs_error("GNI_GetCompleted falied. Error status %s %d",
                       gni_err_str[ugni_rc], ugni_rc);

src/uct/ugni/smsg/ugni_smsg_ep.c

@@ -52,12 +52,12 @@ static ucs_status_t uct_ugni_smsg_mbox_reg(uct_ugni_smsg_iface_t *iface, uct_ugn
         return UCS_ERR_INVALID_PARAM;
     }
 
-    uct_ugni_device_lock(&iface->super.cdm);
+    uct_ugni_cdm_lock(&iface->super.cdm);
     ugni_rc = GNI_MemRegister(uct_ugni_iface_nic_handle(&iface->super), (uint64_t)address,
                               iface->bytes_per_mbox, iface->remote_cq,
                               GNI_MEM_READWRITE,
                               -1, &(mbox->gni_mem));
-    uct_ugni_device_unlock(&iface->super.cdm);
+    uct_ugni_cdm_unlock(&iface->super.cdm);
     if (GNI_RC_SUCCESS != ugni_rc) {
         ucs_error("GNI_MemRegister failed (addr %p, size %zu), Error status: %s %d",
                   address, iface->bytes_per_mbox, gni_err_str[ugni_rc], ugni_rc);
@@ -72,9 +72,9 @@ static ucs_status_t uct_ugni_smsg_mbox_reg(uct_ugni_smsg_iface_t *iface, uct_ugn
 static ucs_status_t uct_ugni_smsg_mbox_dereg(uct_ugni_smsg_iface_t *iface, uct_ugni_smsg_mbox_t *mbox){
     gni_return_t ugni_rc;
 
-    uct_ugni_device_lock(&iface->super.cdm);
+    uct_ugni_cdm_lock(&iface->super.cdm);
     ugni_rc = GNI_MemDeregister(uct_ugni_iface_nic_handle(&iface->super), &mbox->gni_mem);
-    uct_ugni_device_unlock(&iface->super.cdm);
+    uct_ugni_cdm_unlock(&iface->super.cdm);
 
     if (GNI_RC_SUCCESS != ugni_rc) {
         ucs_error("GNI_MemDeregister failed Error status: %s %d",
@@ -162,9 +162,9 @@ ucs_status_t uct_ugni_smsg_ep_connect_to_ep(uct_ep_h tl_ep,
         ucs_error("Could not connect ep in smsg");
         return rc;
     }
-    uct_ugni_device_lock(&iface->cdm);
+    uct_ugni_cdm_lock(&iface->cdm);
     gni_rc = GNI_SmsgInit(ep->super.ep, local_attr, &remote_attr);
-    uct_ugni_device_unlock(&iface->cdm);
+    uct_ugni_cdm_unlock(&iface->cdm);
 
     if(GNI_RC_SUCCESS != gni_rc){
         ucs_error("Failed to initalize smsg. %s [%i]", gni_err_str[gni_rc], gni_rc);
@@ -176,9 +176,9 @@ ucs_status_t uct_ugni_smsg_ep_connect_to_ep(uct_ep_h tl_ep,
     }
 
     ep_hash = (uint32_t)iface_addr->ep_hash;
-    uct_ugni_device_lock(&iface->cdm);
+    uct_ugni_cdm_lock(&iface->cdm);
     gni_rc = GNI_EpSetEventData(ep->super.ep, iface->cdm.domain_id, ep_hash);
-    uct_ugni_device_unlock(&iface->cdm);
+    uct_ugni_cdm_unlock(&iface->cdm);
 
     if(GNI_RC_SUCCESS != gni_rc){
         ucs_error("Could not set GNI_EpSetEventData!");
@@ -199,10 +199,10 @@ uct_ugni_smsg_ep_am_common_send(uct_ugni_smsg_ep_t *ep, uct_ugni_smsg_iface_t *i
 
     desc->msg_id = iface->smsg_id++;
     desc->flush_group = ep->super.flush_group;
-    uct_ugni_device_lock(&iface->super.cdm);
+    uct_ugni_cdm_lock(&iface->super.cdm);
     gni_rc = GNI_SmsgSendWTag(ep->super.ep, header, header_length, 
                               payload, payload_length, desc->msg_id, am_id);
-    uct_ugni_device_unlock(&iface->super.cdm);
+    uct_ugni_cdm_unlock(&iface->super.cdm);
     if(GNI_RC_SUCCESS != gni_rc){
         goto exit_no_res;
     }