fix(axtask): improve might_sleep diagnostics and coverage#1235
Conversation
There was a problem hiding this comment.
评审结论:批准 (APPROVE)
变更概述
此 PR 改进 might_sleep 的诊断能力和覆盖范围,包含三项改动:
-
为 sleep/yield/wait/join/exit 调用链添加
#[track_caller]:覆盖axtask核心 API →arceos_api包装 →arceos_posix_apisyscall 层 →axstd用户态线程 API。原本 wrapper 层未透传 caller location,导致might_sleeppanic 时显示的位置指向转发层而非真正的违规调用点,现已修复。 -
为 Starry
access_user_memory()添加might_sleep()检查:用户内存访问可能因缺页而阻塞,之前仅有 IRQ 开启的 assertion,缺少 preempt 计数的检查。现在might_sleep()会统一检查 IRQ 状态和 preempt 计数。 -
扩展
might_sleeppanic 诊断信息:panic 消息现在包含cpu_id和当前task_id,便于在多核/多任务环境下定位问题。
实现评估
#[track_caller]放置正确且完整:从 trait 声明(lib.rs)到 impl(imp/task.rs等),再到最内层的axtaskAPI,调用链完整覆盖。current_task_id()实现简洁安全:current_may_uninit().map(|curr| curr.id().as_u64()),返回Option<u64>在 panic 消息中以{:?}格式化,在任务未初始化时显示None,行为合理。access_user_memory()中的might_sleep()位置正确:在 IRQ 断言之后、实际内存访问之前插入。- 无 unsafe 代码新增。
- 无
[patch.crates-io]依赖。 - 无公共 API 签名的破坏性变更。
本地验证
| 检查项 | 结果 |
|---|---|
cargo fmt --check |
✅ 通过 |
cargo clippy --manifest-path os/arceos/modules/axtask/Cargo.toml --all-features -- -D warnings |
✅ 通过 |
cargo clippy --manifest-path os/StarryOS/kernel/Cargo.toml --all-features -- -D warnings |
✅ 通过 |
cargo test --manifest-path os/arceos/modules/axtask/Cargo.toml --all-features |
__TimeIf_*),属于工作区通用问题,与 PR 无关 |
CI 状态
CI 运行 #27386153512 仍在进行中。已完成的关键 job:
- Check formatting: ✅ success
- Run sync-lint: ✅ success
- Test axvisor (loongarch64 qemu, riscv64 qemu, x86_64 svm hosted, self-hosted x86_64): ✅ success
- Test with std: ✅ success
- Test arceos aarch64 qemu / run_host: ✅ success
- Test axvisor riscv64 qemu / run_host: ✅ success
- 部分 starry QEMU job (riscv64, aarch64, x86_64) 和 board job (orangepi-5-plus) 仍在运行中。
无 CI 失败。mergeable_state: "unstable" 是由于仍有 in-progress job,符合预期。
重复与重叠分析
repo:rcore-os/tgoskits type:pr state:open might_sleep— 仅返回本 PR #1235,无重复。repo:rcore-os/tgoskits type:pr state:open track_caller— 无匹配。repo:rcore-os/tgoskits type:pr state:open access_user_memory— 仅返回本 PR #1235。- base 分支
access_user_memory中尚无might_sleep()调用,本 PR 是首次添加。
无其他开放 PR 与本 PR 重叠或冲突。
已有 review 和评论
本 PR 无已有 review 或评论。
遗留风险
无阻塞性问题。此 PR 是纯粹的诊断/可调试性改进,不改变任何运行时调度或睡眠语义。#[track_caller] 仅影响 panic 时的位置报告;新增的 might_sleep() 调用在 access_user_memory() 中位置合适且语义正确。
Powered by deepseek-v4-pro
ZR233
left a comment
There was a problem hiding this comment.
已按当前 head e7fc8c4f13a37a48c09434d169f5b322010031e6 复审,整体可以合并。
检查点:
#[track_caller]已覆盖axstd、POSIX API、ax-api、axtask到实际might_sleep()调用点的主要 sleep/yield/wait/join/exit 链路,诊断位置能够向外层调用者传播。access_user_memory()的新增might_sleep()放在 IRQ-enabled 断言之后、切换 accessing-user-memory 标志之前;该路径可能触发缺页/调度,纳入 atomic-context 检查是合理的。might_sleep()的 panic 诊断新增cpu_id和task_id,current_may_uninit()返回None时也能安全格式化,未引入额外 unsafe。- 开放 PR 中未发现重复的
might_sleep/track_caller/access_user_memory改动,也未发现[patch.crates-io]。
本地验证:
cargo fmt --check通过git diff --check origin/dev...HEAD通过cargo xtask clippy --package ax-task通过,18/18 checkscargo xtask clippy --package ax-api通过,13/13 checkscargo xtask clippy --package ax-posix-api通过,15/15 checkscargo xtask clippy --package ax-std通过,41/41 checkscargo xtask clippy --package starry-kernel通过,14/14 checks
当前 CI 大部分相关项已经 success;查询时还有 Test starry x86_64 qemu / run_container 处于 in-progress,建议等 required checks 完成后再合并。
There was a problem hiding this comment.
评审结论:批准 (APPROVE)
变更概述
此 PR 包含 5 个独立提交,围绕 might_sleep 诊断和 select/timer 行为做了三项改进:
-
#[track_caller]贯穿 sleep/yield/wait/join/exit 调用链(提交 1):覆盖axtask→arceos_api→arceos_posix_api→axstd全链路,确保might_sleeppanic 时显示真实违规调用点而非包装层。 -
为
access_user_memory()添加might_sleep()检查(提交 2):用户内存访问可能因缺页而阻塞,原先仅有 IRQ-enabled 的 assert,新增 preempt 计数检查。 -
扩展
might_sleeppanic 诊断(提交 3):新增cpu_id和task_id,current_task_id()返回Option<u64>,在任务未初始化时安全显示None。 -
select fd set 保持(提交 4):重构
do_select()使用临时FdSet位图在 poll 期间收集结果,仅在 poll 完成后写入用户空间 fd set。超时时正确清零输出。这修复了 poll 回调多次调用时可能部分写入的问题。 -
POSIX timer 轮询提前(提交 5):在
new_user_task的返回用户空间路径中,当没有待处理信号时主动调用poll_process_timer(),关闭了过期定时器只在 syscall 返回后才被发现的窗口。
代码评估
#[track_caller]放置完整且正确,无公共 API 签名破坏性变更。current_task_id()使用current_may_uninit().map(...)实现,安全无 unsafe。access_user_memory()中might_sleep()位置正确(IRQ assert 之后、实际访问之前)。- select 重构语义正确:
write_fd_set()辅助函数清晰,超时路径正确写入空 fd set。 poll_process_timer的插入位置合理,仅在无待处理信号时触发,不干扰信号处理流程。- 无
[patch.crates-io]依赖,无新增 unsafe 代码。
PR 描述说明
PR 标题和正文主要描述了 might_sleep 相关的三项改进(#[track_caller]、access_user_memory、panic 诊断),但提交 4(select fd set)和提交 5(poll_process_timer)是独立的 bugfix 行为变更,未在 PR 正文中提及。虽然各自的 commit message 清楚说明了意图,建议作者更新 PR body 以涵盖所有变更。
CI 状态
CI 运行 #6116 (7e89f17) 仍在进行中(in_progress)。已完成的 check runs 均为 skipped(属于 run_host/run_container 互斥矩阵中的预期跳过),未观察到失败。无 classic commit statuses。
已有 review
两条历史 APPROVE review(mai-team-app[bot] 和 ZR233)均针对旧 head e7fc8c4f,不包含提交 4 和 5。当前 review 基于最新 head 7e89f172。
重复与重叠分析
- 开放 PR #1243(sysfs cgroup)和 #1241(mount API ENOSYS)均不涉及
might_sleep、track_caller、select或poll_process_timer,无重叠。 - base 分支无等价的 select/timer 改动。
本地验证
| 检查项 | 结果 |
|---|---|
cargo fmt --check |
✅ 通过 |
cargo xtask clippy --package ax-task |
✅ 18/18 通过 |
cargo xtask clippy --package starry-kernel |
✅ 14/14 通过 |
cargo xtask clippy --package ax-api |
✅ 13/13 通过 |
遗留风险
无阻塞性问题。所有变更均为诊断改进或正确性修复,不影响调度核心语义。建议作者补充 PR body 描述。
Powered by mimo-v2.5-pro
* fix(axtask): preserve might_sleep caller locations * fix(starry-kernel): check user memory access sleepability * fix(axtask): include task context in might_sleep panic * fix(starry-kernel): preserve select fd sets while waiting * fix(starry-kernel): poll expired POSIX timers before user return
Problem
might_sleepalready catches sleep attempts with IRQs disabled or a nonzero preempt count, but a few low-risk gaps made failures harder to diagnose:might_sleepcheck.Changes
#[track_caller]through the axtask sleep/yield/wait/join/exit paths and their ax-api, ax-std, and ax-posix-apiwrappers.
might_sleep()to Starryaccess_user_memory().might_sleeppanic diagnostics withcpu_idand currenttask_id.