diff --git a/AUTHORS b/AUTHORS index 78f1ed59459..b4d6defc319 100644 --- a/AUTHORS +++ b/AUTHORS @@ -80,6 +80,7 @@ Manu Shantharam Marek Schimara Mark Allen Matthew Baker +Michael Beebe Michael Braverman Michal Shalev Mike Dubman diff --git a/src/ucp/core/ucp_ep.c b/src/ucp/core/ucp_ep.c index fbf61df94a5..4798e747086 100644 --- a/src/ucp/core/ucp_ep.c +++ b/src/ucp/core/ucp_ep.c @@ -277,6 +277,7 @@ static ucp_ep_h ucp_ep_allocate(ucp_worker_h worker, const char *peer_name) ep->ext->peer_mem = NULL; ep->ext->unflushed_lanes = 0; ep->ext->fence_seq = 0; + ep->ext->fenced_lane = UCP_NULL_LANE; ep->ext->uct_eps = NULL; ep->ext->flush_sys_dev_map = 0; diff --git a/src/ucp/core/ucp_ep.h b/src/ucp/core/ucp_ep.h index 446e875a0ac..5c97f2d34c5 100644 --- a/src/ucp/core/ucp_ep.h +++ b/src/ucp/core/ucp_ep.h @@ -565,6 +565,9 @@ typedef struct ucp_ep_ext { void *user_data; /* User data associated with ep */ ucs_list_link_t ep_list; /* List entry in worker's all eps list */ ucp_rsc_index_t cm_idx; /* CM index */ + ucp_lane_index_t fenced_lane; /* Lane ordered by a weak fence + since the last strong fence, + or UCP_NULL_LANE */ ucs_ptr_map_key_t local_ep_id; /* Local EP ID */ ucs_ptr_map_key_t remote_ep_id; /* Remote EP ID */ ucp_err_handler_cb_t err_cb; /* Error handler */ diff --git a/src/ucp/rma/flush.c b/src/ucp/rma/flush.c index 1b9de2b1f64..c37baf64233 100644 --- a/src/ucp/rma/flush.c +++ b/src/ucp/rma/flush.c @@ -905,6 +905,7 @@ ucs_status_t ucp_ep_fence_strong(ucp_ep_h ep) } ep->ext->unflushed_lanes = 0; + ep->ext->fenced_lane = UCP_NULL_LANE; ep->ext->fence_seq = ep->worker->fence_seq; return UCS_OK; } diff --git a/src/ucp/rma/rma.inl b/src/ucp/rma/rma.inl index cfc481f95a4..0fe97304fa1 100644 --- a/src/ucp/rma/rma.inl +++ b/src/ucp/rma/rma.inl @@ -150,6 +150,15 @@ ucp_ep_rma_is_fence_required(ucp_ep_h ep) return ep->ext->fence_seq < ep->worker->fence_seq; } +/* A weak fence orders only its own lane, so an operation on another lane + * requires a strong fence */ +static UCS_F_ALWAYS_INLINE int +ucp_ep_rma_is_strong_fence_required(ucp_ep_h ep, ucp_lane_map_t lane_map) +{ + return ucs_unlikely(ep->ext->fenced_lane != UCP_NULL_LANE) && + (lane_map & ~UCS_BIT(ep->ext->fenced_lane)); +} + static UCS_F_ALWAYS_INLINE uint32_t ucp_ep_rma_get_fence_flag(ucp_ep_h ep) { @@ -169,13 +178,24 @@ ucp_ep_rma_handle_fence(ucp_ep_h ep, ucp_request_t *req, /* Apply a fence if EP's sequence is behind worker's */ if (ucs_unlikely(req->flags & UCP_REQUEST_FLAG_FENCE_REQUIRED)) { if (ucs_unlikely(ep->ext->unflushed_lanes == 0)) { - status = UCS_OK; + /* No earlier operation to order */ + ucs_assert(ep->ext->fenced_lane == UCP_NULL_LANE); + ep->ext->fence_seq = ep->worker->fence_seq; + status = UCS_OK; } else if (ucs_likely( ucs_is_pow2_or_zero(ep->ext->unflushed_lanes | lane_map))) { status = ucp_ep_fence_weak(ep); + if (status == UCS_OK) { + /* The fence orders all earlier operations on this single + * lane, so later operations on it need no further fence */ + ep->ext->fence_seq = ep->worker->fence_seq; + ep->ext->fenced_lane = ucs_ffs64(ep->ext->unflushed_lanes); + } } else { status = ucp_ep_fence_strong(ep); } + } else if (ucp_ep_rma_is_strong_fence_required(ep, lane_map)) { + status = ucp_ep_fence_strong(ep); } else { status = UCS_OK; } diff --git a/src/ucp/rma/rma_send.c b/src/ucp/rma/rma_send.c index bdf322a8380..e882fb04fb8 100644 --- a/src/ucp/rma/rma_send.c +++ b/src/ucp/rma/rma_send.c @@ -173,7 +173,9 @@ ucp_put_send_short(ucp_ep_h ep, const void *buffer, size_t length, tl_rkey = ucp_rkey_get_tl_rkey(rkey, rkey_config->put_short.rkey_index); - if (ucs_unlikely(ucp_ep_rma_is_fence_required(ep))) { + if (ucs_unlikely(ucp_ep_rma_is_fence_required(ep) || + ucp_ep_rma_is_strong_fence_required( + ep, UCS_BIT(rkey_config->put_short.lane)))) { /* TODO: check support for fence in fast path short */ return UCS_ERR_NO_RESOURCE; } diff --git a/test/gtest/ucp/test_ucp_rma.cc b/test/gtest/ucp/test_ucp_rma.cc index 6d868dbe908..1c98ba34599 100755 --- a/test/gtest/ucp/test_ucp_rma.cc +++ b/test/gtest/ucp/test_ucp_rma.cc @@ -853,6 +853,14 @@ class test_ucp_ep_based_fence : public test_ucp_rma { modify_config("FENCE_MODE", "ep_based"); modify_config("MAX_RMA_LANES", "2"); test_ucp_memheap::init(); + self = this; + } + + virtual void cleanup() + { + m_mock.cleanup(); + self = nullptr; + test_ucp_memheap::cleanup(); } uint64_t worker_fence_seq() { @@ -955,6 +963,76 @@ class test_ucp_ep_based_fence : public test_ucp_rma { * iterations. */ sender().ep()->ext->unflushed_lanes = 0; + sender().ep()->ext->fenced_lane = UCP_NULL_LANE; + } + + static ucs_status_t ep_fence_count(uct_ep_h ep, unsigned flags) + { + ++self->m_fence_count; + return self->m_mock.orig_func(&ep->iface->ops.ep_fence, ep, flags); + } + + void mock_ep_fence() + { + ucp_ep_h ep = sender().ep(); + + for (ucp_lane_index_t lane = 0; lane < ucp_ep_num_lanes(ep); ++lane) { + if (lane != ucp_ep_get_cm_lane(ep)) { + m_mock.setup(&ucp_ep_get_lane(ep, lane)->iface->ops.ep_fence, + ep_fence_count); + } + } + } + + void do_rma_op_with_fence_once(op_type_t op, void *sbuf, size_t size, + void *target, ucp_rkey_h rkey) + { + unsigned fence_count; + + mock_ep_fence(); + do_rma_op_with_fence(op, sbuf, size, target, rkey); + EXPECT_EQ(worker_fence_seq(), ep_fence_seq()); + + /* The fence is already applied, so it must not be applied again */ + fence_count = m_fence_count; + do_rma_op(op, sbuf, size, target, rkey); + EXPECT_EQ(fence_count, m_fence_count); + EXPECT_EQ(worker_fence_seq(), ep_fence_seq()); + + flush_worker(sender()); + flush_sender_cleanup(); + m_mock.cleanup(); + } + + void expect_fenced_lane_ordered(ucp_lane_map_t fenced_lane_map) + { + /* An operation on a lane other than the weakly fenced one must be + * preceded by a strong fence, which resets the fenced lane */ + if (sender().ep()->ext->unflushed_lanes & ~fenced_lane_map) { + EXPECT_EQ(UCP_NULL_LANE, sender().ep()->ext->fenced_lane); + } + } + + void do_rma_op_with_fence_other_lane(op_type_t op, void *sbuf, size_t size, + void *target, ucp_rkey_h rkey) + { + ucp_lane_map_t fenced_lane_map; + + do_rma_op_with_fence(op, sbuf, size, target, rkey); + if (sender().ep()->ext->fenced_lane != UCP_NULL_LANE) { + fenced_lane_map = UCS_BIT(sender().ep()->ext->fenced_lane); + + /* A short put may use the fast path */ + do_rma_op(OP_PUT, sbuf, 1, target, rkey); + expect_fenced_lane_ordered(fenced_lane_map); + + /* A large put may use multiple lanes */ + do_rma_op(OP_PUT, sbuf, TEST_BUF_SIZE, target, rkey); + expect_fenced_lane_ordered(fenced_lane_map); + } + + flush_workers(); + flush_sender_cleanup(); } void do_rma_op_with_fence_before(op_type_t op, void *sbuf, size_t size, @@ -967,8 +1045,14 @@ class test_ucp_ep_based_fence : public test_ucp_rma { } private: static constexpr size_t TEST_BUF_SIZE = 1000000; + static test_ucp_ep_based_fence *self; + + ucs::mock m_mock; + unsigned m_fence_count = 0; }; +test_ucp_ep_based_fence *test_ucp_ep_based_fence::self = nullptr; + UCS_TEST_P(test_ucp_ep_based_fence, test_ep_based_fence_put) { test_ep_based_fence_common( OP_PUT, &test_ucp_ep_based_fence::do_rma_op_with_fence); @@ -999,6 +1083,36 @@ UCS_TEST_P(test_ucp_ep_based_fence, test_ep_based_fence_before_atomic) { OP_ATOMIC, &test_ucp_ep_based_fence::do_rma_op_with_fence_before); } +UCS_TEST_P(test_ucp_ep_based_fence, test_ep_based_fence_once_put) +{ + test_ep_based_fence_common( + OP_PUT, &test_ucp_ep_based_fence::do_rma_op_with_fence_once); +} + +UCS_TEST_P(test_ucp_ep_based_fence, test_ep_based_fence_once_get) +{ + test_ep_based_fence_common( + OP_GET, &test_ucp_ep_based_fence::do_rma_op_with_fence_once); +} + +UCS_TEST_P(test_ucp_ep_based_fence, test_ep_based_fence_once_atomic) +{ + test_ep_based_fence_common( + OP_ATOMIC, &test_ucp_ep_based_fence::do_rma_op_with_fence_once); +} + +UCS_TEST_P(test_ucp_ep_based_fence, test_ep_based_fence_other_lane_put) +{ + test_ep_based_fence_common( + OP_PUT, &test_ucp_ep_based_fence::do_rma_op_with_fence_other_lane); +} + +UCS_TEST_P(test_ucp_ep_based_fence, test_ep_based_fence_other_lane_get) +{ + test_ep_based_fence_common( + OP_GET, &test_ucp_ep_based_fence::do_rma_op_with_fence_other_lane); +} + UCP_INSTANTIATE_TEST_CASE_TLS(test_ucp_ep_based_fence, all, "all") class test_ucp_rma_sgl : public test_ucp_rma {