Skip to content

gin: make per-peer request ring size env-tunable for high-QP AWS EFA #1206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dmvevents wants to merge 1 commit into from
Closed

gin: make per-peer request ring size env-tunable for high-QP AWS EFA #1206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions include/nccl_ofi_param.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_
23 changes: 18 additions & 5 deletions include/rdma/gin/nccl_ofi_gin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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<bool> proxy semantics.
*/
bool active_put_signal[NCCL_OFI_MAX_REQUESTS];
std::vector<uint8_t> 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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand All @@ -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;

Expand Down
63 changes: 60 additions & 3 deletions src/rdma/gin/nccl_ofi_gin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -25,13 +27,61 @@ 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_)
: resources(resources_arg), resource_releaser { resources }, rank(rank_), nranks(nranks_),
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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down