-
Notifications
You must be signed in to change notification settings - Fork 1
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.
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
RC/KEEPALIVE: move KA progress into iface progress #184
RC/KEEPALIVE: move KA progress into iface progress #184
Conversation
src/uct/ib/rc/accel/rc_mlx5_iface.c
Outdated
if ((iface->keepalive.iter_count++ % UCP_WORKER_KEEPALIVE_ITER_SKIP) != 0) { | ||
return 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe do this check in uct_rc_mlx5_iface_progress?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe do this check in uct_rc_mlx5_iface_progress?
moved into face progress
if (ucs_unlikely(ucs_get_time() < iface->keepalive.time)) { | ||
return 0; | ||
} | ||
|
||
ucs_spin_lock(&iface->super.ep_list_lock); | ||
ucs_list_for_each(ep, &iface->super.ep_list, super.list) { | ||
ucs_trace("send keepalive grant on ep %p", ep); | ||
uct_rc_ep_fc_send_grant(&ep->super); | ||
} | ||
ucs_spin_unlock(&iface->super.ep_list_lock); | ||
|
||
uct_rc_mlx5_iface_print(iface, "keepalive"); | ||
|
||
iface->keepalive.time = ucs_get_time() + iface->config.ka_interval; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
put this in a noninline func
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
put this in a noninline func
marked as non-inline
@yosefe ok to squash? |
src/uct/ib/rc/accel/rc_mlx5_iface.c
Outdated
if ((iface->config.ka_interval != 0) && | ||
((iface->keepalive.iter_count++ % | ||
UCP_WORKER_KEEPALIVE_ITER_SKIP) != 0)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: can invert condition order. since we always enable keepalive, probably the condition with % will be false most of the times
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: can invert condition order. since we always enable keepalive, probably the condition with % will be false most of the times
I set such condition sequence to not update iter_count in case if KA is not enabled...
updated
- performance optimization: move KEEPALIVE progress call into iface progress
527b7df
to
8481059
Compare
squashed |
into iface progress