Skip to content

fix(lockdep): resolve Starry lock order regressions#1375

Merged
ZR233 merged 4 commits into
rcore-os:devfrom
shilei-massclouds:dev-lockdep-0625
Jun 26, 2026
Merged

fix(lockdep): resolve Starry lock order regressions#1375
ZR233 merged 4 commits into
rcore-os:devfrom
shilei-massclouds:dev-lockdep-0625

Conversation

@shilei-massclouds

Copy link
Copy Markdown
Contributor

Problem

Running StarryOS tests with lockdep enabled exposed several lock-order issues:

  • Fork/clone address-space duplication takes the source AddrSpace lock and then the newly created AddrSpace lock, but this structured nesting was not annotated.
  • Futex requeue/wait paths can take nested wait queue locks, which lockdep may classify as same-class recursive locking
    without an explicit subclass.
  • ax-fs-ng cached reads held cached-file locks while writing into the destination buffer. If that buffer points to user
    memory, the copy can fault and acquire AddrSpace, creating a cached I/O/page cache -> AddrSpace edge.
  • The ax-fs-ng block runtime repoll path could keep the pending lock held across repoll, creating unnecessary lock-order
    risk.

Changes

  • Annotate cloned AddrSpace locking with a nested lock subclass.
  • Annotate nested futex wait queue locking with a nested lock subclass.
  • Narrow CachedFile::read_at() locking so cached data is copied into a scratch page while holding cached-file locks, then
    copied to the destination after releasing them.
  • Release the ax-fs-ng block runtime pending lock before repolling.

@mai-team-app mai-team-app Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review 总结

PR 内容:修复 StarryOS 开启 lockdep 后暴露的 4 处锁序问题,涉及 AddrSpace 克隆嵌套锁、futex 等待队列嵌套锁、ax-fs-ng 缓存读取锁范围过大、以及 block runtime repoll 路径递归锁。

变更分析

  1. aspace/mod.rs — 将 try_clone()lock_nested(1) 的魔法数字替换为命名常量 CLONED_ADDR_SPACE_LOCK_SUBCLASS,并添加注释说明结构化嵌套语义。✅ 正确。

  2. futex.rs — 在 wake_opwake_requeue_if 中,对第二个等待队列锁使用 lock_nested(NESTED_WAIT_QUEUE_LOCK_SUBCLASS) 代替 lock(),通过指针比较确保一致的加锁顺序。✅ 正确,与已有 aspace 中的模式一致。

  3. block_runtime/device.rs — 重构 finish_pollpending.lock() 结果存入局部变量后再 match,确保 Repoll 分支重新获取锁前已释放前一次锁。原代码中临时 MutexGuard 的生命周期跨越了整个 match 表达式,导致 repoll 时递归获取同一把锁。✅ 正确修复。

  4. file/cache.rs — 将 io_lockread_at 函数顶部(贯穿整个读操作)移至循环内每次迭代,仅在访问 page cache 时持有。数据先拷贝到本地 scratch 缓冲区,释放锁后再写入 dst(可能触发用户态缺页并获取 AddrSpace 锁)。✅ 正确避免 cached-I/O → AddrSpace 锁序反转。

CI 状态:所有 check run 状态为 skipped(fork PR 受限),无因本 PR 导致的 CI 失败。

本地验证cargo fmt --check 通过(StarryOS kernel 和 axfs-ng)。

代码质量:变更范围小而精准,每个修复都有明确的锁序问题对应,无新 unsafe 代码、无 API 变更、无测试缺失。代码风格与现有代码库一致。

结论:无遗留问题,建议合并。

Powered by mimo-v2.5-pro

@mai-team-app mai-team-app Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review 总结

PR 内容:修复 StarryOS 开启 lockdep 后暴露的 4 处锁序问题,涉及 AddrSpace 克隆嵌套锁、futex 等待队列嵌套锁、ax-fs-ng 缓存读取锁范围过大、以及 block runtime repoll 路径递归锁。

变更分析

  1. aspace/mod.rs — 将 try_clone()lock_nested(1) 的魔法数字替换为命名常量 CLONED_ADDR_SPACE_LOCK_SUBCLASS,并添加注释说明结构化嵌套语义。✅ 正确,锁值不变,仅提升可读性。

  2. futex.rs — 在 wake_opwake_requeue_if 中,通过指针比较确定一致的加锁顺序后,对第二把等待队列锁使用 lock_nested(NESTED_WAIT_QUEUE_LOCK_SUBCLASS) 代替 lock()。✅ 正确,Ordering::Equal 分支仅取一把锁无需嵌套标注,与 aspace 中已有模式一致。

  3. block_runtime/device.rs — 重构 finish_pollpending.lock().finish_pending_poll(key) 的结果存入局部变量后再 match,确保 MutexGuard 在进入 Repoll 分支前已释放,避免 repoll 时递归获取同一把 SpinNoIrq 锁。✅ 正确修复。

  4. file/cache.rs — 将 io_lockread_at 函数顶部移至循环内每次迭代,仅在访问 page cache 时持有。数据先拷贝到本地 scratch 缓冲区,释放锁后再写入 dst(可能触发用户态缺页并获取 AddrSpace 锁)。✅ 正确避免 cached-I/O → AddrSpace 锁序反转。

CI 状态:所有 check run 结论为 skipped(fork PR 受限于安全策略,run_host/run_container 互斥跳过),无因本 PR 导致的 CI 失败。

本地验证cargo fmt --check 通过(worktree at /workspace/tgoskits-review-pr1375)。

重复/重叠分析:其他 open PR(#1376 xHCI USB、#1377 /dev/kmsg)与此锁序修复无重叠。无 patch.crates-io 引入。

代码质量:变更范围小而精准(+29/-17),每个修复都有明确的锁序问题对应。无新 unsafe 代码、无 API 变更、无测试缺失。命名常量和注释显著提升可维护性。

结论:无遗留问题,建议合并。

Powered by mimo-v2.5-pro

@ZR233 ZR233 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

本轮看过改动、当前 head CI、重复 PR 和 lockdep 覆盖后,我认为可以合入。

核对点:

  • 当前 head 仍是 efe9fd63017e342b5219a9eff2c14dc4c82d516e,PR 为 clean merge state。
  • GitHub Actions 当前 head 上格式、同步检查、clippy、std、Starry/ArceOS/Axvisor QEMU 以及板测入口都为通过/预期跳过。
  • 未看到已有 review thread;搜索 open PR 后没有发现相同的 lockdep 修复重复提交。
  • 普通 Starry QEMU CI 配置本身没有默认启用 lockdep,所以本地补跑了目标场景:FEATURES=lockdep cargo xtask starry test qemu --arch x86_64 -c qemu-smp1/system。构建参数中确认带有 ax-std/lockdep,完整 qemu-smp1/system 结果为 1/1 case(s) passed / all starry qemu tests passed,未再触发 lockdep fatal violation。

改动里对同类嵌套锁使用 lock_nested,并把 block runtime repoll 分支、cached file read 的用户拷贝路径从锁持有区间拆出来,和 PR 目标一致。

@ZR233
ZR233 merged commit d930a7a into rcore-os:dev Jun 26, 2026
54 checks passed
@github-actions github-actions Bot mentioned this pull request Jun 26, 2026
Antareske pushed a commit to Antareske/tgoskits that referenced this pull request Jun 27, 2026
* fix(starry-kernel): annotate cloned aspace lock nesting

* fix(starry-kernel): annotate nested futex wait queues

* fix(ax-fs-ng): avoid cached read lock inversion

* fix(ax-fs-ng): drop pending lock before repoll
@github-actions github-actions Bot mentioned this pull request Jun 27, 2026
Antareske pushed a commit to Antareske/tgoskits that referenced this pull request Jun 27, 2026
* fix(starry-kernel): annotate cloned aspace lock nesting

* fix(starry-kernel): annotate nested futex wait queues

* fix(ax-fs-ng): avoid cached read lock inversion

* fix(ax-fs-ng): drop pending lock before repoll
luodeb pushed a commit that referenced this pull request Jun 30, 2026
* fix(starry-kernel): annotate cloned aspace lock nesting

* fix(starry-kernel): annotate nested futex wait queues

* fix(ax-fs-ng): avoid cached read lock inversion

* fix(ax-fs-ng): drop pending lock before repoll
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants