已合并
[Fix] 修改 HcommChannelNotifyWaitOnThread 的 timeout 参数单位为:ms->s #4625
Yuanhao Ji创建于 12 天前
[Fix] 修改 HcommChannelNotifyWaitOnThread 的 timeout 参数单位为:ms->s #4625
已合并
Yuanhao Ji创建于 12 天前
Yuanhao Ji
Yuanhao Ji成员
12 天前

描述

[Fix] 修改 HcommChannelNotifyWaitOnThread 的 timeout 参数单位为:ms->s

变更类型

请选择本次引入的变更类型:

关联的Issue

测试

已完成的测试用例和场景:

  1. A5 RDV全部通过

补充的UT用例:

文档更新

合入检查

likedislike
Pull Request已成功合入, 合并人@CANN-robot
(感谢 Yuanhao Ji 的贡献)
Yuanhao JiYuanhao Ji成员
12 天前 创建了 pull request,commit 3702a2a3
atomgit-bot
atomgit-bot
12 天前 评论:

变更摘要

本 PR 主要修正了 HcommChannelNotifyWaitOnThread 相关等待接口中 timeout 参数的时间单位,将其从毫秒(ms)统一改为秒(s),并同步调整默认超时常量。改动涉及 experimental/base_comm/channel/host_cpu_roce_channel.ccsrc/base_comm/resources/endpoint_pairs/channels/host/host_cpu_roce_channel.cc 两处实现,通过把 std::chrono::milliseconds 替换为 std::chrono::seconds,使 NotifyWaitNotifyWaitHybrid 的等待时长计算与新的秒级单位保持一致。

主要改动

  • 默认超时常量单位调整:将默认超时值由 DEFAULT_NOTIFY_POLL_TIMEOUT_MS = 30000 改为 DEFAULT_NOTIFY_WAIT_TIMEOUT_S = 30(并在另一文件中新增同名常量),数值从毫秒量级的 30000 调整为秒量级的 30
  • NotifyWait 等待时长单位修正:将 std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::milliseconds(timeout)) 改为使用 std::chrono::seconds(timeout),使 timeout 参数按秒解析。
  • NotifyWaitHybrid 默认值与轮询单位修正:将 pollTimeout 的默认值从 DEFAULT_NOTIFY_POLL_TIMEOUT_MS 或硬编码的 30000 改为 DEFAULT_NOTIFY_WAIT_TIMEOUT_S,并将 waitTimestd::chrono::milliseconds(pollTimeout) 改为 std::chrono::seconds(pollTimeout)
likedislike
atomgit-bot
atomgit-bot
12 天前 评论:

代码审查

审查结论

我已完成对两个变更文件的逐一审查:

  • src/base_comm/resources/endpoint_pairs/channels/host/host_cpu_roce_channel.cc —— 发现 2 个问题(P2 忙轮询、P3 timeout==0 语义不一致)
  • experimental/base_comm/channel/host_cpu_roce_channel.cc —— 发现 1 个问题(P2 忙轮询)

发现汇总(按优先级)

优先级 数量 说明
P0 0
P1 0
P2 2 NotifyWait 非混合路径无休眠忙轮询,超时单位 ms→s 后 CPU 空转与 cq_mutex 持锁时长被放大千倍(两个文件各 1 处)
P3 1 timeout==0NotifyWait(立即超时)与 NotifyWaitHybrid(默认 30s)间语义不一致

总体风险判断

本次改动本身方向正确——调用链(HcommSetNotifyWaitTimeOut/GetNotifyWaitTimeOutHcommChannelNotifyWaitOnThread)全程以“秒”为单位并打印 timeOut[%u s],此前 channel 侧按毫秒解释确实是一个 bug,改为秒是正确的修复;常量重命名(DEFAULT_NOTIFY_POLL_TIMEOUT_MSDEFAULT_NOTIFY_WAIT_TIMEOUT_S)无残留引用,无编译问题。

但该修复把非混合 NotifyWait 缺失退避的忙轮询从“约 1.8 秒”放大到“最坏约 30 分钟”,属于中风险性能/可靠性回归,建议在合入前为轮询循环补上退避睡眠;timeout==0 的两模式语义分歧属低风险遗留问题,可顺手对齐。

类型 数量
🔴 阻塞 0
🟡 建议 2

💬 仅评论

likedislike
CANN-robotCANN-robot成员
12 天前 添加了label:cann-cla/yes
CANN-robot
CANN-robot成员
12 天前 评论:

Thanks for your pull-request.
The full list of commands accepted by me can be found at here.
You can get sig-info at here.
For more, you also can visit HICANN.


PR Approval Progress

Congratulations! All modules have met the lgtm and approve requirements.

Module Approval Details

module lgtm status approve status
experimental 颜业峰, lilin_137 (2/2) 颜业峰 (1/1)
src/base_comm temper7620, 颜业峰, lilin_137 (3/2) temper7620, 颜业峰 (2/1)
test temper7620, 颜业峰, dingweiqin_57, lilin_137 (4/2) temper7620, 颜业峰, dingweiqin_57 (3/1)

💡 Tip:

  • Committer can comment /approve or /lgtm
  • Commenting /approve implies both code review (lgtm) and intent to merge (approve)

CLA Signature Pass

jiyuanhao, thanks for your pull request. All authors of the commits have signed the CLA. 👍

likedislike
CANN-robot
CANN-robot成员
12 天前 评论:

CLA Signature Pass

jiyuanhao, thanks for your pull request. All authors of the commits have signed the CLA. 👍

likedislike
CANN-robotCANN-robot成员
12 天前 将leabclove,yanglianxiang2,ccNeo824,chengym,ccl_ligang,linyf950,yanzhenghang,chenliang267,yanyefeng,chenhao_,dingweiqin_57,songmingyang,wenxuemin,yangxiaoyuan,laodazhao1,yangshaohua,chenke2026,zhongqin2020,bz_lee,Innoecho,chenyia,temper7620,zhangxp1030,abiggg,lilin_137,lilianlin_hw,hulk1997,ouyangxizi设为评审人
CANN-robotCANN-robot成员
12 天前 将leabclove,yanyefeng,chenhao_,wenxuemin,Innoecho,temper7620设为审查人
Yuanhao Ji
Yuanhao Ji成员
12 天前 评论:

/compile

likedislike
Yuanhao JiYuanhao Ji成员
12 天前 update merge request[project id: 7874083, iid: 4625, commit_id: 99a3791b00899e6d41ddb9df3f9d3750ffa45f3e] virtual merging success
CANN-robotCANN-robot成员
12 天前 添加了label:ci-pipeline-running
CANN-robot
CANN-robot成员
12 天前 评论:

流水线任务触发成功
任务链接 [1732b4f1a61f41689eb3cfb16d481c60][流水线指导]

任务名称状态日志下载链接
SCA ✅ SUCCESS >>>>>
antipoison ✅ SUCCESS >>>>>
Check_Pr ✅ SUCCESS >>>>>
Compile_Ascend_X86 ✅ SUCCESS >>>>> >>>>>
Compile_Ascend_ARM ✅ SUCCESS >>>>> >>>>>
pre_comment ✅ SUCCESS >>>>>
Compile_Ascend_X86_ubuntu24 ✅ SUCCESS >>>>> >>>>>
Compile_Ascend_ARM_ubuntu24 ✅ SUCCESS >>>>> >>>>>
StaticCheck_codespell ✅ SUCCESS
StaticCheck_link_validity ✅ SUCCESS
StaticCheck_resource_existence ✅ SUCCESS
StaticCheck_tag_closed ✅ SUCCESS
StaticCheck_markdownlint ✅ SUCCESS
ST_Test ✅ SUCCESS
API_Check ✅ SUCCESS >>>>>
codecheck ✅ SUCCESS >>>>>
codecheck_style ✅ SUCCESS >>>>>
UT_Test ✅ SUCCESS >>>>>
precommit ✅ SUCCESS >>>>>
PreSmoke_A900_npupool ✅ SUCCESS >>>>>
PreSmoke_A3_npupool ✅ SUCCESS >>>>>

[2026-08-14 16:15:22]    CI执行结束

likedislike
CANN-robotCANN-robot成员
12 天前 添加了label:api-check-pass
CANN-robotCANN-robot成员
12 天前 删除了label:ci-pipeline-running
CANN-robotCANN-robot成员
12 天前 添加了label:ci-pipeline-passed
CANN-robot
CANN-robot成员
12 天前 评论:
流水线 PR-pipeline_cann_hcomm#17897 [ commitID:960206e2 ] 已完成
likedislike
atomgit-bot
atomgit-bot12 天前进行代码检视1
experimental/base_comm/channel/host_cpu_roce_channel.cc
@@ -968,3 +968,3 @@
968968 // 2.轮询rq_cq
969969 auto startTime = std::chrono::steady_clock::now();
970- auto waitTime = std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::milliseconds(timeout));
970+ auto waitTime = std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::seconds(timeout));
atomgit-bot
atomgit-bot12 天前评论:

🟡 Medium Priority

changed line → affected behavior → failure mode → suggested fix:
第 970 行将 std::chrono::milliseconds(timeout) 改为 std::chrono::seconds(timeout),使非混合模式 NotifyWait 轮询超时上限从毫秒级放大到秒级。但该函数 while (true) 循环内只有 ibv_poll_cq(非阻塞),无任何 sleep/yield(对比本文件 NotifyWaitHybrid 每轮 SaluSleep(pollInterval)),且循环整体位于函数开头 std::lock_guard<std::mutex> lock(cq_mutex)(第 961 行)持锁范围。
后果:对端 notify 迟迟未到时,本端线程 100% CPU 空转并长时间持有 cq_mutex,最坏持续整个超时(默认约 30 分钟)。改动前同值按毫秒解释(约 1.8 秒),问题几乎不可触发;改为秒后缺失退避的忙轮询被真实放大。

建议:在非混合模式 NotifyWait 的轮询循环内加入退避睡眠(参考 NotifyWaitHybrid 的 SaluSleep),避免秒级超时下 100% CPU 空转并长时间持有 cq_mutex。

likedislike
atomgit-bot
atomgit-bot12 天前进行代码检视1
src/base_comm/resources/endpoint_pairs/channels/host/host_cpu_roce_channel.cc
@@ -899,3 +900,3 @@
899900 // 2.轮询rq_cq
900901 auto startTime = std::chrono::steady_clock::now();
901- auto waitTime = std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::milliseconds(timeout));
902+ auto waitTime = std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::seconds(timeout));
atomgit-bot
atomgit-bot12 天前评论:

🟡 Medium Priority

changed line → affected behavior → failure mode → suggested fix:
第 902 行将 std::chrono::milliseconds(timeout) 改为 std::chrono::seconds(timeout),使非混合模式 NotifyWait 的轮询超时上限从毫秒级放大到秒级(调用方 HcommChannelNotifyWaitOnThread 传入的 timeOut 单位本就是秒,默认 NOTIFY_WAIT_TIMEOUT_DEFAULT = 1836 秒)。但该函数的 while (true) 循环内只有 ibv_poll_cq(非阻塞),没有任何 sleep/yield(对比 NotifyWaitHybrid 中每轮有 SaluSleep(pollInterval)),且整个循环在函数开头 std::lock_guard<std::mutex> lock(cq_mutex)(第 893 行)的持锁范围内。
后果:当对端 notify 迟迟未到达(对端仍在计算,或发生死锁)时,本端线程会以 100% CPU 空转、并长时间持有 cq_mutex,最坏持续到整个超时(默认约 30 分钟),阻塞其他需要该 CQ 的线程。改动前同样的值被按毫秒解释(约 1.8 秒),此问题几乎不可触发;改为秒后该缺失 backoff 的忙轮询问题被真实放大。

建议:在非混合模式 NotifyWait 的轮询循环内加入退避睡眠(参考 NotifyWaitHybrid 的 SaluSleep),避免秒级超时下 100% CPU 空转并长时间持有 cq_mutex。

likedislike
Yuanhao JiYuanhao Ji成员
9 天前 update merge request[project id: 7874083, iid: 4625, commit_id: cd6e7b7e394946949aca520a8b2966377c34bcdc] virtual merging success
此处折叠了5条事件消息 查看更多
CANN-robotCANN-robot成员
9 天前 添加了label:cann-cla/yes
CANN-robot
CANN-robot成员
9 天前 评论:

Notification

This pull request has been changed(code update) or closed, so removes the following label(s): ci-pipeline-passed.

likedislike
CANN-robotCANN-robot成员
9 天前 将dingweiqin_57,chenke2026,bz_lee设为审查人
Yuanhao JiYuanhao Ji成员
9 天前 解决了最后一个问题
Yuanhao Ji
Yuanhao Ji成员
9 天前 评论:

/compile

likedislike
Yuanhao JiYuanhao Ji成员
9 天前 update merge request[project id: 7874083, iid: 4625, commit_id: 5b3d0d5fd0673969830859fc07c83410256ca297] virtual merging success
CANN-robotCANN-robot成员
9 天前 删除了label:api-check-pass
CANN-robotCANN-robot成员
9 天前 添加了label:ci-pipeline-running
CANN-robot
CANN-robot成员
9 天前 评论:

流水线任务触发成功
任务链接 [5df606ba42dc4ec98ca1be7cbfc2c2e1][流水线指导]

任务名称状态日志下载链接
SCA ✅ SUCCESS >>>>>
antipoison ✅ SUCCESS >>>>>
Check_Pr ✅ SUCCESS >>>>>
Compile_Ascend_X86 ✅ SUCCESS >>>>> >>>>>
Compile_Ascend_ARM ✅ SUCCESS >>>>> >>>>>
pre_comment ✅ SUCCESS >>>>>
Compile_Ascend_X86_ubuntu24 ✅ SUCCESS >>>>> >>>>>
Compile_Ascend_ARM_ubuntu24 ✅ SUCCESS >>>>> >>>>>
StaticCheck_codespell ✅ SUCCESS
StaticCheck_link_validity ✅ SUCCESS
StaticCheck_resource_existence ✅ SUCCESS
StaticCheck_tag_closed ✅ SUCCESS
StaticCheck_markdownlint ✅ SUCCESS
ST_Test ✅ SUCCESS
API_Check ✅ SUCCESS >>>>>
codecheck ✅ SUCCESS >>>>>
codecheck_style ✅ SUCCESS >>>>>
UT_Test ✅ SUCCESS >>>>>
precommit ✅ SUCCESS >>>>>
PreSmoke_A900_npupool ✅ SUCCESS >>>>>
PreSmoke_A3_npupool ✅ SUCCESS >>>>>

[2026-08-17 10:22:34]    CI执行结束

likedislike
Yuanhao JiYuanhao Ji成员
9 天前 修改了pull request 的描述
CANN-robotCANN-robot成员
9 天前 添加了label:api-check-pass
CANN-robotCANN-robot成员
9 天前 删除了label:ci-pipeline-running
CANN-robotCANN-robot成员
9 天前 添加了label:ci-pipeline-passed
CANN-robot
CANN-robot成员
9 天前 评论:
流水线 PR-pipeline_cann_hcomm#17981 [ commitID:b16d85e5 ] 已完成
likedislike
Yuanhao JiYuanhao Ji成员
9 天前 修改了pull request 的描述
lilin_137成员9 天前进行代码检视2
experimental/base_comm/channel/host_cpu_roce_channel.cc
@@ -36,7 +36,7 @@ constexpr u32 MEM_BLOCK_SIZE = 128;
3636constexpr uint16_t DEFAULT_LISTENING_PORT = 60001;
3737constexpr u32 SEND_RQE_COUNT = 16;
3838constexpr u32 HYBRID_QP_STATUS_POLL_SLEEP_US = 1000;
39-constexpr u32 DEFAULT_NOTIFY_POLL_TIMEOUT_MS = 30000;
39+constexpr u32 DEFAULT_NOTIFY_WAIT_TIMEOUT_S = 30;
4040constexpr uint32_t kHostResourceId = 0U;
4141 
4242static inline CommMemType HcclMemTypeToCommMemType(HcclMemType type)
@@ -938,7 +938,7 @@
938938 return HCCL_SUCCESS;
939939}
940940 
941941HcclResult HostCpuRoceChannel::NotifyWait(const uint32_t localNotifyIdx, const uint32_t timeout)
lilin_1379 天前评论:

请确认本次修改hccl会配合适配,该timeout会涉及对外接口

likedislike
Yuanhao Ji
Yuanhao Ji成员
9 天前 评论:

hccl 会同步修改

Llilin_137成员
9 天前 解决了最后一个问题
lilin_137成员
9 天前 评论:

/lgtm

likedislike
57_Viking
57_Viking成员
9 天前 评论:

/approve

likedislike
Yuanhao JiYuanhao Ji成员
8 天前 修改了pull request 的描述
temper7620成员
8 天前 评论:

/approve

likedislike
philo
philo成员
8 天前 评论:

/approve

likedislike
CANN-robotCANN-robot成员
8 天前 添加了label:lgtmapproved
CANN-robotCANN-robot成员
8 天前 合入了pull request