diff --git a/include/nccl_ofi_param.h b/include/nccl_ofi_param.h index 6e5ca34ad4..a32ccd65ea 100644 --- a/include/nccl_ofi_param.h +++ b/include/nccl_ofi_param.h @@ -380,4 +380,23 @@ OFI_NCCL_PARAM(NVTX_TRACE_DIMENSION, nvtx_trace_dimension, "NVTX_TRACE_DIMENSIO */ OFI_NCCL_PARAM(bool, gin_gdaki, "GIN_GDAKI", false); +/* + * Maximum number of outstanding put-signal requests per peer for the GIN + * plugin. Controls the size of the per-peer request ring used to track + * in-flight putSignal sequence numbers before an ACK is received. Must be + * a power of two. Default 128 preserves the historical compile-time limit. + * + * Applications that issue many concurrent putSignal operations per peer + * (for example, DeepEP V2 on AWS EFA with QP >= 5) may overrun the + * 128-slot ring and trigger "Next sequence number is in use" asserts. In + * such cases this can be raised to 256, 512, or 1024 at the cost of a + * small increase in per-peer memory (1 byte per slot per peer). + * + * The upper bound is GIN_IMM_SEQ_MASK + 1 (defined in + * rdma/gin/nccl_ofi_gin_types.h), which is 2048 by default, but must + * also leave at least 2 * GIN_ACK_INTERVAL (64) slots of headroom so + * that merged-ack ranges fit inside the window. + */ +OFI_NCCL_PARAM(size_t, gin_max_requests, "GIN_MAX_REQUESTS", 128); + #endif // End NCCL_OFI_PARAM_H_ diff --git a/include/rdma/gin/nccl_ofi_gin.h b/include/rdma/gin/nccl_ofi_gin.h index e0eab88e87..eb70c8e905 100644 --- a/include/rdma/gin/nccl_ofi_gin.h +++ b/include/rdma/gin/nccl_ofi_gin.h @@ -93,7 +93,7 @@ struct nccl_ofi_gin_peer_rank_info { This allows the remote rank to enforce ordering of signal delivery A 16-bit integer is large enough to store all outstanding requests, because the - plugin and NCCL limit max inflight requests. (See NCCL_OFI_MAX_REQUESTS.) */ + plugin and NCCL limit max inflight requests. (See OFI_NCCL_GIN_MAX_REQUESTS.) */ uint16_t next_target_seq_num = 0; /** @@ -103,12 +103,16 @@ struct nccl_ofi_gin_peer_rank_info { uint16_t next_delivered_signal_seq_num = 0; /* Flag, stored at initiator, indicating the given sequence number (mod - max_requests) is in use at initiator side. This allows initiator to + gin_max_requests_) is in use at initiator side. This allows initiator to track in-use sequence numbers to avoid overflow and only mark iputSignal complete when it has received the ack from the target, which has delivered the signal atomic. + + Sized at comm construction time to the runtime value of + OFI_NCCL_GIN_MAX_REQUESTS. Using uint8_t rather than bool for + stable layout and to avoid std::vector proxy semantics. */ - bool active_put_signal[NCCL_OFI_MAX_REQUESTS]; + std::vector active_put_signal; /* Counter for consecutive PUT-only messages without ACK. When this reaches OFI_NCCL_GIN_ACK_INTERVAL, the next PUT will request an ACK. @@ -213,12 +217,12 @@ class nccl_ofi_rdma_gin_put_comm : public nccl_ofi_gin_put_comm_t { bool query_ack_outstanding(uint32_t peer_rank, uint16_t msg_seq_num) const { - return rank_comms[peer_rank].active_put_signal[msg_seq_num % NCCL_OFI_MAX_REQUESTS]; + return rank_comms[peer_rank].active_put_signal[msg_seq_num % gin_max_requests_] != 0; } void clear_ack_outstanding(uint32_t peer_rank, uint16_t msg_seq_num) { - rank_comms[peer_rank].active_put_signal[msg_seq_num % NCCL_OFI_MAX_REQUESTS] = false; + rank_comms[peer_rank].active_put_signal[msg_seq_num % gin_max_requests_] = 0; } void clear_ack_range(uint32_t peer_rank, uint16_t ack_seq_num, uint16_t ack_count) @@ -301,6 +305,11 @@ class nccl_ofi_rdma_gin_put_comm : public nccl_ofi_gin_put_comm_t { int handle_ack_completion(fi_addr_t src_addr, uint16_t rail_id, const gin_ack_msg_t *ack_msg); + /* Return the effective per-peer request ring size that was established + for this communicator (from OFI_NCCL_GIN_MAX_REQUESTS, or the + default of 128). */ + size_t get_max_requests() const { return gin_max_requests_; } + private: nccl_ofi_gin_resources &resources; nccl_ofi_gin_resource_releaser resource_releaser; @@ -311,6 +320,10 @@ class nccl_ofi_rdma_gin_put_comm : public nccl_ofi_gin_put_comm_t { int nranks; int dev; + /* Per-peer request ring size (power of two). Read once from + OFI_NCCL_GIN_MAX_REQUESTS at comm construction. */ + size_t gin_max_requests_ = 128; + /* AllGather ring for metadata exchange */ nccl_ofi_gin_allgather_comm ag_comm; diff --git a/src/rdma/gin/nccl_ofi_gin.cpp b/src/rdma/gin/nccl_ofi_gin.cpp index 3b0b6c7298..2983ab21ed 100644 --- a/src/rdma/gin/nccl_ofi_gin.cpp +++ b/src/rdma/gin/nccl_ofi_gin.cpp @@ -8,8 +8,10 @@ #include "nccl_ofi_assert.h" #include "nccl_ofi_gdrcopy.h" +#include "nccl_ofi_param.h" #include "nccl_ofi_rdma.h" #include "nccl_ofi_tracepoint.h" +#include "rdma/gin/nccl_ofi_gin_types.h" struct gin_connect_handle { /* Number of rails */ @@ -25,6 +27,52 @@ struct gin_connect_handle { nccl_ofi_addr ep_names[MAX_NUM_RAILS]; }; +/** + * Compute the effective per-peer request ring size for the GIN plugin. + * + * Reads OFI_NCCL_GIN_MAX_REQUESTS, rounds down to a power of two, and + * clamps to [1, GIN_IMM_SEQ_MASK + 1 - 2*GIN_ACK_INTERVAL]. The upper + * bound keeps at least 2*GIN_ACK_INTERVAL slots of headroom inside the + * sequence-number window so merged ACK ranges fit. + * + * Returns the default value (128) on any invalid input. + */ +static size_t gin_resolve_max_requests() +{ + constexpr size_t default_val = 128; + constexpr size_t seq_space = GIN_IMM_SEQ_MASK + 1; + /* Keep headroom so ACK merged ranges never wrap past the seq window. */ + constexpr size_t hi_bound = seq_space - 2 * GIN_ACK_INTERVAL; + + size_t v = ofi_nccl_gin_max_requests(); + if (v == 0) { + NCCL_OFI_WARN("OFI_NCCL_GIN_MAX_REQUESTS=0 is invalid, using default %zu", + default_val); + return default_val; + } + + /* Round down to the nearest power of two. */ + size_t pow2 = 1; + while ((pow2 << 1) <= v) { + pow2 <<= 1; + } + if (pow2 != v) { + NCCL_OFI_WARN("OFI_NCCL_GIN_MAX_REQUESTS=%zu is not a power of two," + " rounding down to %zu", v, pow2); + v = pow2; + } + + if (v > hi_bound) { + NCCL_OFI_WARN("OFI_NCCL_GIN_MAX_REQUESTS=%zu exceeds max %zu" + " (GIN_IMM_SEQ_MASK+1 minus ACK headroom), clamping", + v, hi_bound); + v = 1; + while ((v << 1) <= hi_bound) v <<= 1; + } + + return v; +} + nccl_ofi_rdma_gin_put_comm::nccl_ofi_rdma_gin_put_comm(nccl_ofi_gin_resources &resources_arg, int rank_, int nranks_, nccl_net_ofi_send_comm *s_comm_, nccl_net_ofi_recv_comm *r_comm_) @@ -32,6 +80,8 @@ nccl_ofi_rdma_gin_put_comm::nccl_ofi_rdma_gin_put_comm(nccl_ofi_gin_resources &r dev(s_comm_->dev_id), ag_comm(s_comm_, r_comm_, rank_, nranks_), metadata_fl(nullptr, &freelist_deleter) { + gin_max_requests_ = gin_resolve_max_requests(); + auto &ep = resources.get_ep(); std::lock_guard scoped_ep_lock(ep.ep_lock); @@ -171,6 +221,10 @@ int nccl_ofi_rdma_gin_listen_comm::connect(nccl_net_ofi_conn_handle_t *handles[] } gin_comm->rank_comms.resize(nranks); + /* Size each peer's request-ring tracking vector to the runtime max. */ + for (auto &rc : gin_comm->rank_comms) { + rc.active_put_signal.assign(gin_comm->gin_max_requests_, 0); + } /** * Exchange connection metadata with all ranks using bootstrap ring @@ -424,8 +478,11 @@ int nccl_ofi_rdma_gin_put_comm::iputSignal(uint64_t srcOff, nccl_ofi_gin_symm_mr uint16_t rail_id = resources.get_next_rail(); auto scheduler = gin_ep.get_scheduler(); - if (OFI_UNLIKELY(rank_comm.active_put_signal[msg_seq_num % NCCL_OFI_MAX_REQUESTS])) { - NCCL_OFI_WARN("Next sequence number is in use"); + if (OFI_UNLIKELY(rank_comm.active_put_signal[msg_seq_num % gin_max_requests_] != 0)) { + NCCL_OFI_WARN("Next sequence number is in use (gin_max_requests=%zu)." + " Raise OFI_NCCL_GIN_MAX_REQUESTS if your workload" + " issues many concurrent putSignals per peer.", + gin_max_requests_); assert(false); return -EBUSY; } @@ -585,7 +642,7 @@ int nccl_ofi_rdma_gin_put_comm::iputSignal(uint64_t srcOff, nccl_ofi_gin_symm_mr /* Update umbrella request with send_req */ req->send_req = send_req; - rank_comm.active_put_signal[msg_seq_num % NCCL_OFI_MAX_REQUESTS] = is_ack_requested; + rank_comm.active_put_signal[msg_seq_num % gin_max_requests_] = is_ack_requested ? 1 : 0; rank_comm.next_target_seq_num = (rank_comm.next_target_seq_num + 1) & GIN_IMM_SEQ_MASK; *request = req;