From bbaffd3681f433ca59c9e27cb96174f9baecf51f Mon Sep 17 00:00:00 2001 From: Sergey Oblomov Date: Tue, 22 May 2018 22:11:23 +0300 Subject: [PATCH 1/5] MCA/UCX: atomic add/swap are moved to new UCX atomic API Signed-off-by: Sergey Oblomov --- oshmem/mca/atomic/ucx/atomic_ucx.h | 4 +++ oshmem/mca/atomic/ucx/atomic_ucx_cswap.c | 31 ++++++++--------- oshmem/mca/atomic/ucx/atomic_ucx_fadd.c | 42 ++++++++--------------- oshmem/mca/atomic/ucx/atomic_ucx_module.c | 21 ++++++++++++ 4 files changed, 53 insertions(+), 45 deletions(-) diff --git a/oshmem/mca/atomic/ucx/atomic_ucx.h b/oshmem/mca/atomic/ucx/atomic_ucx.h index 4db6008c1f..2f54f799d8 100644 --- a/oshmem/mca/atomic/ucx/atomic_ucx.h +++ b/oshmem/mca/atomic/ucx/atomic_ucx.h @@ -60,6 +60,10 @@ struct mca_atomic_ucx_module_t { typedef struct mca_atomic_ucx_module_t mca_atomic_ucx_module_t; OBJ_CLASS_DECLARATION(mca_atomic_ucx_module_t); + +void mca_atomic_ucx_complete_cb(void *request, ucs_status_t status); +ucs_status_t mca_atomic_ucx_wait_request(ucs_status_ptr_t request); + END_C_DECLS #endif /* MCA_ATOMIC_MXM_H */ diff --git a/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c b/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c index fc4c7a33f5..0137e5b086 100644 --- a/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c +++ b/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c @@ -27,23 +27,23 @@ int mca_atomic_ucx_cswap(void *target, int pe) { ucs_status_t status; + ucs_status_ptr_t status_ptr; spml_ucx_mkey_t *ucx_mkey; uint64_t rva; + uint64_t val; + + if ((8 != nlong) && (4 != nlong)) { + ATOMIC_ERROR("[#%d] Type size must be 4 or 8 bytes.", my_pe); + return OSHMEM_ERROR; + } ucx_mkey = mca_spml_ucx_get_mkey(pe, target, (void *)&rva); + val = (8 == nlong) ? *(uint64_t*)value : *(uint32_t*)value; if (NULL == cond) { - switch (nlong) { - case 4: - status = ucp_atomic_swap32(mca_spml_self->ucp_peers[pe].ucp_conn, - *(uint32_t *)value, rva, ucx_mkey->rkey, prev); - break; - case 8: - status = ucp_atomic_swap64(mca_spml_self->ucp_peers[pe].ucp_conn, - *(uint64_t *)value, rva, ucx_mkey->rkey, prev); - break; - default: - goto err_size; - } + status_ptr = ucp_atomic_fetch_nb(mca_spml_self->ucp_peers[pe].ucp_conn, + UCP_ATOMIC_FETCH_OP_SWAP, val, prev, nlong, + rva, ucx_mkey->rkey, mca_atomic_ucx_complete_cb); + status = mca_atomic_ucx_wait_request(status_ptr); } else { switch (nlong) { @@ -56,15 +56,12 @@ int mca_atomic_ucx_cswap(void *target, *(uint64_t *)cond, *(uint64_t *)value, rva, ucx_mkey->rkey, prev); break; default: - goto err_size; + assert(0); /* should not be here */ + break; } } return ucx_status_to_oshmem(status); - -err_size: - ATOMIC_ERROR("[#%d] Type size must be 1/2/4 or 8 bytes.", my_pe); - return OSHMEM_ERROR; } diff --git a/oshmem/mca/atomic/ucx/atomic_ucx_fadd.c b/oshmem/mca/atomic/ucx/atomic_ucx_fadd.c index a1b88c95de..8e95e9cedc 100644 --- a/oshmem/mca/atomic/ucx/atomic_ucx_fadd.c +++ b/oshmem/mca/atomic/ucx/atomic_ucx_fadd.c @@ -26,43 +26,29 @@ int mca_atomic_ucx_fadd(void *target, struct oshmem_op_t *op) { ucs_status_t status; + ucs_status_ptr_t status_ptr; spml_ucx_mkey_t *ucx_mkey; uint64_t rva; + uint64_t val; + + if ((8 != nlong) && (4 != nlong)) { + ATOMIC_ERROR("[#%d] Type size must be 4 or 8 bytes.", my_pe); + return OSHMEM_ERROR; + } ucx_mkey = mca_spml_ucx_get_mkey(pe, target, (void *)&rva); + val = (8 == nlong) ? *(uint64_t*)value : *(uint32_t*)value; if (NULL == prev) { - switch (nlong) { - case 4: - status = ucp_atomic_add32(mca_spml_self->ucp_peers[pe].ucp_conn, - *(uint32_t *)value, rva, ucx_mkey->rkey); - break; - case 8: - status = ucp_atomic_add64(mca_spml_self->ucp_peers[pe].ucp_conn, - *(uint64_t *)value, rva, ucx_mkey->rkey); - break; - default: - goto err_size; - } + status = ucp_atomic_post(mca_spml_self->ucp_peers[pe].ucp_conn, + UCP_ATOMIC_POST_OP_ADD, val, nlong, rva, ucx_mkey->rkey); } else { - switch (nlong) { - case 4: - status = ucp_atomic_fadd32(mca_spml_self->ucp_peers[pe].ucp_conn, - *(uint32_t *)value, rva, ucx_mkey->rkey, prev); - break; - case 8: - status = ucp_atomic_fadd64(mca_spml_self->ucp_peers[pe].ucp_conn, - *(uint64_t *)value, rva, ucx_mkey->rkey, prev); - break; - default: - goto err_size; - } + status_ptr = ucp_atomic_fetch_nb(mca_spml_self->ucp_peers[pe].ucp_conn, + UCP_ATOMIC_FETCH_OP_FADD, val, prev, nlong, + rva, ucx_mkey->rkey, mca_atomic_ucx_complete_cb); + status = mca_atomic_ucx_wait_request(status_ptr); } return ucx_status_to_oshmem(status); - -err_size: - ATOMIC_ERROR("[#%d] Type size must be 1/2/4 or 8 bytes.", my_pe); - return OSHMEM_ERROR; } diff --git a/oshmem/mca/atomic/ucx/atomic_ucx_module.c b/oshmem/mca/atomic/ucx/atomic_ucx_module.c index 0b570043a6..18c92d9303 100644 --- a/oshmem/mca/atomic/ucx/atomic_ucx_module.c +++ b/oshmem/mca/atomic/ucx/atomic_ucx_module.c @@ -49,3 +49,24 @@ mca_atomic_ucx_query(int *priority) return NULL ; } +void mca_atomic_ucx_complete_cb(void *request, ucs_status_t status) +{ +} + +ucs_status_t mca_atomic_ucx_wait_request(ucs_status_ptr_t request) +{ + ucs_status_t status; + + /* check for request completed or failed */ + if (UCS_OK == request) { + return UCS_OK; + } else if (UCS_PTR_IS_ERR(request)) { + return UCS_PTR_STATUS(request); + } + + while (UCS_INPROGRESS == (status = ucp_request_check_status(request))) { + ucp_worker_progress(mca_spml_self->ucp_worker); + } + ucp_request_free(request); + return status; +} From 0c3ed93ef0a3fb4c533814f46243dba368462132 Mon Sep 17 00:00:00 2001 From: Sergey Oblomov Date: Tue, 29 May 2018 11:34:40 +0300 Subject: [PATCH 2/5] MCA/UCX: added opal progress call to wait request routine - added opal_progress call to wait function to avoid possible [dead]lock issues - wait call is declared as inline - minor fixes Signed-off-by: Sergey Oblomov --- oshmem/mca/atomic/ucx/atomic_ucx.h | 29 +++++++++++++++++++++-- oshmem/mca/atomic/ucx/atomic_ucx_cswap.c | 1 + oshmem/mca/atomic/ucx/atomic_ucx_module.c | 17 ------------- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/oshmem/mca/atomic/ucx/atomic_ucx.h b/oshmem/mca/atomic/ucx/atomic_ucx.h index 2f54f799d8..3b5ba03b73 100644 --- a/oshmem/mca/atomic/ucx/atomic_ucx.h +++ b/oshmem/mca/atomic/ucx/atomic_ucx.h @@ -62,8 +62,33 @@ OBJ_CLASS_DECLARATION(mca_atomic_ucx_module_t); void mca_atomic_ucx_complete_cb(void *request, ucs_status_t status); -ucs_status_t mca_atomic_ucx_wait_request(ucs_status_ptr_t request); +static inline +ucs_status_t mca_atomic_ucx_wait_request(ucs_status_ptr_t request) +{ + ucs_status_t status; + int i; + + /* check for request completed or failed */ + if (UCS_OK == request) { + return UCS_OK; + } else if (UCS_PTR_IS_ERR(request)) { + return UCS_PTR_STATUS(request); + } + + while (1) { + /* call UCX progress */ + for (i = 0; i < 100; i++) { + if (UCS_INPROGRESS != (status = ucp_request_check_status(request))) { + ucp_request_free(request); + return status; + } + ucp_worker_progress(mca_spml_self->ucp_worker); + } + /* call OPAL progress on every 100 call to UCX progress */ + opal_progress(); + } +} END_C_DECLS -#endif /* MCA_ATOMIC_MXM_H */ +#endif /* MCA_ATOMIC_UCX_H */ diff --git a/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c b/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c index 0137e5b086..74c08aa5fc 100644 --- a/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c +++ b/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c @@ -57,6 +57,7 @@ int mca_atomic_ucx_cswap(void *target, break; default: assert(0); /* should not be here */ + status = UCS_ERR_INVALID_PARAM; break; } } diff --git a/oshmem/mca/atomic/ucx/atomic_ucx_module.c b/oshmem/mca/atomic/ucx/atomic_ucx_module.c index 18c92d9303..a59783d186 100644 --- a/oshmem/mca/atomic/ucx/atomic_ucx_module.c +++ b/oshmem/mca/atomic/ucx/atomic_ucx_module.c @@ -53,20 +53,3 @@ void mca_atomic_ucx_complete_cb(void *request, ucs_status_t status) { } -ucs_status_t mca_atomic_ucx_wait_request(ucs_status_ptr_t request) -{ - ucs_status_t status; - - /* check for request completed or failed */ - if (UCS_OK == request) { - return UCS_OK; - } else if (UCS_PTR_IS_ERR(request)) { - return UCS_PTR_STATUS(request); - } - - while (UCS_INPROGRESS == (status = ucp_request_check_status(request))) { - ucp_worker_progress(mca_spml_self->ucp_worker); - } - ucp_request_free(request); - return status; -} From 6be4066e23fdf0870fcaf3bc0be701487acd07d3 Mon Sep 17 00:00:00 2001 From: Sergey Oblomov Date: Tue, 29 May 2018 19:42:27 +0300 Subject: [PATCH 3/5] MCA/UCX: cswap call if updated to non-blocking API - minor fixes Signed-off-by: Sergey Oblomov --- oshmem/mca/atomic/ucx/atomic_ucx_cswap.c | 42 ++++++++++++++---------- oshmem/mca/atomic/ucx/atomic_ucx_fadd.c | 15 +++++---- 2 files changed, 33 insertions(+), 24 deletions(-) diff --git a/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c b/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c index 74c08aa5fc..9039da6421 100644 --- a/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c +++ b/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c @@ -31,37 +31,43 @@ int mca_atomic_ucx_cswap(void *target, spml_ucx_mkey_t *ucx_mkey; uint64_t rva; uint64_t val; + uint64_t cmp; - if ((8 != nlong) && (4 != nlong)) { + if (8 == nlong) { + val = *(uint64_t*)value; + } else if (4 == nlong) { + val = *(uint32_t*)value; + } else { ATOMIC_ERROR("[#%d] Type size must be 4 or 8 bytes.", my_pe); return OSHMEM_ERROR; } ucx_mkey = mca_spml_ucx_get_mkey(pe, target, (void *)&rva); - val = (8 == nlong) ? *(uint64_t*)value : *(uint32_t*)value; if (NULL == cond) { status_ptr = ucp_atomic_fetch_nb(mca_spml_self->ucp_peers[pe].ucp_conn, - UCP_ATOMIC_FETCH_OP_SWAP, val, prev, nlong, - rva, ucx_mkey->rkey, mca_atomic_ucx_complete_cb); + UCP_ATOMIC_FETCH_OP_SWAP, val, prev, nlong, + rva, ucx_mkey->rkey, mca_atomic_ucx_complete_cb); status = mca_atomic_ucx_wait_request(status_ptr); } else { - switch (nlong) { - case 4: - status = ucp_atomic_cswap32(mca_spml_self->ucp_peers[pe].ucp_conn, - *(uint32_t *)cond, *(uint32_t *)value, rva, ucx_mkey->rkey, prev); - break; - case 8: - status = ucp_atomic_cswap64(mca_spml_self->ucp_peers[pe].ucp_conn, - *(uint64_t *)cond, *(uint64_t *)value, rva, ucx_mkey->rkey, prev); - break; - default: - assert(0); /* should not be here */ - status = UCS_ERR_INVALID_PARAM; - break; + if (8 == nlong) { + cmp = *(uint64_t*)cond; + } else { + cmp = *(uint32_t*)cond; + } + status_ptr = ucp_atomic_fetch_nb(mca_spml_self->ucp_peers[pe].ucp_conn, + UCP_ATOMIC_FETCH_OP_CSWAP, cmp, &val, nlong, + rva, ucx_mkey->rkey, mca_atomic_ucx_complete_cb); + status = mca_atomic_ucx_wait_request(status_ptr); + if (UCS_OK == status) { + assert(NULL != prev); + if (8 == nlong) { + *(uint64_t*)prev = val; + } else { + *(uint32_t*)prev = val; + } } } - return ucx_status_to_oshmem(status); } diff --git a/oshmem/mca/atomic/ucx/atomic_ucx_fadd.c b/oshmem/mca/atomic/ucx/atomic_ucx_fadd.c index 8e95e9cedc..b8639d2d00 100644 --- a/oshmem/mca/atomic/ucx/atomic_ucx_fadd.c +++ b/oshmem/mca/atomic/ucx/atomic_ucx_fadd.c @@ -31,22 +31,25 @@ int mca_atomic_ucx_fadd(void *target, uint64_t rva; uint64_t val; - if ((8 != nlong) && (4 != nlong)) { + if (8 == nlong) { + val = *(uint64_t*)value; + } else if (4 == nlong) { + val = *(uint32_t*)value; + } else { ATOMIC_ERROR("[#%d] Type size must be 4 or 8 bytes.", my_pe); return OSHMEM_ERROR; } ucx_mkey = mca_spml_ucx_get_mkey(pe, target, (void *)&rva); - val = (8 == nlong) ? *(uint64_t*)value : *(uint32_t*)value; - if (NULL == prev) { status = ucp_atomic_post(mca_spml_self->ucp_peers[pe].ucp_conn, - UCP_ATOMIC_POST_OP_ADD, val, nlong, rva, ucx_mkey->rkey); + UCP_ATOMIC_POST_OP_ADD, val, nlong, rva, + ucx_mkey->rkey); } else { status_ptr = ucp_atomic_fetch_nb(mca_spml_self->ucp_peers[pe].ucp_conn, - UCP_ATOMIC_FETCH_OP_FADD, val, prev, nlong, - rva, ucx_mkey->rkey, mca_atomic_ucx_complete_cb); + UCP_ATOMIC_FETCH_OP_FADD, val, prev, nlong, + rva, ucx_mkey->rkey, mca_atomic_ucx_complete_cb); status = mca_atomic_ucx_wait_request(status_ptr); } From daad71f0361988508e06ae409113d2a72d02c2dd Mon Sep 17 00:00:00 2001 From: Sergey Oblomov Date: Tue, 29 May 2018 22:33:33 +0300 Subject: [PATCH 4/5] MCA/UCX: switch/case optimization Signed-off-by: Sergey Oblomov --- oshmem/mca/atomic/ucx/atomic_ucx_cswap.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c b/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c index 9039da6421..0727879b8c 100644 --- a/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c +++ b/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c @@ -50,22 +50,14 @@ int mca_atomic_ucx_cswap(void *target, status = mca_atomic_ucx_wait_request(status_ptr); } else { - if (8 == nlong) { - cmp = *(uint64_t*)cond; - } else { - cmp = *(uint32_t*)cond; - } + cmp = (4 == nlong) ? *(uint32_t*)cond : *(uint64_t*)cond; status_ptr = ucp_atomic_fetch_nb(mca_spml_self->ucp_peers[pe].ucp_conn, UCP_ATOMIC_FETCH_OP_CSWAP, cmp, &val, nlong, rva, ucx_mkey->rkey, mca_atomic_ucx_complete_cb); status = mca_atomic_ucx_wait_request(status_ptr); if (UCS_OK == status) { assert(NULL != prev); - if (8 == nlong) { - *(uint64_t*)prev = val; - } else { - *(uint32_t*)prev = val; - } + memcpy(prev, &val, nlong); } } return ucx_status_to_oshmem(status); From 319bb376f9a00404accd794733d5c525f3fe41c6 Mon Sep 17 00:00:00 2001 From: Sergey Oblomov Date: Wed, 30 May 2018 07:14:43 +0300 Subject: [PATCH 5/5] MCA/UCX: branch optimization in cswap call Signed-off-by: Sergey Oblomov --- oshmem/mca/atomic/ucx/atomic_ucx_cswap.c | 47 ++++++++++++++++-------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c b/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c index 0727879b8c..57723cf0ae 100644 --- a/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c +++ b/oshmem/mca/atomic/ucx/atomic_ucx_cswap.c @@ -19,12 +19,15 @@ #include "atomic_ucx.h" -int mca_atomic_ucx_cswap(void *target, - void *prev, - const void *cond, - const void *value, - size_t nlong, - int pe) +/* nlong argument should be constant to hint compiler + * to calculate nlong relative branches in compile time */ +static inline +int mca_atomic_ucx_cswap_inner(void *target, + void *prev, + const void *cond, + const void *value, + size_t nlong, + int pe) { ucs_status_t status; ucs_status_ptr_t status_ptr; @@ -33,15 +36,7 @@ int mca_atomic_ucx_cswap(void *target, uint64_t val; uint64_t cmp; - if (8 == nlong) { - val = *(uint64_t*)value; - } else if (4 == nlong) { - val = *(uint32_t*)value; - } else { - ATOMIC_ERROR("[#%d] Type size must be 4 or 8 bytes.", my_pe); - return OSHMEM_ERROR; - } - + val = (4 == nlong) ? *(uint32_t*)value : *(uint64_t*)value; ucx_mkey = mca_spml_ucx_get_mkey(pe, target, (void *)&rva); if (NULL == cond) { status_ptr = ucp_atomic_fetch_nb(mca_spml_self->ucp_peers[pe].ucp_conn, @@ -58,9 +53,29 @@ int mca_atomic_ucx_cswap(void *target, if (UCS_OK == status) { assert(NULL != prev); memcpy(prev, &val, nlong); + if (4 == nlong) { + *(uint32_t*)prev = val; + } else { + *(uint64_t*)prev = val; + } } } return ucx_status_to_oshmem(status); } - +int mca_atomic_ucx_cswap(void *target, + void *prev, + const void *cond, + const void *value, + size_t nlong, + int pe) +{ + if (8 == nlong) { + return mca_atomic_ucx_cswap_inner(target, prev, cond, value, 8, pe); + } else if (4 == nlong) { + return mca_atomic_ucx_cswap_inner(target, prev, cond, value, 4, pe); + } else { + ATOMIC_ERROR("[#%d] Type size must be 4 or 8 bytes.", my_pe); + return OSHMEM_ERROR; + } +}