Repository navigation
UCT/IB/MLX5: Purge outstanding AM short operations #11857
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.
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
Changes from all commits
65b8b97
2e3e70d
1a39e5f
337510a
369e583
751da50
30c4fa2
485eb51
f9a2484
e9f208d
81c63d1
981d6ad
23e6ddc
c72dbd0
3f6c0a5
b76e1d6
a44bce3
26af207
aec2bcb
c5ed002
bd56d22
10bd703
c2942cd
75fc3a4
d54595f
1f518a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -858,6 +858,23 @@ void *uct_ib_mlx5_txwq_get_wqe(const uct_ib_mlx5_txwq_t *txwq, uint16_t pi) | |
| return UCS_PTR_BYTE_OFFSET(txwq->qstart, (pi % num_bb) * MLX5_SEND_WQE_BB); | ||
| } | ||
|
|
||
| size_t uct_ib_mlx5_wqe_size(const struct mlx5_wqe_ctrl_seg *ctrl) | ||
| { | ||
| uint8_t ds = ntohl(ctrl->qpn_ds) & UINT8_MAX; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The old num_posted_wqes used (ctrl->qpn_ds >> 24), which extracts ds only on little-endian; the new ntohl(ctrl->qpn_ds) & UINT8_MAX is correct on both endiannesses. This is effectively a big-endian bug fix and should be called out. |
||
|
|
||
| ucs_assertv_always( | ||
| (ds > 0) && | ||
| ((ds * UCT_IB_MLX5_WQE_SEG_SIZE) <= | ||
| UCT_IB_MLX5_MAX_SEND_WQE_SIZE), | ||
| "ds=%u", ds); | ||
| return ds * UCT_IB_MLX5_WQE_SEG_SIZE; | ||
| } | ||
|
|
||
| uint16_t uct_ib_mlx5_txwq_next_wqe_index(uint16_t index, size_t wqe_size) | ||
| { | ||
| return index + ucs_div_round_up(wqe_size, MLX5_SEND_WQE_BB); | ||
| } | ||
|
|
||
| uint16_t uct_ib_mlx5_txwq_num_posted_wqes(const uct_ib_mlx5_txwq_t *txwq, | ||
| uint16_t outstanding) | ||
| { | ||
|
|
@@ -873,8 +890,8 @@ uint16_t uct_ib_mlx5_txwq_num_posted_wqes(const uct_ib_mlx5_txwq_t *txwq, | |
| ucs_assert(pi == txwq->hw_ci); | ||
| do { | ||
| ctrl = uct_ib_mlx5_txwq_get_wqe(txwq, pi); | ||
| wqe_size = (ctrl->qpn_ds >> 24) * UCT_IB_MLX5_WQE_SEG_SIZE; | ||
| pi += (wqe_size + MLX5_SEND_WQE_BB - 1) / MLX5_SEND_WQE_BB; | ||
| wqe_size = uct_ib_mlx5_wqe_size(ctrl); | ||
| pi = uct_ib_mlx5_txwq_next_wqe_index(pi, wqe_size); | ||
| ++count; | ||
| } while (pi != txwq->sw_pi); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,6 +64,8 @@ | |
| #define UCT_IB_MLX5_CQ_SET_CI 0 | ||
| #define UCT_IB_MLX5_CQ_ARM_DB 1 | ||
| #define UCT_IB_MLX5_LOG_MAX_MSG_SIZE 30 | ||
| #define UCT_IB_MLX5_PSN_BITS 24 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO |
||
| #define UCT_IB_MLX5_PSN_MASK UCS_MASK(UCT_IB_MLX5_PSN_BITS) | ||
| #define UCT_IB_MLX5_ATOMIC_MODE_COMP 1 | ||
| #define UCT_IB_MLX5_ATOMIC_MODE_EXT 3 | ||
| #define UCT_IB_MLX5_CQE_FLAG_L3_IN_DATA UCS_BIT(28) /* GRH/IP in the receive buffer */ | ||
|
|
@@ -709,7 +711,7 @@ typedef struct uct_ib_mlx5_txwq { | |
| static UCS_F_ALWAYS_INLINE uint32_t | ||
| uct_ib_mlx5_txwq_get_next_wqe_psn(const uct_ib_mlx5_txwq_t *txwq) | ||
| { | ||
| return txwq->next_wqe_psn & UCS_MASK(24); | ||
| return txwq->next_wqe_psn & UCT_IB_MLX5_PSN_MASK; | ||
| } | ||
|
|
||
|
|
||
|
|
@@ -917,6 +919,12 @@ ucs_status_t uct_ib_mlx5_txwq_init(uct_priv_worker_t *worker, | |
| /* Get pointer to a WQE by producer index */ | ||
| void *uct_ib_mlx5_txwq_get_wqe(const uct_ib_mlx5_txwq_t *txwq, uint16_t pi); | ||
|
|
||
| /* Get the WQE size in bytes from its control segment */ | ||
| size_t uct_ib_mlx5_wqe_size(const struct mlx5_wqe_ctrl_seg *ctrl); | ||
|
roiedanino marked this conversation as resolved.
Resolved
|
||
|
|
||
| /* Get the index of the WQE that follows a WQE of the given size */ | ||
| uint16_t uct_ib_mlx5_txwq_next_wqe_index(uint16_t index, size_t wqe_size); | ||
|
|
||
| /* Count how many WQEs are currently posted */ | ||
| uint16_t uct_ib_mlx5_txwq_num_posted_wqes(const uct_ib_mlx5_txwq_t *txwq, | ||
| uint16_t outstanding); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,11 +16,6 @@ | |||||||||||
| #include <string.h> | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| static void uct_ib_mlx5_wqe_dump(uct_ib_iface_t *iface, void *wqe, void *qstart, | ||||||||||||
| void *qend, int max_sge, int dump_qp, | ||||||||||||
| uct_log_data_dump_func_t packet_dump_cb, | ||||||||||||
| char *buffer, size_t max, uct_ib_log_sge_t *log_sge); | ||||||||||||
|
|
||||||||||||
| static void uct_ib_mlx5_resp_error_dump(const uct_ib_mlx5_srq_seg_t *seg, | ||||||||||||
| unsigned max_strides, char *buffer, | ||||||||||||
| size_t max); | ||||||||||||
|
|
@@ -281,10 +276,10 @@ static size_t uct_ib_mlx5_dump_dgram(char *buf, size_t max, void *seg, int is_et | |||||||||||
| UCT_IB_MLX5_AV_FULL_SIZE : UCT_IB_MLX5_AV_BASE_SIZE; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| static void uct_ib_mlx5_wqe_dump(uct_ib_iface_t *iface, void *wqe, void *qstart, | ||||||||||||
| void *qend, int max_sge, int dump_qp, | ||||||||||||
| uct_log_data_dump_func_t packet_dump_cb, | ||||||||||||
| char *buffer, size_t max, uct_ib_log_sge_t *log_sge) | ||||||||||||
| void uct_ib_mlx5_wqe_dump(uct_ib_iface_t *iface, void *wqe, void *qstart, | ||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. minor: pls re-align the argument list after dropping
Suggested change
|
||||||||||||
| void *qend, int max_sge, int dump_qp, | ||||||||||||
| uct_log_data_dump_func_t packet_dump_cb, | ||||||||||||
| char *buffer, size_t max, uct_ib_log_sge_t *log_sge) | ||||||||||||
| { | ||||||||||||
| static uct_ib_opcode_t opcodes[] = { | ||||||||||||
| [MLX5_OPCODE_NOP] = { "NOP", 0 }, | ||||||||||||
|
|
@@ -308,18 +303,26 @@ static void uct_ib_mlx5_wqe_dump(uct_ib_iface_t *iface, void *wqe, void *qstart, | |||||||||||
| uint8_t opcode = ctrl->opmod_idx_opcode >> 24; | ||||||||||||
| uint8_t opmod = ctrl->opmod_idx_opcode & 0xff; | ||||||||||||
| uint32_t qp_num = ntohl(ctrl->qpn_ds) >> 8; | ||||||||||||
| int ds = ctrl->qpn_ds >> 24; | ||||||||||||
| uct_ib_opcode_t *op = &opcodes[opcode]; | ||||||||||||
| int ds = ntohl(ctrl->qpn_ds) & UINT8_MAX; | ||||||||||||
| char *s = buffer; | ||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still present (belongs on the existing thread): this still uses the LE-only |
||||||||||||
| char *ends = buffer + max; | ||||||||||||
| const char* sg_prefix_arr = (op->flags & UCT_IB_OPCODE_FLAG_HAS_DMA) ? | ||||||||||||
| "GS" : NULL; | ||||||||||||
| uct_ib_opcode_t *op; | ||||||||||||
| const char *sg_prefix_arr; | ||||||||||||
| struct ibv_sge sg_list[16]; | ||||||||||||
| uint64_t inline_bitmap; | ||||||||||||
| int i, is_inline, is_eth; | ||||||||||||
| size_t dg_size; | ||||||||||||
| void *seg; | ||||||||||||
|
|
||||||||||||
| if ((opcode >= ucs_static_array_size(opcodes)) || | ||||||||||||
| (opcodes[opcode].name == NULL)) { | ||||||||||||
| snprintf(buffer, max, "unknown opcode 0x%x", opcode); | ||||||||||||
| return; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| op = &opcodes[opcode]; | ||||||||||||
| sg_prefix_arr = (op->flags & UCT_IB_OPCODE_FLAG_HAS_DMA) ? "GS" : NULL; | ||||||||||||
|
|
||||||||||||
| /* QP and WQE index */ | ||||||||||||
| if (dump_qp) { | ||||||||||||
| snprintf(s, ends - s, "QP 0x%x [%03ld] ", qp_num, | ||||||||||||
|
|
@@ -387,8 +390,8 @@ static void uct_ib_mlx5_wqe_dump(uct_ib_iface_t *iface, void *wqe, void *qstart, | |||||||||||
|
|
||||||||||||
| /* Extended atomic segment */ | ||||||||||||
| if (op->flags & UCT_IB_OPCODE_FLAG_HAS_EXT_ATOMIC) { | ||||||||||||
| uint64_t add, boundary, compare, swap, compare_mask, swap_mask; | ||||||||||||
| int size = 1 << ((opmod & 7) + 2); | ||||||||||||
| uint64_t add, boundary, compare, swap, compare_mask, swap_mask; | ||||||||||||
|
|
||||||||||||
| if (opcode == MLX5_OPCODE_ATOMIC_MASKED_FA) { | ||||||||||||
| add = network_to_host(seg, size); | ||||||||||||
|
|
||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Asymmetric endianness handling between the two new helpers is worth a one-line note. uct_ib_mlx5_wqe_size() correctly reads ds via ntohl(ctrl->qpn_ds) & UINT8_MAX (fixing the big-endian bug in the old >> 24), but uct_ib_mlx5_wqe_opcode() deliberately keeps ctrl->opmod_idx_opcode >> 24. This is correct because opmod_idx_opcode is built with host-order shifts rather than a plain htonl, so the opcode is only in the top byte on little-endian. The asymmetry looks like an oversight but is intentional — worth a short comment noting the opcode field is written in host order (unlike qpn_ds), so the raw shift is intended and not a missed BE fix. Optional.