Skip to content

Commit 858ffea

Browse files
tvegas1tvegas1
authored
UCP/WIREUP: Improve p2p lane matching diagnostics (#11912)
1 parent d5b6b27 commit 858ffea

6 files changed

Lines changed: 66 additions & 36 deletions

File tree

‎src/ucp/core/ucp_ep.c‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright (c) NVIDIA CORPORATION & AFFILIATES, 2001-2020. ALL RIGHTS RESERVED.
2+
* Copyright (c) NVIDIA CORPORATION & AFFILIATES, 2001-2026. ALL RIGHTS RESERVED.
33
* Copyright (C) Los Alamos National Security, LLC. 2019 ALL RIGHTS RESERVED.
44
*
55
* See file LICENSE for terms.
@@ -1656,12 +1656,11 @@ ucp_ep_set_failed(ucp_ep_h ucp_ep, ucp_lane_index_t lane, ucs_status_t status)
16561656
log_level = ucp_ep_config_err_handling_enabled(ucp_ep) ?
16571657
UCS_LOG_LEVEL_ERROR : UCS_LOG_LEVEL_DIAG;
16581658

1659-
ucp_ep_get_lane_info_str(ucp_ep, lane, &lane_info_strb);
16601659
ucs_log(log_level,
1661-
"ep %p: error '%s' on %s will not be handled"
1660+
UCP_EP_LANE_FMT ": error '%s' will not be handled"
16621661
" since no error callback is installed",
1663-
ucp_ep, ucs_status_string(status),
1664-
ucs_string_buffer_cstr(&lane_info_strb));
1662+
UCP_EP_LANE_ARG(ucp_ep, lane, &lane_info_strb),
1663+
ucs_status_string(status));
16651664
return UCS_ERR_UNSUPPORTED;
16661665
} else {
16671666
ucp_ep_invoke_err_cb(ucp_ep, status);
@@ -4340,24 +4339,25 @@ void ucp_ep_get_tl_bitmap(const ucp_ep_config_key_t *key,
43404339
}
43414340
}
43424341

4343-
void ucp_ep_get_lane_info_str(ucp_ep_h ucp_ep, ucp_lane_index_t lane,
4344-
ucs_string_buffer_t *lane_info_strb)
4342+
const char *ucp_ep_get_lane_info_str(ucp_ep_h ucp_ep, ucp_lane_index_t lane,
4343+
ucs_string_buffer_t *lane_info_strb)
43454344
{
4346-
ucp_rsc_index_t rsc_index;
4347-
uct_tl_resource_desc_t *tl_rsc;
4345+
ucs_string_buffer_reset(lane_info_strb);
43484346

43494347
if (lane == UCP_NULL_LANE) {
4350-
ucs_string_buffer_appendf(lane_info_strb, "NULL lane");
4348+
ucs_string_buffer_appendf(lane_info_strb, "no lane");
43514349
} else if (lane == ucp_ep_get_cm_lane(ucp_ep)) {
4352-
ucs_string_buffer_appendf(lane_info_strb, "CM lane");
4350+
ucs_string_buffer_appendf(lane_info_strb, "lane[%d] cm", lane);
43534351
} else {
4354-
rsc_index = ucp_ep_get_rsc_index(ucp_ep, lane);
4355-
tl_rsc = &ucp_ep->worker->context->tl_rscs[rsc_index].tl_rsc;
4356-
43574352
ucs_string_buffer_appendf(lane_info_strb,
4358-
UCT_TL_RESOURCE_DESC_FMT,
4359-
UCT_TL_RESOURCE_DESC_ARG(tl_rsc));
4353+
"lane[%d] " UCT_TL_RESOURCE_DESC_FMT ".%d",
4354+
lane,
4355+
UCT_TL_RESOURCE_DESC_ARG(
4356+
ucp_ep_get_tl_rsc(ucp_ep, lane)),
4357+
ucp_ep_get_path_index(ucp_ep, lane));
43604358
}
4359+
4360+
return ucs_string_buffer_cstr(lane_info_strb);
43614361
}
43624362

43634363
void ucp_ep_invoke_err_cb(ucp_ep_h ep, ucs_status_t status)

‎src/ucp/core/ucp_ep.h‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@
2626
#define UCP_MAX_IOV 16UL
2727

2828

29+
/* Print an ep lane, using a caller-provided string buffer as scratch space */
30+
#define UCP_EP_LANE_FMT "ep %p: %s"
31+
#define UCP_EP_LANE_ARG(_ep, _lane, _strb) \
32+
(_ep), ucp_ep_get_lane_info_str(_ep, _lane, _strb)
33+
34+
2935
/* Endpoint flags type */
3036
#if ENABLE_DEBUG_DATA || UCS_ENABLE_ASSERT
3137
typedef uint32_t ucp_ep_flags_t;
@@ -868,8 +874,8 @@ size_t ucp_ep_tag_offload_min_rndv_thresh(ucp_context_h context,
868874
void ucp_ep_config_rndv_zcopy_commit(ucp_lane_index_t lanes_count,
869875
ucp_ep_rndv_zcopy_config_t *rndv_zcopy);
870876

871-
void ucp_ep_get_lane_info_str(ucp_ep_h ucp_ep, ucp_lane_index_t lane,
872-
ucs_string_buffer_t *lane_info_strb);
877+
const char *ucp_ep_get_lane_info_str(ucp_ep_h ucp_ep, ucp_lane_index_t lane,
878+
ucs_string_buffer_t *lane_info_strb);
873879

874880
void ucp_ep_config_rndv_zcopy_commit(ucp_lane_index_t lanes_count,
875881
ucp_ep_rndv_zcopy_config_t *rndv_zcopy);

‎src/ucp/wireup/address.c‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1853,12 +1853,15 @@ ucs_status_t ucp_address_unpack(ucp_worker_t *worker, const void *buffer,
18531853
}
18541854

18551855
ucp_address_trace(unpack_flags,
1856-
"unpack addr[%d] : sysdev %d paths %d eps %u"
1857-
" tl_flags 0x%" PRIx64 " bw %.2f/nMBs"
1856+
"unpack addr[%d] : %s md[%d] sysdev %d paths %d"
1857+
" eps %u tl_flags 0x%" PRIx64 " bw %.2f/nMBs"
18581858
" ovh %.0fns lat_ovh %.0fns dev_priority %d"
18591859
" a32 0x%" PRIx64 "/0x%" PRIx64 " a64 0x%" PRIx64
18601860
"/0x%" PRIx64,
1861-
(int)(address - address_list), address->sys_dev,
1861+
(int)(address - address_list),
1862+
ucp_find_tl_name_by_csum(worker->context,
1863+
address->tl_name_csum),
1864+
address->md_index, address->sys_dev,
18621865
address->dev_num_paths, address->num_ep_addrs,
18631866
address->iface_attr.flags,
18641867
address->iface_attr.bandwidth / UCS_MBYTE,

‎src/ucp/wireup/select.c‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2888,8 +2888,17 @@ ucp_wireup_construct_lanes(const ucp_wireup_select_params_t *select_params,
28882888
key->lanes[lane].path_index = ucp_wireup_default_path_index(
28892889
select_ctx->lane_descs[lane].path_index);
28902890

2891-
ucs_trace("ep %p: construct lane %d to addr_index %d", ep, lane,
2892-
select_ctx->lane_descs[lane].addr_index);
2891+
rsc_index = select_ctx->lane_descs[lane].rsc_index;
2892+
if (rsc_index == UCP_NULL_RESOURCE) {
2893+
ucs_trace("ep %p: construct lane %d cm", ep, lane);
2894+
} else {
2895+
ucs_trace("ep %p: construct lane %d " UCT_TL_RESOURCE_DESC_FMT
2896+
".%d to addr_index %d", ep, lane,
2897+
UCT_TL_RESOURCE_DESC_ARG(
2898+
&context->tl_rscs[rsc_index].tl_rsc),
2899+
key->lanes[lane].path_index,
2900+
select_ctx->lane_descs[lane].addr_index);
2901+
}
28932902

28942903
if (select_ctx->lane_descs[lane].lane_types & UCS_BIT(UCP_LANE_TYPE_CM)) {
28952904
ucs_assert(key->cm_lane == UCP_NULL_LANE);

‎src/ucp/wireup/wireup.c‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,8 @@ ucp_wireup_match_p2p_lanes(ucp_ep_h ep,
444444
const unsigned *addr_indices,
445445
ucp_lane_index_t *lanes2remote)
446446
{
447+
UCS_STRING_BUFFER_ONSTACK(lane_strb, 64);
448+
ucp_context_h context = ep->worker->context;
447449
const ucp_address_entry_t *address;
448450
unsigned address_index;
449451
ucp_lane_index_t lane, remote_lane, num_lanes;
@@ -474,21 +476,29 @@ ucp_wireup_match_p2p_lanes(ucp_ep_h ep,
474476
/* Select next remote ep address within the address_index as specified
475477
* by addr_indices argument
476478
*/
477-
address_index = addr_indices[lane];
478-
address = &remote_address->address_list[address_index];
479-
ep_addr_index = ep_addr_indexes[address_index]++;
480-
ucs_assertv(ep_addr_index < address->num_ep_addrs,
481-
"lane=%d/%d tl_name_csum=0x%02x address_index=%u "
482-
"ep_addr_index=%u num_ep_addrs=%u",
483-
lane, num_lanes, address->tl_name_csum, address_index,
484-
ep_addr_index, address->num_ep_addrs);
479+
address_index = addr_indices[lane];
480+
address = &remote_address->address_list[address_index];
481+
ep_addr_index = ep_addr_indexes[address_index]++;
482+
if (ep_addr_index >= address->num_ep_addrs) {
483+
/* The peer packs one ep address per own lane on that resource, so
484+
* this ep selected more lanes towards it than the peer provides */
485+
ucs_fatal(UCP_EP_LANE_FMT " -> addr[%u] %s: no ep address %u,"
486+
" only %u provided",
487+
UCP_EP_LANE_ARG(ep, lane, &lane_strb), address_index,
488+
ucp_find_tl_name_by_csum(context, address->tl_name_csum),
489+
ep_addr_index, address->num_ep_addrs);
490+
}
491+
485492
remote_lane = address->ep_addrs[ep_addr_index].lane;
486493
lanes2remote[lane] = remote_lane;
487494

488495
if (used_remote_lanes & UCS_BIT(remote_lane)) {
489-
ucs_fatal("ep %p: remote lane %d is used more than once", ep,
496+
ucs_fatal(UCP_EP_LANE_FMT " -> addr[%u]: remote lane %d is"
497+
" used more than once",
498+
UCP_EP_LANE_ARG(ep, lane, &lane_strb), address_index,
490499
remote_lane);
491500
}
501+
492502
used_remote_lanes |= UCS_BIT(remote_lane);
493503

494504
ucs_trace("ep %p: lane[%d]->remote_lane[%d] (address[%d].ep_address[%d])",
@@ -524,6 +534,7 @@ ucp_wireup_connect_local(ucp_ep_h ep,
524534
const ucp_unpacked_address_t *remote_address,
525535
const ucp_lane_index_t *lanes2remote)
526536
{
537+
UCS_STRING_BUFFER_ONSTACK(lane_strb, 64);
527538
ucp_lane_index_t lane, remote_lane;
528539
const ucp_address_entry_t *address_entry;
529540
const ucp_address_entry_ep_addr_t *ep_entry;
@@ -542,8 +553,9 @@ ucp_wireup_connect_local(ucp_ep_h ep,
542553
status = ucp_wireup_find_remote_p2p_addr(ep, remote_lane, remote_address,
543554
&address_entry, &ep_entry);
544555
if (status != UCS_OK) {
545-
ucs_error("ep %p: no remote ep address for lane[%d]->remote_lane[%d]",
546-
ep, lane, remote_lane);
556+
ucs_error(UCP_EP_LANE_FMT ": no remote ep address for remote"
557+
" lane %d",
558+
UCP_EP_LANE_ARG(ep, lane, &lane_strb), remote_lane);
547559
goto out;
548560
}
549561

‎test/gtest/ucp/test_ucp_sockaddr.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -823,8 +823,8 @@ class test_ucp_sockaddr : public ucp_test {
823823
if (level == UCS_LOG_LEVEL_ERROR) {
824824
std::string err_str = format_message(message, ap);
825825

826-
if (err_str.find("on CM lane will not be handled since no error"
827-
" callback is installed") != std::string::npos) {
826+
if (err_str.find("will not be handled since no error callback is"
827+
" installed") != std::string::npos) {
828828
UCS_TEST_MESSAGE << "< " << err_str << " >";
829829
++m_err_count;
830830
return UCS_LOG_FUNC_RC_STOP;

0 commit comments

Comments
 (0)