WIREUP: exclude ep_check lanes where KA supported from map #6039
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
src/ucp/wireup/select.c
Outdated
| for (lane = 0; lane < key->num_lanes; ++lane) { | ||
| /* add lanes to ep_check map */ | ||
| rsc_index = key->lanes[lane].rsc_index; | ||
| if (rsc_index == UCP_NULL_RESOURCE) { | ||
| continue; | ||
| } | ||
|
|
||
| dev_index = context->tl_rscs[rsc_index].dev_index; | ||
| ucs_assert(dev_index < (sizeof(dev_map_used) * 8)); | ||
| iface_attr = ucp_worker_iface_get_attr(worker, rsc_index); | ||
| if (iface_attr->cap.flags & UCT_IFACE_FLAG_EP_KEEPALIVE) { | ||
| dev_map_used |= UCS_BIT(dev_index); | ||
| } | ||
| } |
There was a problem hiding this comment.
can you do it inside one loop (below)?
hoopoepg
force-pushed
the
topic/update-ep-check-map
branch
from
2706e0a to
94a1f3e
Compare
src/ucp/wireup/select.c
Outdated
| /* exclude from ep_check map all devices where built-in | ||
| * keepalive is supported - mark all such devices as used */ | ||
| for (lane = 0; lane < key->num_lanes; ++lane) { |
There was a problem hiding this comment.
can we unite the 2 loops?
with the one in line 1539
There was a problem hiding this comment.
no: we need 2 loops (nested or pipe-lined) in any case
@brminich wanted to propose any unification, but still no news from him
There was a problem hiding this comment.
seems to be no clear way to unite given generic use-case.
There was a problem hiding this comment.
right, I didn't find any was... I can add some macro to make code bit shorter, but in fact there will be same pair of loops
There was a problem hiding this comment.
how about
for (lane ...) {
...
dev_index = ...
iface_attr = ...
if (FLAG_EP_KEEPALIVE) {
dev_map_used |= UCS_BIT(dev_index);
} else if ((!(UCS_BIT(dev_index) & dev_map_used) && EP_CHECK) {
kep_ep_check_map |= ...
dev_map_used |= UCS_BIT(dev_index);
}
}
There was a problem hiding this comment.
or maybe precompute it during worker init?
There was a problem hiding this comment.
how about
will not work: in case if any lane-bit is added to ep_check map and later we found that another lane on same device has built-in keepalive, then we have to remove such lane-bit from ep_check map - this is issue. and we can't pre-compile it in worker init because in general case there is no correllation between ep_check_map and devices
There was a problem hiding this comment.
ok. so please add comments.
to 1st loop: /* find all devices with built-in keepalive support */
for 2nd loop: /* send ep_check on devices without built-in keepalive */
- exclude from ep_check map lanes where built-in keepalive feature is supported - updated ep-check gtests to disable built-in keepalive
hoopoepg
force-pushed
the
topic/update-ep-check-map
branch
from
94a1f3e to
ea23a9d
Compare
|
io_demo failed |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
failed due to #6027 |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
feature is supported