RC_MLX5/IFACE: fixed assert #6046
Conversation
hoopoepg
commented
Dec 18, 2020
•
hoopoepg
commented
- fixed issue in iface recv prepost when adaptive progress enabled
- fixed assert: rc_mlx5_common.c:128 Assertion `rc_iface->rx.srq.available >= count' failed
- don't try prepost recv twice
hoopoepg
force-pushed
the
topic/fixed-assert-adaptive-routing
branch
from
d90a5cb
to
c2d9d08
Compare
src/uct/ib/rc/accel/rc_mlx5_iface.c
Outdated
| (iface->super.rx.srq.quota > 0)) { /* prepost recvs only if quota available | ||
| * (recvs were not preposted before) */ |
There was a problem hiding this comment.
- nice catch 👍
- is it fixing the srq avail assertion? can you add this to PR description?
- maybe move the >0 check inside uct_rc_mlx5_iface_common_prepost_recvs?
There was a problem hiding this comment.
- pls submit to v1.10.x as well
There was a problem hiding this comment.
imo, >0 check better to keep here, because uct_rc_mlx5_iface_common_prepost_recvs is called much more frequently, so check would add additional overhead
There was a problem hiding this comment.
i see uct_rc_mlx5_iface_common_prepost_recvs is called only from uct_rc_mlx5_iface_progress_enable and dc iface init, what am i missing?
There was a problem hiding this comment.
BTW, probably good to add ucs_assert(available>0) to uct_rc_mlx5_iface_srq_post_recv
There was a problem hiding this comment.
may fault, mixed it with uct_rc_mlx5_iface_srq_post_recv
There was a problem hiding this comment.
is it fixing the srq avail assertion? can you add this to PR description?
yes, updated PR/commit comment
maybe move the >0 check inside uct_rc_mlx5_iface_common_prepost_recvs?
moved to uct_rc_mlx5_iface_common_prepost_recvs
good to add ucs_assert(available>0) to uct_rc_mlx5_iface_srq_post_recv
added new assert
pls submit to v1.10.x as well
will do backport after merge into master
hoopoepg
force-pushed
the
topic/fixed-assert-adaptive-routing
branch
from
c2d9d08
to
1e5e1d8
Compare
src/uct/ib/rc/accel/rc_mlx5_common.c
Outdated
| uct_rc_mlx5_iface_srq_post_recv(iface); | ||
| /* prepost recvs only if quota available | ||
| * (recvs were not preposted before) */ | ||
| if (iface->super.rx.srq.quota > 0) { |
src/uct/ib/rc/accel/rc_mlx5_common.c
Outdated
| iface->super.rx.srq.quota = 0; | ||
| uct_rc_mlx5_iface_srq_post_recv(iface); | ||
| /* prepost recvs only if quota available | ||
| * (recvs were not preposted before) */ |
There was a problem hiding this comment.
hmmm... unbuitified :)
@hoopoepg do you mean adaptive progress in the description? |
- fixed issue in iface recv prepost when adaptive routing enabled - fixed assert: rc_mlx5_common.c:128 Assertion `rc_iface->rx.srq.available >= count' failed - don't try prepost recv twice
hoopoepg
force-pushed
the
topic/fixed-assert-adaptive-routing
branch
from
1e5e1d8
to
daa69c5
Compare
yes, fixed comment |
|
@hoopoepg can you pls also port to v1.10.x and int3 branches? |
|
io_demo issue |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
