已合并
[feat] 用 HcclDlopen/HcclDlsym/HcclDlclose 弱符号封装 dlopen/dlsym/dlclose #2574
HXDW创建于 23 天前
[feat] 用 HcclDlopen/HcclDlsym/HcclDlclose 弱符号封装 dlopen/dlsym/dlclose #2574
已合并
HXDW创建于 23 天前
HXDW成员
23 天前

描述

关联的Issue

测试

文档更新

类型标签

likedislike
Pull Request已成功合入, 合并人@CANN-robot
(感谢 HXDW 的贡献)
HHXDW成员
23 天前 创建了 pull request,commit 3ae147a4
atomgit-bot
atomgit-bot
23 天前 评论:

变更摘要

此 PR 引入了一套弱符号封装机制,用 HcclDlopen/HcclDlsym/HcclDlclose 替代项目中直接对标准 dlopen/dlsym/dlclose 的调用。新增 hccl_dl.h 声明三个封装函数,新增 hccl_dl.cc 通过 weak_alias 宏将内部实现(__HcclDlopen 等)以弱符号方式导出为 HcclDlopen 等符号,内部仍调用标准 dlfcn.h 函数。随后将所有调用点统一替换为新的封装函数,并更新相关 CMake 构建配置。

主要改动

  • 新增弱符号封装层 hccl_dl.h / hccl_dl.cc:定义 HcclDlopenHcclDlsymHcclDlclose 三个函数,内部通过 weak_alias 宏将 __HcclDlopen/__HcclDlsym/__HcclDlclose 实现以弱符号方式导出,实现对标准 dlopen/dlsym/dlclose 的间接封装。
  • INIT_SUPPORT_FLAG 宏中 dlsym 替换为 HcclDlsym:在 dlsym_common.h 中,将 INIT_SUPPORT_FLAG 宏内的 dlsym 调用改为 HcclDlsym,并新增 #include "hccl_dl.h"
  • hcomm_device_dlsym.cchcomm_dlsym.ccdlopen 替换:将 dlopen("libccl_kernel.so", ...)dlopen("libhcomm.so", ...) 分别改为 HcclDlopen 调用。
  • dlhcomm_function.cc 中全部动态加载函数替换:将析构函数中的 dlclose 改为 HcclDlclose,初始化中的三处 dlsym 调用改为 HcclDlsymdlopen 改为 HcclDlopen,并新增 #include "hccl_dl.h"
  • barrier_op.cchcomm_primitives_dl.ccdlsym 替换:将 BarrierFallbackToOldFlow 中的 dlsym(RTLD_NEXT, ...)HcommPrimitivesDlInit 中的 dlsym 调用均改为 HcclDlsym
  • 构建配置更新hccl_compat.cmakehccl_kernel_compat.cmake 中将新增的 hccl_dl.cc 加入对应编译目标(hccl / hccl_compat / hccl_kernel_compat)。
  • 移除 hccl_res_dl.h 中条件编译的 HcclDedicatedThreadType 枚举定义:删除了 CANN_VERSION_NUM < CANN_VERSION(9, 2, 0) 条件下的该枚举类型声明。
likedislike
atomgit-bot
atomgit-bot
23 天前 评论:

代码审查

审查总结

优先级 数量
P0 0
P1 1
P2 0
P3 1

各文件审查结果:

文件 结果
src/common/hcomm_dlsym/dlsym_common.h 无问题
src/common/hcomm_dlsym/hccl_compat.cmake 无问题
src/common/hcomm_dlsym/hccl_dl.cc 无问题
src/common/hcomm_dlsym/hccl_dl.h P3 建议:不必要的 #include <dlfcn.h>
src/common/hcomm_dlsym/hccl_kernel_compat.cmake 无问题
src/common/hcomm_dlsym/hccl_res_dl.h P1:移除 HcclDedicatedThreadType 后引用该类型的 DECL_WEAK_FUNC 未同步保护
src/common/hcomm_dlsym/hcomm_device_dlsym.cc 无问题
src/common/hcomm_dlsym/hcomm_dlsym.cc 无问题

整体风险评估:中等。 核心变更(弱符号封装 dlopen/dlsym/dlclose)实现正确,CMake 集成完整,调用点迁移到位。但 hccl_res_dl.h 中移除 HcclDedicatedThreadType 枚举定义而未同步调整其引用处,在 CANN_VERSION_NUM < CANN_VERSION(9, 2, 0) 的构建中将导致编译失败,建议优先修复。


现在我已经彻底审查了这三个文件。让我总结一下我的发现:

审查分析

文件 1:src/common/hcomm_dlsym/hcomm_primitives_dl.cc

  • 第 141 行dlsym(libHcommHandle, "HcommBatchTransferOnThread")HcclDlsym(libHcommHandle, "HcommBatchTransferOnThread")
  • INIT_SUPPORT_FLAG 宏(该文件中第 104-139 行各处调用)内部使用的是 HcclDlsym,而第 141 行原本使用的是原生 dlsym。这一变更使手动调用与所有其他初始化调用保持一致。HcclDlsym 通过包含链 hcomm_primitives_dl.hdlsym_common.hhccl_dl.h 已可用。语义完全等价。没有问题。

文件 2:src/ops/barrier/barrier_op.cc

  • 第 18 行:新增 #include "hccl_dl.h" —— 使用 HcclDlsym 所必需。
  • 第 34 行dlsym(RTLD_NEXT, "HcclBarrier")HcclDlsym(RTLD_NEXT, "HcclBarrier")
  • 由于 _GNU_SOURCE 在第 12 行(<dlfcn.h> 通过 hccl_dl.h 被包含之前)已定义,RTLD_NEXT 的可用性保持不变。包装函数只是透传调用,行为等价。没有问题。

文件 3:src/ops/op_common/dlhcomm_function.cc

  • 第 12 行:新增 #include "hccl_dl.h",该文件原本通过传递依赖已可用;显式包含是安全的。
  • 第 32 行dlclose(h)HcclDlclose(h)
  • 第 39、40、43 行:三处 dlsym(handle_, ...)HcclDlsym(handle_, ...)
  • 第 54 行dlopen("libhcomm.so", RTLD_NOW)HcclDlopen("libhcomm.so", RTLD_NOW)
  • 所有包装函数都只是将其参数透传给底层的 dlfcn 函数;语义不变。第 55 行的 dlerror() 调用仍然有效,因为 HcclDlopen 直接委托给 dlopen。没有问题。

最终结论

这三个文件均无可报告的问题。此 diff 是一个直接的、机械性的重构,将所有 dlopen/dlsym/dlclose 原生调用替换为其以 Hccl 为前缀的弱符号包装函数,并添加了必要的 #include "hccl_dl.h" 头文件引入。功能行为完全等价,未引入逻辑错误、安全问题、可靠性风险或破坏性变更。

按文件汇总

文件 结果
src/common/hcomm_dlsym/hcomm_primitives_dl.cc 无问题
src/ops/barrier/barrier_op.cc 无问题
src/ops/op_common/dlhcomm_function.cc 无问题

按严重程度统计:P0: 0,P1: 0,P2: 0,P3: 0。整体风险评估:低风险,单纯的包装层替换。

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

⛔ 需要修改

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

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


PR Approval Progress

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

Module Approval Details

module lgtm status approve status
repo-cann/hccl bz_lee, chenyia (2/2) bz_lee (1/1)
src chenyia, bz_lee (2/2) bz_lee (1/1)

💡 Tip:

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

CLA Signature Pass

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

likedislike
此处折叠了155条消息 查看更多
bz_lee成员
21 天前 评论:

/approve

likedislike
CANN-robotCANN-robot成员
21 天前 添加了label:lgtmapproved
CANN-robot
CANN-robot成员
21 天前 评论:

The MR can not be merged, because of CodeReview discussion not resolved

If you want to solve this problem, you can click here to do it in the FAQs.

likedislike
HHXDW成员
21 天前 解决了最后一个问题
CANN-robotCANN-robot成员
21 天前 合入了pull request