REVIEW: address feedback (remove API changes)

src/components/tl/ucp/tl_ucp.c

@@ -189,11 +189,6 @@ ucc_config_field_t ucc_tl_ucp_lib_config_table[] = {
      ucc_offsetof(ucc_tl_ucp_lib_config_t, use_reordering),
      UCC_CONFIG_TYPE_BOOL},
 
-    {"USE_XGVMI", "n",
-     "Use XGVMI for onesided collectives",
-     ucc_offsetof(ucc_tl_ucp_lib_config_t, use_xgvmi),
-     UCC_CONFIG_TYPE_BOOL},
-
     {NULL}};
 
 static ucs_config_field_t ucc_tl_ucp_context_config_table[] = {

src/components/tl/ucp/tl_ucp.h

@@ -80,7 +80,6 @@ typedef struct ucc_tl_ucp_lib_config {
     uint32_t                 alltoallv_hybrid_pairwise_num_posts;
     ucc_ternary_auto_value_t use_topo;
     int                      use_reordering;
-    int                      use_xgvmi;
 } ucc_tl_ucp_lib_config_t;
 
 typedef struct ucc_tl_ucp_context_config {
@@ -107,7 +106,6 @@ typedef struct ucc_tl_ucp_remote_info {
     void * va_base;
     size_t len;
     void * mem_h;
-    void * packed_memh;
     void * packed_key;
     size_t packed_key_len;
 } ucc_tl_ucp_remote_info_t;

src/components/tl/ucp/tl_ucp_coll.c

@@ -193,26 +193,16 @@ ucc_status_t ucc_tl_ucp_memmap_segment(ucc_tl_ucp_task_t *task,
     ucp_mem_h             mh;
 
     /* map the memory */
-    if (map->resource != NULL) {
-        mmap_params.field_mask = UCP_MEM_MAP_PARAM_FIELD_EXPORTED_MEMH_BUFFER;
-        mmap_params.exported_memh_buffer               = map->resource;
-        tl_ctx->dynamic_remote_info[segid].packed_memh = map->resource;
-    } else {
-        mmap_params.field_mask =
-            UCP_MEM_MAP_PARAM_FIELD_ADDRESS | UCP_MEM_MAP_PARAM_FIELD_LENGTH;
-        mmap_params.address                            = map->address;
-        mmap_params.length                             = map->len;
-        tl_ctx->dynamic_remote_info[segid].packed_memh = NULL;
-    }
-    /* map exported memory handle */
+    mmap_params.field_mask =
+        UCP_MEM_MAP_PARAM_FIELD_ADDRESS | UCP_MEM_MAP_PARAM_FIELD_LENGTH;
+    mmap_params.address                            = map->address;
+    mmap_params.length                             = map->len;
+
     ucs_status = ucp_mem_map(tl_ctx->worker.ucp_context, &mmap_params, &mh);
-    if (ucs_status == UCS_ERR_UNREACHABLE) {
-        tl_error(tl_ctx->super.super.lib, "exported memh is unsupported");
-        return UCC_ERR_MEM_MAP_FAILURE;
-    } else if (ucs_status < UCS_OK) {
+    if (ucs_status < UCS_OK) {
         tl_error(tl_ctx->super.super.lib,
                  "ucp_mem_map failed with error code: %d", ucs_status);
-        return UCC_ERR_MEM_MAP_FAILURE;
+        return ucs_status_to_ucc_status(ucs_status);
     }
     /* generate rkeys / packed keys */
     tl_ctx->dynamic_remote_info[segid].va_base = map->address;
@@ -237,7 +227,6 @@ ucc_status_t ucc_tl_ucp_coll_dynamic_segment_init(ucc_coll_args_t   *coll_args,
     ucc_tl_ucp_team_t    *tl_team    = UCC_TL_UCP_TASK_TEAM(task);
     ucc_tl_ucp_context_t *ctx        = UCC_TL_UCP_TEAM_CTX(tl_team);
     int                   i          = 0;
-    ucc_mem_map_t        *maps       = coll_args->mem_map.segments;
     ucc_mem_map_t        *seg_maps   = NULL;
     size_t                n_segments = 3;
     uint64_t              need_map   = UCC_TL_UCP_DYN_SEG_UPDATE_SRC |
@@ -286,18 +275,17 @@ ucc_status_t ucc_tl_ucp_coll_dynamic_segment_init(ucc_coll_args_t   *coll_args,
             seg_maps[index].address = coll_args->src.info.buffer;
             seg_maps[index].len = (coll_args->src.info.count) *
                           ucc_dt_size(coll_args->src.info.datatype);
-            seg_maps[index++].resource = NULL;
+            ++index;
         }
         if (need_map & UCC_TL_UCP_DYN_SEG_UPDATE_DST) {
             seg_maps[index].address = coll_args->dst.info.buffer;
             seg_maps[index].len = (coll_args->dst.info.count) *
                           ucc_dt_size(coll_args->dst.info.datatype);
-            seg_maps[index++].resource = NULL;
+            ++index;
         }
         if (need_map & UCC_TL_UCP_DYN_SEG_UPDATE_GLOBAL) {
             seg_maps[index].address = coll_args->global_work_buffer;
             seg_maps[index].len = (ONESIDED_SYNC_SIZE + ONESIDED_REDUCE_SIZE) * sizeof(long);
-            seg_maps[index++].resource = NULL;
         }
     }
 
@@ -318,14 +306,6 @@ ucc_status_t ucc_tl_ucp_coll_dynamic_segment_init(ucc_coll_args_t   *coll_args,
             }
             ++ctx->n_dynrinfo_segs;
         }
-        for (i = 0; i < coll_args->mem_map.n_segments; i++) {
-            status = ucc_tl_ucp_memmap_segment(task, &maps[i], i + n_segments);
-            if (status != UCC_OK) {
-                tl_error(UCC_TASK_LIB(task), "failed to memory map a segment");
-                goto failed_memory_map;
-            }
-            ++ctx->n_dynrinfo_segs;
-        }
         if (n_segments) {
             free(seg_maps);
         }
@@ -343,9 +323,6 @@ ucc_status_t ucc_tl_ucp_coll_dynamic_segment_init(ucc_coll_args_t   *coll_args,
         if (ctx->dynamic_remote_info[i].packed_key) {
             ucp_rkey_buffer_release(ctx->dynamic_remote_info[i].packed_key);
         }
-        if (ctx->dynamic_remote_info[i].packed_memh) {
-            ucp_rkey_buffer_release(ctx->dynamic_remote_info[i].packed_memh);
-        }
     }
     ctx->n_dynrinfo_segs = 0;
     if (n_segments) {
@@ -488,10 +465,6 @@ ucc_status_t ucc_tl_ucp_coll_dynamic_segment_finalize(ucc_tl_ucp_task_t *task)
             if (ctx->dynamic_remote_info[i].packed_key) {
                 ucp_rkey_buffer_release(ctx->dynamic_remote_info[i].packed_key);
             }
-            if (ctx->dynamic_remote_info[i].packed_memh) {
-                ucp_rkey_buffer_release(
-                    ctx->dynamic_remote_info[i].packed_memh);
-            }
         }
         /* destroy rkeys */
         for (i = 0; i < UCC_TL_TEAM_SIZE(tl_team); i++) {

src/components/tl/ucp/tl_ucp_context.c

@@ -166,9 +166,6 @@ UCC_CLASS_INIT_FUNC(ucc_tl_ucp_context_t,
                             UCP_PARAM_FIELD_NAME;
     ucp_params.features =
         UCP_FEATURE_TAG | UCP_FEATURE_AM | UCP_FEATURE_RMA | UCP_FEATURE_AMO64;
-    if (lib->cfg.use_xgvmi) {
-        ucp_params.features |= UCP_FEATURE_EXPORTED_MEMH;
-    }
     ucp_params.tag_sender_mask = UCC_TL_UCP_TAG_SENDER_MASK;
     ucp_params.name            = "UCC_UCP_CONTEXT";
 
@@ -364,8 +361,8 @@ ucc_status_t ucc_tl_ucp_rinfo_destroy(ucc_tl_ucp_context_t *ctx)
     }
     ucc_free(ctx->remote_info);
     ucc_free(ctx->rkeys);
-    ctx->remote_info         = NULL;
-    ctx->rkeys               = NULL;
+    ctx->remote_info = NULL;
+    ctx->rkeys       = NULL;
 
     return UCC_OK;
 }
@@ -493,28 +490,17 @@ ucc_status_t ucc_tl_ucp_ctx_remote_populate(ucc_tl_ucp_context_t * ctx,
     }
 
     for (i = 0; i < nsegs; i++) {
-        if (lib->cfg.use_xgvmi == 0 ||
-            (lib->cfg.use_xgvmi == 1 && map.segments[i].resource == NULL)) {
-            mmap_params.field_mask = UCP_MEM_MAP_PARAM_FIELD_ADDRESS |
-                                     UCP_MEM_MAP_PARAM_FIELD_LENGTH;
-            mmap_params.address = map.segments[i].address;
-            mmap_params.length  = map.segments[i].len;
-        } else {
-            mmap_params.field_mask =
-                UCP_MEM_MAP_PARAM_FIELD_EXPORTED_MEMH_BUFFER;
-            mmap_params.exported_memh_buffer = map.segments[i].resource;
-        }
+        mmap_params.field_mask = UCP_MEM_MAP_PARAM_FIELD_ADDRESS |
+                                 UCP_MEM_MAP_PARAM_FIELD_LENGTH;
+        mmap_params.address = map.segments[i].address;
+        mmap_params.length  = map.segments[i].len;
+
         status = ucp_mem_map(ctx->worker.ucp_context, &mmap_params, &mh);
         if (UCS_OK != status) {
             tl_error(lib, "ucp_mem_map failed with error code: %d", status);
             ucc_status = ucs_status_to_ucc_status(status);
             goto fail_mem_map;
         }
-        if (lib->cfg.use_xgvmi && map.segments[i].resource != NULL) {
-            ctx->remote_info[i].packed_memh = map.segments[i].resource;
-        } else {
-            ctx->remote_info[i].packed_memh = NULL;
-        }
 
         ctx->remote_info[i].mem_h = (void *)mh;
         status                    = ucp_rkey_pack(ctx->worker.ucp_context, mh,

src/components/tl/ucp/tl_ucp_sendrecv.h

@@ -247,7 +247,7 @@ static inline int resolve_segment(const void *va, size_t *key_sizes,
 static inline ucc_status_t
 ucc_tl_ucp_resolve_p2p_by_va(ucc_tl_ucp_team_t *team, void *va, size_t msglen,
                              ucp_ep_h *ep, ucc_rank_t peer, uint64_t *rva,
-                             ucp_rkey_h *rkey, void **packed_memh, int *segment)
+                             ucp_rkey_h *rkey, int *segment)
 {
     ucc_tl_ucp_context_t *ctx            = UCC_TL_UCP_TEAM_CTX(team);
     ptrdiff_t             key_offset     = 0;
@@ -276,9 +276,6 @@ ucc_tl_ucp_resolve_p2p_by_va(ucc_tl_ucp_team_t *team, void *va, size_t msglen,
     if (*segment >= 0) {
         *rva = rvas[*segment] +
                ((uint64_t)va - (uint64_t)ctx->remote_info[*segment].va_base);
-        *packed_memh = (ctx->remote_info[*segment].packed_memh)
-                           ? ctx->remote_info[*segment].mem_h
-                           : NULL;
         if (ucc_unlikely(NULL == UCC_TL_UCP_REMOTE_RKEY(ctx, peer, *segment))) {
             ucs_status_t ucs_status = ucp_ep_rkey_unpack(
                 *ep, PTR_OFFSET(keys, key_offset),
@@ -302,9 +299,6 @@ ucc_tl_ucp_resolve_p2p_by_va(ucc_tl_ucp_team_t *team, void *va, size_t msglen,
         *rva = rvas[*segment] +
                ((uint64_t)va -
                 (uint64_t)ctx->dynamic_remote_info[*segment].va_base);
-        *packed_memh = (ctx->dynamic_remote_info[*segment].packed_memh)
-                           ? ctx->dynamic_remote_info[*segment].mem_h
-                           : NULL;
         if (ucc_unlikely(NULL ==
                          UCC_TL_UCP_DYN_REMOTE_RKEY(ctx, peer, *segment))) {
             ucs_status_t ucs_status = ucp_ep_rkey_unpack(
@@ -374,7 +368,6 @@ static inline ucc_status_t ucc_tl_ucp_put_nb(void *buffer, void *target,
     int                 segment     = 0;
     ucp_rkey_h          rkey        = NULL;
     uint64_t            rva         = 0;
-    void               *packed_memh = NULL;
     ucs_status_ptr_t    ucp_status;
     ucc_status_t        status;
     ucp_ep_h            ep;
@@ -386,7 +379,7 @@ static inline ucc_status_t ucc_tl_ucp_put_nb(void *buffer, void *target,
 
     status =
         ucc_tl_ucp_resolve_p2p_by_va(team, target, msglen, &ep, dest_group_rank,
-                                     &rva, &rkey, &packed_memh, &segment);
+                                     &rva, &rkey, &segment);
     if (ucc_unlikely(UCC_OK != status)) {
         return status;
     }
@@ -397,10 +390,6 @@ static inline ucc_status_t ucc_tl_ucp_put_nb(void *buffer, void *target,
     req_param.cb.send     = ucc_tl_ucp_put_completion_cb;
     req_param.user_data   = (void *)task;
     req_param.memory_type = ucc_memtype_to_ucs[mtype];
-    if (packed_memh) {
-        req_param.op_attr_mask |= UCP_OP_ATTR_FIELD_MEMH;
-        req_param.memh = packed_memh;
-    }
 
     ucp_status = ucp_put_nbx(ep, buffer, msglen, rva, rkey, &req_param);
     task->onesided.put_posted++;
@@ -425,7 +414,6 @@ static inline ucc_status_t ucc_tl_ucp_get_nb(void *buffer, void *target,
     int                 segment     = 0;
     ucp_rkey_h          rkey        = NULL;
     uint64_t            rva         = 0;
-    void               *packed_memh = NULL;
     ucs_status_ptr_t    ucp_status;
     ucc_status_t        status;
     ucp_ep_h            ep;
@@ -437,7 +425,7 @@ static inline ucc_status_t ucc_tl_ucp_get_nb(void *buffer, void *target,
 
     status =
         ucc_tl_ucp_resolve_p2p_by_va(team, target, msglen, &ep, dest_group_rank,
-                                     &rva, &rkey, &packed_memh, &segment);
+                                     &rva, &rkey, &segment);
     if (ucc_unlikely(UCC_OK != status)) {
         return status;
     }
@@ -448,10 +436,6 @@ static inline ucc_status_t ucc_tl_ucp_get_nb(void *buffer, void *target,
     req_param.cb.send     = ucc_tl_ucp_get_completion_cb;
     req_param.user_data   = (void *)task;
     req_param.memory_type = ucc_memtype_to_ucs[mtype];
-    if (packed_memh) {
-        req_param.op_attr_mask |= UCP_OP_ATTR_FIELD_MEMH;
-        req_param.memh = packed_memh;
-    }
 
     ucp_status = ucp_get_nbx(ep, buffer, msglen, rva, rkey, &req_param);
     task->onesided.get_posted++;
@@ -475,7 +459,6 @@ static inline ucc_status_t ucc_tl_ucp_atomic_inc(void *     target,
     uint64_t            one         = 1;
     ucp_rkey_h          rkey        = NULL;
     uint64_t            rva         = 0;
-    void               *packed_memh = NULL;
     ucs_status_ptr_t    ucp_status;
     ucc_status_t        status;
     ucp_ep_h            ep;
@@ -487,7 +470,7 @@ static inline ucc_status_t ucc_tl_ucp_atomic_inc(void *     target,
 
     status = ucc_tl_ucp_resolve_p2p_by_va(team, target, sizeof(uint64_t), &ep,
                                           dest_group_rank, &rva, &rkey,
-                                          &packed_memh, &segment);
+                                          &segment);
     if (ucc_unlikely(UCC_OK != status)) {
         return status;
     }

src/ucc/api/ucc.h

@@ -890,27 +890,13 @@ typedef struct ucc_oob_coll {
 typedef ucc_oob_coll_t ucc_context_oob_coll_t;
 typedef ucc_oob_coll_t ucc_team_oob_coll_t;
 
-/**
- * @ingroup UCC_CONTEXT_DT
- */
-typedef enum {
-    UCC_MEM_MAP_TYPE_SEND_BUF,
-    UCC_MEM_MAP_TYPE_RECV_BUF,
-    UCC_MEM_MAP_TYPE_SEND_RECV_BUF,
-} ucc_mem_map_usage_t;
-
 /**
  *
  *  @ingroup UCC_CONTEXT_DT
  */
 typedef struct ucc_mem_map {
-    void               *address;  /*!< the address of a buffer to be attached to
-                                     a UCC context */
-    size_t              len;      /*!< the length of the buffer */
-    ucc_mem_map_usage_t type;     /*!< the usage type of buffer being mapped. */
-    void               *resource; /*!< resource associated with the address.
-                                     examples of resources include memory
-                                     keys. */
+    void  *address;  /*!< the address of a buffer to be attached to a UCC context */
+    size_t len;      /*!< the length of the buffer */
 } ucc_mem_map_t;
 
 /**
@@ -1812,8 +1798,7 @@ enum ucc_coll_args_field {
     UCC_COLL_ARGS_FIELD_TAG                             = UCC_BIT(1),
     UCC_COLL_ARGS_FIELD_CB                              = UCC_BIT(2),
     UCC_COLL_ARGS_FIELD_GLOBAL_WORK_BUFFER              = UCC_BIT(3),
-    UCC_COLL_ARGS_FIELD_ACTIVE_SET                      = UCC_BIT(4),
-    UCC_COLL_ARGS_FIELD_MEM_MAP                         = UCC_BIT(5)
+    UCC_COLL_ARGS_FIELD_ACTIVE_SET                      = UCC_BIT(4)
 };
 
 /**
@@ -1888,14 +1873,6 @@ typedef struct ucc_coll_args {
         int64_t  stride;
         uint64_t size;
     } active_set;
-    ucc_mem_map_params_t            mem_map; /*!< Memory regions to be used
-                                                  for the current collective.
-                                                  If set, the designated regions
-                                                  will be mapped and information
-                                                  exchanged. Memory is unmapped
-                                                  at collective completion. Not
-                                                  necessary for two-sided
-                                                  collectives. */
 } ucc_coll_args_t;
 
 /**

src/ucc/api/ucc_status.h

@@ -40,7 +40,6 @@ typedef enum {
     UCC_ERR_NO_MESSAGE                  =   -6, /*!< General purpose return code without specific error */
     UCC_ERR_NOT_FOUND                   =   -7,
     UCC_ERR_TIMED_OUT                   =   -8,
-    UCC_ERR_MEM_MAP_FAILURE             =   -9,
     UCC_ERR_LAST                        = -100,
 } ucc_status_t;
 

src/utils/ucc_status.c

@@ -34,8 +34,6 @@ const char *ucc_status_string(ucc_status_t status)
         return "Not found";
     case UCC_ERR_TIMED_OUT:
         return "Timeout expired";
-    case UCC_ERR_MEM_MAP_FAILURE:
-        return "Failed to memory map address";
     default:
         snprintf(error_str, sizeof(error_str) - 1, "Unknown error %d", status);
         return error_str;