UCP/PROTO: Consider FAST_CMPL flag only for eager protocols #10277
Conversation
| params->super.super.select_param->op_attr); | ||
|
|
||
| if ((op_attr_mask & UCP_OP_ATTR_FLAG_FAST_CMPL)) { | ||
| ucs_warn("RNDV is initialized with FAST_CMPL which is unexpected. " |
There was a problem hiding this comment.
maybe ucs_debug since fast_cmpl is best effort and UCP users should (generally) not even be aware that a multi step protocol is used?
There was a problem hiding this comment.
That warning means that user passed FAST_CMPL flag to recv operation and in case of choosing RNDV it can cause undesired performance prediction change. So I would prefer to leave it as warning so users can see that and remove FAST_CMPL from recv operation.
| @@ -630,6 +644,8 @@ ucp_proto_rndv_bulk_init(const ucp_proto_multi_init_params_t *init_params, | |||
| const char *proto_name; | |||
| ucs_status_t status; | |||
|
|
|||
| ucp_proto_rndv_report_fast_cmpl(init_params); | |||
There was a problem hiding this comment.
seems we remove FAST_CMPL when we handle RTR. So in which case we sould still have FAST_CMPL here?
maybe it should be an assertion?
There was a problem hiding this comment.
AFAIU that can happen if customer set FAST_CMPL flag for receive operation. In that case RNDV-get would be initialized with FAST_CMPL flag
There was a problem hiding this comment.
so maybe make the warning more clear, or move it to tag_recv_nbx, or mask-out FAST_CMPL flag during tag_recv?
There was a problem hiding this comment.
I can move this warning to tag_recv_nbx (or maybe ucp_proto_rndv_receive_start) but then it would be extra operation on the fast path. And if we want to print a waring/debug message in that case it would cost extra if on fast path. Are we OK with that?
Co-authored-by: Yossi Itigin <[email protected]>
What?
Remove considering FAST_CMPL flag for RNDV protocols
Why?
FAST_CMPL shouldn't affect RNDV protocols since they can be completed only when all operations are done.