UCP/PROTO: Disable protocols that need memory type copy #10490

tvegas1 · 2025-02-13T17:27:06Z

What?

Make sure to disable any protocol using locally or remotely, a memory type based copy.

Why?

Need to avoid all cuda calls from under ucp_worker_progress(). Cuda memory queries are fine since they are performed under send/recv operations. cuda_ipc transport can be disabled altogether, and gdr_copy is anyways not of interest when specifying UCX_MEMTYPE_AVOID_COPY=y.

How?

Disable applicable protocols altogether, for any memory type (cuda/host..).

Test

Might need to make inline sizes to 0, see below otherwise.

UCX_IB_TX_INLINE_RESP=0 UCX_MEMTYPE_AVOID_COPY=y ucx_perftest
UCX_IB_TX_INLINE_RESP=0 UCX_MEMTYPE_AVOID_COPY=y ucx_perftest -t ucp_put_bw -m cuda,cuda rock

Used by perftest to exchange psn_t sn:

  | perftest self cfg#2 | remote memory read by ucp_get* into host memory from cuda    |
  +---------------------+-------------------------------------+------------------------+
  |             65..inf | zero-copy                           | rc_mlx5/mlx5_2:1/path0 |

iyastreb · 2025-02-14T06:50:14Z

src/ucp/tag/offload/rndv.c

@@ -192,7 +192,8 @@ ucp_tag_rndv_offload_sw_proto_probe(const ucp_proto_init_params_t *init_params)
    ucp_proto_rndv_ctrl_priv_t rpriv;

    if (!ucp_tag_rndv_check_op_id(init_params) ||
-        !ucp_ep_config_key_has_tag_lane(init_params->ep_config_key)) {
+        !ucp_ep_config_key_has_tag_lane(init_params->ep_config_key) ||


Maybe add a simple mock test to verify the protocol selection?

need to discuss because there are many protocols to tests in that case.

iyastreb · 2025-02-14T06:58:48Z

src/ucp/am/eager_multi.c

@@ -49,7 +49,8 @@ ucp_am_eager_multi_bcopy_proto_probe(const ucp_proto_init_params_t *init_params)
    };

    if (!ucp_am_check_init_params(init_params, UCP_PROTO_AM_OP_ID_MASK,
-                                  UCP_PROTO_SELECT_OP_FLAG_AM_RNDV)) {
+                                  UCP_PROTO_SELECT_OP_FLAG_AM_RNDV) ||
+        context->config.ext.avoid_copy_mem_types) {


is it feasible to move this check in ucp_proto_init_check_op and then apply it when send_op != ZCOPY?

eager zcopy should also be disabled

fixed, and egr single too

brminich · 2025-02-14T07:17:51Z

src/ucp/am/eager_multi.c

@@ -49,7 +49,8 @@ ucp_am_eager_multi_bcopy_proto_probe(const ucp_proto_init_params_t *init_params)
    };

    if (!ucp_am_check_init_params(init_params, UCP_PROTO_AM_OP_ID_MASK,
-                                  UCP_PROTO_SELECT_OP_FLAG_AM_RNDV)) {
+                                  UCP_PROTO_SELECT_OP_FLAG_AM_RNDV) ||
+        context->config.ext.avoid_copy_mem_types) {


eager zcopy should also be disabled

brminich · 2025-02-14T07:20:40Z

src/ucp/am/eager_single.c

@@ -118,7 +118,8 @@ ucp_am_eager_short_probe_common(const ucp_proto_init_params_t *init_params,

    if (!ucp_am_check_init_params(init_params, UCS_BIT(op_id),
                                  UCP_PROTO_SELECT_OP_FLAG_AM_RNDV) ||
-        !ucp_proto_is_short_supported(select_param)) {
+        !ucp_proto_is_short_supported(select_param) ||
+        init_params->worker->context->config.ext.avoid_copy_mem_types) {


maybe you could define a wrapper on top of ucp_am_check_init_params to use for all AM eager protocols and check for avoid_copy_mem_types inside

brminich · 2025-02-14T07:53:33Z

src/ucp/rndv/rndv_rkey_ptr.c

@@ -396,7 +396,7 @@ ucp_proto_t ucp_rndv_rkey_ptr_mtype_proto = {
    .name     = "rndv/rkey_ptr/mtype",
    .desc     = "copy to mapped remote memory",
    .flags    = 0,
-    .probe     = ucp_proto_rndv_rkey_ptr_mtype_probe,
+    .probe    = ucp_proto_rndv_rkey_ptr_mtype_probe,


unrelated
also we need to disable rkey_ptr_mtype

it is disabled as it uses ucp_proto_rndv_mtype_init() which contains the added check. restored space

brminich · 2025-02-14T07:55:43Z

src/ucp/rndv/rndv_am.c

@@ -120,6 +120,10 @@ static void ucp_rndv_am_bcopy_probe(const ucp_proto_init_params_t *init_params)
        .middle.tl_cap_flags = UCT_IFACE_FLAG_AM_BCOPY
    };

+    if (init_params->worker->context->config.ext.avoid_copy_mem_types) {


pls move it to ucp_rndv_am_probe_common, because we need to also disable am_zcopy (it also implies unpacking on the receiver)

brminich · 2025-02-14T07:59:42Z

src/ucp/rma/amo_offload.c

@@ -177,7 +177,8 @@ static void ucp_proto_amo_probe(const ucp_proto_init_params_t *init_params,
    };

    if ((init_params->select_param->dt_class != UCP_DATATYPE_CONTIG) ||
-        !ucp_proto_init_check_op(init_params, UCS_BIT(op_id))) {
+        !ucp_proto_init_check_op(init_params, UCS_BIT(op_id)) ||
+        init_params->worker->context->config.ext.avoid_copy_mem_types) {


maybe you can disable it only for non-host memtypes?

should be fixed now

rakhmets · 2025-02-14T14:09:50Z

src/ucp/core/ucp_context.c

+  {"MEMTYPE_AVOID_COPY", "n",
+   "Avoid memory type copies when activated.\n",
+   ucs_offsetof(ucp_context_config_t, avoid_copy_mem_types), UCS_CONFIG_TYPE_BOOL},


Suggested change

{"MEMTYPE_AVOID_COPY", "n",

"Avoid memory type copies when activated.\n",

ucs_offsetof(ucp_context_config_t, avoid_copy_mem_types), UCS_CONFIG_TYPE_BOOL},

{"MEMTYPE_COPY_ENABLE", "y",

"Enable memory type copies",

ucs_offsetof(ucp_context_config_t, mem_type_copy_enable), UCS_CONFIG_TYPE_BOOL},

ok to fix from my side, since we had agreed on naming, @yosefe @brminich shall we go ahead with MEMTYPE_COPY_ENABLE?

Just to explain my comment. We have dozens UCX variables ended by "ENABLE", but with "AVOID". And I think it is inconvenient to set a negative value to a variable with a negative in the name to enable a feature.

UCP/PROTO: Disable protocols that need memory type copy

5dedabe

iyastreb reviewed Feb 14, 2025

View reviewed changes

brminich reviewed Feb 14, 2025

View reviewed changes

rakhmets reviewed Feb 14, 2025

View reviewed changes

tvegas1 added 3 commits February 14, 2025 17:26

UCP/PROTO: Disable protocols that need memory type copy

545bbee

UCP/PROTO: Disable protocols that need memory type copy

943d3e2

UCP/PROTO: Disable protocols that need memory type copy

ecdb39e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UCP/PROTO: Disable protocols that need memory type copy #10490

UCP/PROTO: Disable protocols that need memory type copy #10490

tvegas1 commented Feb 13, 2025

iyastreb Feb 14, 2025

tvegas1 Feb 14, 2025

iyastreb Feb 14, 2025

brminich Feb 14, 2025

tvegas1 Feb 14, 2025

brminich Feb 14, 2025

brminich Feb 14, 2025

tvegas1 Feb 14, 2025

tvegas1 Feb 14, 2025

brminich Feb 14, 2025

tvegas1 Feb 14, 2025

brminich Feb 14, 2025

tvegas1 Feb 14, 2025

tvegas1 Feb 14, 2025

brminich Feb 14, 2025

tvegas1 Feb 14, 2025

rakhmets Feb 14, 2025 •

edited

Loading

tvegas1 Feb 14, 2025

rakhmets Feb 14, 2025 •

edited

Loading

UCP/PROTO: Disable protocols that need memory type copy #10490

Are you sure you want to change the base?

UCP/PROTO: Disable protocols that need memory type copy #10490

Conversation

tvegas1 commented Feb 13, 2025

What?

Why?

How?

Test

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rakhmets Feb 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rakhmets Feb 14, 2025 • edited Loading

Choose a reason for hiding this comment

rakhmets Feb 14, 2025 •

edited

Loading

rakhmets Feb 14, 2025 •

edited

Loading