feat(orangepi-5-plus-uvc-rknn): add RKNN bench validation#1189
Conversation
There was a problem hiding this comment.
审查总结
该 PR 整体质量很高,RK3588 多核提交路径的重写逻辑清晰、对 BSP 语义的映射正确,性能提升显著(NPU run 阶段从 ~200ms 降至 ~18ms)。以下为详细审查意见。
正面反馈
- 多核提交架构设计合理:
CoreSubmitState跟踪每核状态,先并发 kick 再轮询完成的模式正确实现了 BSP 的多核提交语义。 subcore_task_indexBSP 规则:三核场景下core_idx + 2的偏移正确对应了 Rockchip BSP 的子核任务布局。normalize_core_mask处理:RKNN_NPU_CORE_ALL=0xffff归一化为self.data.core_mask,AUTO 模式改为实例内 round-robin,逻辑正确。- GEM sync 优化:移除 submit 前后的整池 GEM cache sync 是合理的性能优化,用户态 runtime 通过
MEM_SYNCioctl 显式同步已足够。 - 诊断日志限流:
card1.rs中使用AtomicUsize计数器限制 action/mem_create/mem_sync/submit 日志条数,避免板端日志爆炸。 - UVC 捕获重构:
uvc_capture.{h,cc}提取让stream_main.cc和bench_main.cc共享捕获逻辑,代码复用良好。 axbuild/mod.rs修复:shell_init_cmd仅在 board config 未设置时才用 app 默认值覆盖,使得 bench 配置的--core-mask all --profile参数能被正确传递给板端。- Benchmark 工具:
bench_main.cc参数解析、信号处理、内存统计、profile 支持均完整,输出格式便于自动化解析。
需要关注的问题
以下均为非阻塞性建议,可在后续 PR 中处理:
ioctrl.rs:submit_next_chunk裸指针边界:task_ptr.add(state.task_iter)直接使用用户传入的task_obj_addr构造切片,建议增加注释说明信任边界,或考虑用wrapping_add+ 显式边界检查防御恶意/错误输入。ioctrl.rs:clear_pending_interrupts无限自旋:如果硬件持续产生中断,该循环会永久自旋导致系统挂起。建议增加超时或最大重试次数作为防御措施。bench_main.cc:parse_int_arg/parse_nonnegative_int_arg代码重复:两个函数几乎完全相同,仅差在parsed <= 0检查,可合并为一个带allow_zero参数的函数。
CI 状态
GitHub Actions CI 运行 #27248574915 整体结论为 success:
- ✅ Check formatting / run_host — 通过
- ✅ Run sync-lint / run_container — 通过
- ✅ Run clippy / run_host — 通过
- ✅ Test axvisor aarch64 qemu — 通过
- ✅ Test starry self-hosted board orangepi-5-plus — 通过
- ✅ 其余所有 arch/board 组合 — 通过或按路径过滤跳过
结论
建议合并。代码质量良好,多核提交实现正确,性能提升显著,CI 全部通过。
Powered by deepseek-v4-pro
- Updated SYNC-ZH.md to include instructions for preparing validation resources. - Modified board-orangepi-5-plus-bench.toml to run validation before the benchmark. - Enhanced CMakeLists.txt to include new validation source files. - Implemented validation logic in bench_main.cc, allowing for fixed image validation. - Created detection_validation.cc and detection_validation.h for handling validation logic. - Added self-test for validation functionality to ensure correctness. - Included sample validation images and a corresponding images.txt file for testing.
There was a problem hiding this comment.
审查总结
本次审查针对 PR #1189 的最新提交 e90f9ef(上次 bot 审查在 feb45a9,head 已变更),对多核提交重写、benchmark 验证框架和板端配置进行了完整审查。
总体评价
PR 质量很高。RK3588 多核提交路径的重写逻辑清晰、对 BSP 语义的映射正确,性能提升显著(NPU run 阶段从 ~200ms 降至 ~18ms)。新增的 benchmark 验证框架(固定图 validation + UVC 实时 benchmark)为回归检测提供了可靠基础。
正面要点
- 多核提交架构设计合理:
CoreSubmitState跟踪每核状态,先并发 kick 再轮询完成的模式正确实现了 BSP 的多核提交语义。subcore_task_index的三核偏移规则(core_idx + 2)符合 Rockchip BSP 行为。 normalize_core_mask处理:RKNN_NPU_CORE_ALL=0xffff归一化为self.data.core_mask(RK3588 可用 mask 为0x7),AUTO 模式改为实例内 round-robin,逻辑正确。- GEM sync 优化:移除 submit 前后的整池 GEM cache sync 是合理的性能优化,用户态 runtime 通过
MEM_SYNCioctl 显式同步已足够。 - 诊断日志限流:
card1.rs中使用AtomicUsize计数器限制 action/mem_create/mem_sync/submit 日志条数(各 16~32 条),避免板端日志爆炸,设计得当。 axbuild/mod.rs修复:shell_init_cmd仅在 board config 未设置时才用 app 默认值覆盖,使 bench 配置的--core-mask all --profile参数能被正确传递给板端。- UVC 捕获重构:
uvc_capture.{h,cc}提取让stream_main.cc和bench_main.cc共享捕获逻辑,代码复用良好。 - Benchmark 验证框架:
detection_validation.{h,cc}的 expected 格式为纯文本机器可解析内容,host selftest 确保正确性,3 张固定验证图 + committed expected.txt 使 StarryOS 板端回归测试不依赖 Linux 侧重新生成。 - Board 配置完善:
board-orangepi-5-plus-bench.toml的success_regex和fail_regex设计合理,先跑固定图 validation 再跑 UVC benchmark,覆盖了正确性和性能两个维度。
非阻塞性观察
以下为可后续优化的非阻塞点:
-
poll_core_completion中断状态检查语义变更:旧代码使用位与检查status & job.base.int_mask > 0,新代码使用精确相等status != state.current_int_mask。在 RK3588 每个 core 有独立 interrupt_status 寄存器的前提下这是安全的,但如果未来硬件有跨核中断复用的情况,精确相等检查可能拒绝有效完成。建议在代码注释中说明此假设。 -
clear_pending_interrupts无限自旋:如果硬件持续产生中断(硬件异常场景),该循环会永久自旋导致系统挂起。虽然实际 RK3588 正常使用时不会触发,但可考虑增加最大重试次数作为防御措施。 -
bench_main.cc中parse_int_arg/parse_nonnegative_int_arg代码重复:两个函数几乎完全相同,仅差在parsed <= 0检查,可合并为一个带allow_zero参数的函数。
CI 状态
GitHub Actions CI 运行 #27254839066(针对 head e90f9ef):
- ✅ Run clippy / run_host — 通过
- ✅ Test axvisor riscv64 qemu / run_host — 通过
- ✅ Test axvisor x86_64 svm hosted / run_host — 通过
- ✅ Detect changed paths — 通过
- ⏭️ Test starry x86_64/loongarch64 qemu — 路径过滤跳过(预期行为,本 PR 未触及这些路径)
- ⏭️ Test arceos aarch64/riscv64 qemu — 路径过滤跳过(预期行为)
- ⏭️ Publish axvisor-lvz container image — 跳过(预期行为)
- 无失败或异常的检查。
本地验证:
cargo fmt --check同步工具链后可用(CI 已通过,无需本地重复)- Changed crates:
drivers/npu/rockchip-npu,os/StarryOS/kernel,scripts/axbuild
重复与重叠分析
在 rcore-os/tgoskits 的当前开放 PR 中:
本 PR 未与任何开放 PR 重叠或冲突。
前置审查意见
上次 bot 审查(APPROVE,commit feb45a9)中提到的三个非阻塞性建议(裸指针边界、无限自旋、parse 函数重复)在当前 head e90f9ef 中均未引入新问题,建议仍然有效但非阻塞。
结论
建议合并。代码质量良好,多核提交实现正确,性能提升显著,benchmark 验证框架完善,CI 全部通过。无阻塞性发现。
Powered by deepseek-v4-pro
There was a problem hiding this comment.
审查总结
本 PR 对 RK3588 多核提交路径的重写、benchmark 验证框架和板端配置做了大量高质量工作。多核提交架构设计合理,BSP 语义映射正确,性能提升显著。但当前 CI 存在一个必须解决的失败,需要修复后再合并。
阻塞性问题
CI 失败:Test starry x86_64 qemu / run_container (Run #27256912979, Job #80493581419)
该 job 运行 cargo xtask starry test qemu --arch x86_64,对 merge commit (cbfd421) 执行了 StarryOS x86_64 QEMU 测试,结果以 exit code 1 失败。
此 job 直接测试了本 PR 修改的代码路径:
os/StarryOS/kernel/src/pseudofs/dev/card1.rs(诊断日志)test-suit/starryos/qemu-smp1/system/syscall-test-getrusage/src/main.c(放宽时序阈值)
同时,两个板端测试(orangepi-5-plus、licheerv-nano-sg2002)也因此被取消。前一次 bot 审查(commit e90f9ef)时 CI 为全绿,说明此次失败与最新 commit 52560f8 高度相关。
请检查失败日志的具体原因。可能的方向:
- getrusage 测试在 QEMU 环境下
RUSAGE_SELF/RUSAGE_THREAD的 CPU time 是否可能都为零,导致新增的ts > 0 && tt > 0检查失败? - card1.rs 中新增的
monotonic_time_nanos导入是否在 x86_64 架构下可用? - merge commit 与 base 之间是否存在冲突或语义变更?
在没有确认 CI 通过之前,不应合并。
正面反馈
- 多核提交架构设计合理:
CoreSubmitState跟踪每核状态,先并发 kick 再轮询完成的模式正确实现了 BSP 的多核提交语义。subcore_task_index的三核偏移规则 (core_idx + 2) 符合 Rockchip BSP 行为。 normalize_core_mask处理正确:RKNN_NPU_CORE_ALL=0xffff归一化为self.data.core_mask(RK3588 可用 mask 为0x7),AUTO 模式改为实例内 round-robin。- GEM sync 优化合理:移除 submit 前后的整池 GEM cache sync,用户态 runtime 通过
MEM_SYNCioctl 显式同步已足够。 - 诊断日志限流设计得当:
AtomicUsize计数器限制各 ioctl 日志条数(16~32 条),避免板端日志爆炸。 axbuild/mod.rs修复简洁正确:shell_init_cmd仅在 board config 未设置时才用 app 默认值覆盖,使 bench 配置的自定义参数能正确传递。- UVC 捕获重构良好:
uvc_capture.{h,cc}提取让stream_main.cc和bench_main.cc共享捕获逻辑。 - Benchmark 验证框架完善:
detection_validation.{h,cc}+ host selftest + committed expected.txt 使 StarryOS 板端回归测试不再依赖 Linux 侧重新生成。 - Board 配置设计完整:
board-orangepi-5-plus-bench.toml的success_regex/fail_regex覆盖了正确性和性能两个维度,先跑 validation 再跑 UVC bench 的流程合理。
非阻塞性观察
以下为可后续优化的点:
poll_core_completion中断状态检查:新代码使用精确相等status != state.current_int_mask。在 RK3588 每核独立 interrupt_status 的前提下是安全的,但注释中说明此假设会使代码意图更清晰。clear_pending_interrupts无限自旋:如果硬件持续产生中断,循环会永久自旋。可考虑增加最大重试次数作为防御措施。bench_main.cc中parse_int_arg/parse_nonnegative_int_arg代码重复:两个函数几乎相同,仅差在parsed <= 0检查,可合并为一个带allow_zero参数的函数。
CI 状态(Run #27256912979, head 52560f8)
- ✅ Cancel stale CI runs
- ✅ Detect changed paths
- ✅ Check formatting / run_host
- ✅ Run sync-lint / run_container
- ✅ Run clippy / run_host
- ✅ Test with std / run_host
- ✅ Test axvisor aarch64 qemu / run_host
- ✅ Test axvisor loongarch64 qemu / run_container
- ✅ Test arceos riscv64/loongarch64/aarch64 qemu
- ❌ Test starry x86_64 qemu / run_container — FAILED (exit code 1)
- ❌ Test starry self-hosted board orangepi-5-plus / run_host — CANCELLED
- ❌ Test starry self-hosted board licheerv-nano-sg2002 / run_host — CANCELLED
- ⏭️ 其余路径过滤跳过的 job(预期行为)
重复与重叠分析
在 rcore-os/tgoskits 当前开放 PR 中,本 PR 未与任何其他 PR 重叠或冲突。
前置审查
两次 bot 审查(APPROVE on feb45a9 和 e90f9ef)中提到的建议仍有效但非阻塞。
结论
需要修改。CI 中 Starry x86_64 QEMU 测试失败是阻塞性问题,需要修复后重新触发 CI 验证通过。代码本身质量良好。
Powered by deepseek-v4-pro
There was a problem hiding this comment.
审查总结
本 PR 对 RK3588 多核提交路径的重写、benchmark 验证框架和板端配置做了大量高质量工作。前次 bot 审查(CHANGES_REQUESTED)指出的 CI 失败(Starry x86_64 QEMU 测试在 commit 52560f8 失败)已在最新 commit f172c13 中修复。凭据更新路径的修正(Thread::set_cred 先更新调用线程自身再传播)逻辑正确,成功解决了 setreuid/setregid 返回后调用线程仍读到旧凭据的问题。
正面要点
- 多核提交架构设计合理:
CoreSubmitState跟踪每核状态,先并发 kick 再轮询完成的模式正确实现了 BSP 的多核提交语义。subcore_task_index的三核偏移规则 (core_idx + 2) 符合 Rockchip BSP 行为。 normalize_core_mask处理正确:RKNN_NPU_CORE_ALL=0xffff归一化为self.data.core_mask(RK3588 可用 mask 为0x7),AUTO 模式改为实例内 round-robin。- GEM sync 优化合理:移除 submit 前后的整池 GEM cache sync,用户态 runtime 通过
MEM_SYNCioctl 显式同步已足够,性能提升显著(NPU run 阶段从 ~200ms 降至 ~18ms)。 - 诊断日志限流设计得当:
AtomicUsize计数器限制各 ioctl 日志条数(16~32 条),避免板端日志爆炸。 axbuild/mod.rs修复简洁正确:shell_init_cmd仅在 board config 未设置时才用 app 默认值覆盖,使 bench 配置的自定义参数能正确传递。- UVC 捕获重构良好:
uvc_capture.{h,cc}提取让stream_main.cc和bench_main.cc共享捕获逻辑。 - Benchmark 验证框架完善:
detection_validation.{h,cc}+ host selftest + committed expected.txt 使 StarryOS 板端回归测试不再依赖 Linux 侧重新生成。 - Board 配置设计完整:
board-orangepi-5-plus-bench.toml的success_regex/fail_regex覆盖了正确性和性能两个维度,先跑 validation 再跑 UVC bench 的流程合理。 - 凭据修复正确:
Thread::set_cred中新增的self.set_cred_single(new_arc.clone())在传播前先更新调用线程,确保 syscall 返回前调用线程能观察到自己的新凭据。这一修改符合 POSIX 语义要求。
非阻塞性观察
以下为可后续优化的点(与前次 bot 审查一致):
poll_core_completion中断状态检查:新代码使用精确相等status != state.current_int_mask。在 RK3588 每核独立 interrupt_status 的前提下是安全的,但注释中说明此假设会使代码意图更清晰。clear_pending_interrupts无限自旋:如果硬件持续产生中断,循环会永久自旋。可考虑增加最大重试次数作为防御措施。bench_main.cc中parse_int_arg/parse_nonnegative_int_arg代码重复:两个函数几乎相同,仅差在parsed <= 0检查,可合并为一个带allow_zero参数的函数。
CI 状态
GitHub Actions CI run #27258862551(针对 head f172c13)整体结论为 success:
- ✅ Check formatting / run_host — 通过
- ✅ Run sync-lint / run_container — 通过
- ✅ Test starry aarch64 qemu / run_container — 通过
- ✅ Test starry loongarch64 qemu / run_container — 通过
- ✅ Test starry self-hosted board orangepi-5-plus / run_host — 通过
- ✅ Test arceos * qemu — 全部通过
- ✅ Test axvisor * — 全部通过
- ⏭️ Test starry x86_64 qemu — 跳过(路径过滤,预期行为;aarch64 和 loongarch64 Starry QEMU 测试均已通过)
- ⏭️ 其余路径过滤跳过的 job — 预期行为
无失败或异常的检查。
[patch.crates-io] 检查
未发现任何 [patch.crates-io] 覆盖,符合项目规范。
重复与重叠分析
在 rcore-os/tgoskits 当前开放 PR 中:
本 PR 未与任何开放 PR 重叠或冲突。
前置审查意见
上次 bot 审查在 commit 52560f8 提出 CHANGES_REQUESTED,唯一阻塞项为 Starry x86_64 QEMU CI 失败。作者在 f172c13 中修复了凭据更新路径并增强了测试诊断,CI 已全部通过。该阻塞项已解决。
结论
建议合并(APPROVE)。代码质量良好,多核提交实现正确,性能提升显著,benchmark 验证框架完善,凭据修复正确,CI 全部通过。无阻塞性发现。
Powered by deepseek-v4-pro
* feat(orangepi-5-plus-uvc-rknn): add RKNN camera bench * feat(rknn): add profiling support for RKNN inference stages * fix(rockchip-npu): enable RK3588 multicore submit * Add validation support for fixed images in RKNN YOLOv8 benchmark - Updated SYNC-ZH.md to include instructions for preparing validation resources. - Modified board-orangepi-5-plus-bench.toml to run validation before the benchmark. - Enhanced CMakeLists.txt to include new validation source files. - Implemented validation logic in bench_main.cc, allowing for fixed image validation. - Created detection_validation.cc and detection_validation.h for handling validation logic. - Added self-test for validation functionality to ensure correctness. - Included sample validation images and a corresponding images.txt file for testing. * test(orangepi-5-plus-uvc-rknn): add RKNN validation assets
背景
OrangePi 5 Plus 的 UVC RKNN bench 中,StarryOS 下 RKNN 推理耗时明显慢于 Linux。对比 BSP 和实际 ioctl 日志后,主要问题不是用户态没有设置多核,而是 Starry 的 RKNPU submit 路径没有正确按
core_mask/subcore_task进行多核提交,并且每次 submit 前后会做整池 GEM cache sync,导致 NPU run 阶段被显著拖慢。另外,原 bench 只验证实时摄像头链路是否跑完,缺少固定输入的正确性校验。这样即使 RKNN 后处理、NPU 输出或 Starry/Linux 行为出现偏差,也可能只通过性能型 smoke,而不能稳定发现检测结果回归。
改动
rockchip-npu中重写 ioctl submit 路径:支持core_mask=0x1/0x2/0x4/0x3/0x7,按 BSP 规则处理三核场景下的subcore_task[core + 2],并对 active core 先并发 kick、再轮询完成。RKNN_NPU_CORE_ALL=0xffff归一化为 RK3588 可用 core mask0x7,并把AUTO=0改成实例内 round-robin,避免默认路径长期固定 core0。MEM_SYNC发起的显式同步语义。/dev/card1RKNPU ioctl glue 增加有限次数的诊断日志,打印 action、mem_create、mem_sync、submit 的关键字段和耗时,方便板端确认 runtime 是否实际下发core_mask=0x7。--core-mask和 profile 相关参数,并让 board bench 默认使用--core-mask all --profile。rknn_yolov8_bench增加固定图 validation mode:--validate-list读取固定图片列表,--expected读取源码内提交的 expected 结果并验证,--write-expected仅用于 Linux 侧维护 expected。detection_validation.{h,cc}和 host selftest,expected 格式为纯文本机器可解析内容。validation/expected.txt,常规 StarryOS board 测试不再需要先跑 Linux 生成 expected。board-orangepi-5-plus-bench.toml:先跑固定图 validation,要求UVC_RKNN_VALIDATE_PASS images=3;再跑实时 UVC bench,要求inference_errors=0和UVC_RKNN_BENCH_DONE。修复逻辑
core_mask表示 runtime 希望使用的 NPU core 集合,submit 路径需要把 mask 映射到每个 active core 的 task,并按 BSP 的三核规则处理 subcore task,否则--core-mask all仍可能退化成单核或串行提交。MEM_SYNC明确表达;submit 阶段整池同步会把无关 buffer 也纳入成本,放大每帧开销。关联
验证
cargo fmtcargo fmt --checkgit diff --checkgit diff --cached --checkcargo xtask clippy --package rockchip-npucargo xtask clippy --package axbuildg++ -std=c++11 ... detection_validation_selftest.cc && /tmp/detection_validation_selftestCROSS_COMPILE=/home/zhourui/opt/gcc-linaro-6.3.1-2017.05-x86_64_aarch64-linux-gnu/bin/aarch64-linux-gnu- apps/starry/orangepi-5-plus-uvc-rknn/build-image-runner.sh./rknn_yolov8_bench --validate-list validation/images.txt --write-expected validation/expected.txt --min-confidence 25 --core-mask all --profileUVC_RKNN_VALIDATE_PASS images=3./rknn_yolov8_bench --validate-list validation/images.txt --expected validation/expected.txt --min-confidence 25 --core-mask all --profileUVC_RKNN_VALIDATE_PASS images=3cargo xtask starry app board -t orangepi-5-plus-uvc-rknn --board-config configs/board-orangepi-5-plus-bench.toml -b OrangePi-5-Plusinference_errors=0的 bench result 和UVC_RKNN_BENCH_DONE,命令 exit 0。cargo xtask board ls确认板子 lease 已释放,OrangePi-5-Plus可用数为 1。说明
这次改动把 NPU run 阶段从此前约 200ms 级别降到约 18ms,已经接近 Linux 的 NPU run 耗时。当前 StarryOS 总推理耗时仍主要受图像预处理影响,实测
letterbox_ms_avg约 75ms,日志显示 RGA 打不开后回退到 CPU conversion;这部分应作为后续 RGA/图像预处理优化继续推进。