Skip to content
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

UCT/MLX5/UCS: Print error message if SRQ topology is invalid #10479

Merged
merged 1 commit into from
Feb 12, 2025
Merged

UCT/MLX5/UCS: Print error message if SRQ topology is invalid #10479

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
7 changes: 2 additions & 5 deletions src/tools/perf/perftest_run.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,8 @@ void print_progress(void *UCS_V_UNUSED rte_group,
!(ctx->flags & TEST_FLAG_PRINT_CSV)) {
if (ctx->flags & TEST_FLAG_PRINT_FINAL) {
/* Print test name in the final and only output line */
for (i = 0; i < ctx->num_batch_files; ++i) {
ucs_string_buffer_appendf(&test_name, "%s/",
ctx->test_names[i]);
}
ucs_string_buffer_rtrim(&test_name, "/");
ucs_string_buffer_append_array(&strb, "/", "%s", ctx->test_names,
ctx->num_batch_files);
ucs_string_buffer_appendf(&strb, "%10s",
ucs_string_buffer_cstr(&test_name));
} else {
Expand Down
23 changes: 23 additions & 0 deletions src/ucs/datastruct/string_buffer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,29 @@ void ucs_string_buffer_translate(ucs_string_buffer_t *strb,
_tok != NULL; \
_tok = ucs_string_buffer_next_token(_strb, _tok, _delim))


/**
* Append a generic array, separated by a custom token.
*
* @param _strb String buffer to append to.
* @param _sep Use this string as the separator.
* @param _fmt Formal string for a single element.
* @param _array Array of elements to append.
* @param _count Number of elements in the array.
*/
#define ucs_string_buffer_append_array(_strb, _sep, _fmt, _array, _count) \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, now it's cool
Just one minor thing: currently this is suitable for C arrays, maybe it makes sense to have an "overload" for ucs_array?

// C array
#define ucs_string_buffer_append_carray(_strb, _sep, _fmt, _array, _count)  << current impl

#define ucs_string_buffer_append_array(_strb, _sep, _fmt, _array) \  << overload for UCS array
    ucs_string_buffer_append_carray(_strb, _sep, _fmt, _array.buffer, _array.length)

Up to you, not really critical

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Today we don't have a place we use it for ucs_array so IMO it's not needed

{ \
size_t _i; \
\
if ((_count) > 0) { \
ucs_string_buffer_appendf((_strb), _fmt, (_array)[0]); \
tvegas1 marked this conversation as resolved.
Show resolved Hide resolved
} \
for (_i = 1; _i < (_count); ++_i) { \
ucs_string_buffer_appendf((_strb), "%s" _fmt, (_sep), \
(_array)[_i]); \
} \
}

END_C_DECLS

#endif
7 changes: 7 additions & 0 deletions src/uct/ib/mlx5/rc/rc_mlx5_iface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@ uct_rc_mlx5_iface_parse_srq_topo(uct_ib_mlx5_md_t *md,
((init_attr->qp_type == UCT_IB_QPT_DCI) ?
UCT_IB_MLX5_MD_FLAG_DEVX_DC_SRQ :
UCT_IB_MLX5_MD_FLAG_DEVX_RC_SRQ);
ucs_string_buffer_t strb = UCS_STRING_BUFFER_INITIALIZER;
int i;

for (i = 0; i < config->srq_topo.count; ++i) {
Expand All @@ -403,6 +404,12 @@ uct_rc_mlx5_iface_parse_srq_topo(uct_ib_mlx5_md_t *md,
}
}

ucs_string_buffer_append_array(&strb, ",", "%s", config->srq_topo.types,
config->srq_topo.count);
ucs_error("%s: none of the provided SRQ topology modes %s is supported",
uct_ib_device_name(&md->super.dev),
ucs_string_buffer_cstr(&strb));
ucs_string_buffer_cleanup(&strb);
return UCS_ERR_INVALID_PARAM;
}

Expand Down
14 changes: 14 additions & 0 deletions test/gtest/ucs/test_string.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,20 @@ UCS_TEST_F(test_string_buffer, flags) {
EXPECT_EQ(std::string("one|three"), ucs_string_buffer_cstr(&strb));
}

UCS_TEST_F(test_string_buffer, array) {
static const char *str_array[] = {"once", "upon", "a", "time"};
UCS_STRING_BUFFER_ONSTACK(strb, 128);
ucs_string_buffer_append_array(&strb, " ", "%s", str_array,
ucs_static_array_size(str_array));
EXPECT_EQ(std::string("once upon a time"), ucs_string_buffer_cstr(&strb));

ucs_string_buffer_reset(&strb);
static int num_array[] = {1, 2, 3, 4};
ucs_string_buffer_append_array(&strb, ",", "%d", num_array,
ucs_static_array_size(num_array));
EXPECT_EQ(std::string("1,2,3,4"), ucs_string_buffer_cstr(&strb));
}

UCS_TEST_F(test_string_buffer, dump) {
UCS_STRING_BUFFER_ONSTACK(strb, 128);
ucs_string_buffer_appendf(&strb, "hungry\n");
Expand Down
Loading