Skip to content

feat(starry-kernel): support cgroup2 process migration#1045

Closed
LetsWalkInLine wants to merge 11 commits into
rcore-os:devfrom
LetsWalkInLine:feat/support-cgroup2-process-migration
Closed

feat(starry-kernel): support cgroup2 process migration#1045
LetsWalkInLine wants to merge 11 commits into
rcore-os:devfrom
LetsWalkInLine:feat/support-cgroup2-process-migration

Conversation

@LetsWalkInLine

@LetsWalkInLine LetsWalkInLine commented May 30, 2026

Copy link
Copy Markdown
Contributor

背景

本 PR 继续前置 PR #1015 做 cgroup 支持工作,工作进度持续更新在 #582 中。

当前 cgroup2 已支持 mount、基础 interface files 以及 child cgroup hierarchy,但仍缺少 process membership。用户态无法通过 cgroup.procs 把进程迁入 child cgroup,也无法通过 /proc/[pid]/cgroup 观察进程所属 cgroup。本轮实现 Slice 3 的 cgroup.procs 迁移和 procfs cgroup 视图,为后续 fork/exit membership 语义和 controller 支持打基础。

变更内容

  • os/StarryOS/kernel/src/cgroup/mod.rs

    • 新增 process membership core API,支持 process 注册、注销和迁移。
    • cgroup.procs read 改为按真实 live process membership 输出,每行一个 TGID。
    • cgroup.procs write 支持显式 live TGID,将整个 process membership 迁移到目标 cgroup。
    • 新增 cgroup.procs 输入解析:空白、非数字、溢出和 0 返回 EINVAL,不存在 TGID 返回 ESRCH
    • populated child cgroup 的 rmdir 使用真实 live count 判定,返回 EBUSY 且保留目录。
    • 新增 procfs cgroup 文本生成,格式为 0::<path>\n
    • attach_process() 和 membership release 通过 cgroup tree lock 串行化;对已被 reap/release 的 inactive pid 返回 ESRCH,避免 stale ProcessData 污染 live count。
  • os/StarryOS/kernel/src/task/mod.rs

    • ProcessData 新增当前 cgroup id 和 cgroup_membership_active,追踪 live membership 是否仍有效。
    • 初始进程默认进入 root;非 CLONE_THREAD fork/clone 子进程继承父进程 cgroup id,并注册到对应 cgroup live count。
    • remove_process() 和 ProcessData::Drop 通过同一 membership release 路径注销 live count,保证 reap 与 Drop 不会重复扣减。
  • os/StarryOS/kernel/src/pseudofs/proc.rs

    • /proc/[pid]/proc/self 目录新增只读 cgroup 文件。
    • live process 读取 /proc/[pid]/cgroup 返回当前 global cgroup hierarchy path。
  • test-suit/starryos/normal/qemu-smp1/cgroup-basic/c/src/main.c

    • 扩展 cgroup-basic,覆盖 /proc/self/cgroupcgroup.procs 迁移、错误码和 populated rmdir
    • 补齐 cgroup.controllers 写入返回 EACCES 的负例。
  • os/StarryOS/kernel/src/task/ops.rs

    • remove_process() 在 wait reap 时释放 cgroup membership,保证 cgroup.procs 可见状态和 populated rmdir 判定同步。

测试用例

cgroup-basic 本轮新增和扩展的覆盖点:

  • /proc/self/cgroup 初始返回 0::/\n
  • child cgroup 创建后,向 child cgroup.procs 写入空白、非数字、0 和不存在 TGID 均失败,且 membership 不变。
  • 写当前 TGID 到 child cgroup.procs 成功,/proc/self/cgroup 变为 0::/migrate\n
  • child cgroup.procs 包含当前 TGID,root cgroup.procs 不再包含当前 TGID。
  • populated child cgroup rmdir 返回 EBUSY,失败后目录仍存在。
  • 写当前 TGID 到 root cgroup.procs 成功迁回 root。
  • 迁回后 root cgroup.procs 包含当前 TGID,child cgroup.procs 为空,child cgroup 可删除。
  • cgroup.controllers 写入返回 EACCES
  • cgroup v1 mount 仍返回 ENODEV,确认本轮只接入 cgroup2。
  • fork 子进程继承迁移后的 cgroup,/proc/self/cgroup 返回 0::/migrate\n。
  • fork 子进程 live 时出现在 child cgroup.procs、不出现在 root cgroup.procs;wait reap 后 membership 被释放。

实现逻辑

cgroup membership 的 source of truth 放在 cgroup core 和 ProcessData 中。ProcessData 保存当前 cgroup id,cgroup tree 保存每个 node 的 live process count;cgroup2fs 只负责把 interface file read/write 映射到 core API,不保存 membership 状态。

attach_process() 先查找目标 process,随后在 cgroup tree lock 内确认 old/target cgroup 仍存在,并在同一临界区更新 old/target live count 与 ProcessData::cgroup_id。这样 populated rmdir 可以只依赖 tree 内 live count,不需要在删除路径扫描 process table。

procs_text() 只先确认 cgroup node 存在,然后释放 cgroup tree lock,再扫描 process table 并过滤 membership。这样避免 cgroup tree lock 和 process table lock 形成反向嵌套。

/proc/[pid]/cgroup 直接通过 process 当前 cgroup id 生成 global hierarchy path。本轮不实现 cgroup namespace path remap。

验证

以下命令均在 Docker 容器内执行:

cargo fmt
cargo xtask clippy --package starry-kernel
cargo xtask starry test qemu --arch x86_64 -g normal -c cgroup-basic
cargo xtask starry test qemu --arch aarch64 -g normal -c cgroup-basic
cargo xtask starry test qemu --arch riscv64 -g normal -c cgroup-basic
cargo xtask starry test qemu --arch loongarch64 -g normal -c cgroup-basic

结果:

starry-kernel clippy: 13/13 checks passed
x86_64:      DONE: 62 pass, 0 fail, result: 1/1 case(s) passed
aarch64:     DONE: 62 pass, 0 fail, result: 1/1 case(s) passed
riscv64:     DONE: 62 pass, 0 fail, result: 1/1 case(s) passed
loongarch64: DONE: 62 pass, 0 fail, result: 1/1 case(s) passed

说明

  • pid == 0 写入 cgroup.procs 暂不实现 Linux shorthand,本轮固定返回 EINVAL,避免误用当前 get_process_data(0) 行为导致静默迁移 current process。
  • loongarch64 普通 build 仍会打印既有 kprobe selftest dead-code warnings,与本次 cgroup 改动无关。
  • 当前仓库没有 scripts/test/clippy_crates.csv,因此本轮没有更新 clippy whitelist。
  • 注意到可能和当前打开的PR feat(starry-kernel): add unshare, procfs namespace files, and claw-code tests #1031 会有一点语义冲突或者代码冲突,麻烦reviewer确定一下合并顺序

未覆盖和后续

  • 已覆盖 wait reap 后 membership 释放的基本路径;尚未覆盖 zombie 未被 reap、waitid WNOWAIT 等更细分的用户态可见语义。
  • 尚未覆盖 /proc/[other-pid]/cgroup 的独立用户态正例。
  • 尚未实现 CLONE_NEWCGROUPCLONE_INTO_CGROUPclone3.cgroup
  • 尚未实现 cgroup namespace、threaded cgroup、cgroup.threads 和 pids/cpu/memory 等 controller。

Track cgroup v2 process membership in ProcessData and the cgroup core tree. Writing an explicit live TGID to cgroup.procs now migrates the process, updates source and target live counts, and makes cgroup.procs reads reflect the real membership.

Expose /proc/[pid]/cgroup and /proc/self/cgroup using the global cgroup hierarchy path. The current Slice 3 policy rejects whitespace-only input, non-numeric input, overflow, and pid 0 with EINVAL, and returns ESRCH for missing TGIDs.

Extend cgroup-basic to cover process migration into and out of a child cgroup, procfs path updates, invalid cgroup.procs writes, populated rmdir returning EBUSY, and cgroup.controllers write returning EACCES.

Validation: cargo fmt; cargo xtask clippy --package starry-kernel; cargo xtask starry test qemu --arch x86_64/aarch64/riscv64/loongarch64 -g normal -c cgroup-basic.
Copilot AI review requested due to automatic review settings May 30, 2026 08:09

Copilot AI 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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds cgroup v2 process membership tracking in the kernel and exposes it via /proc/<pid>/cgroup, along with a C test that validates cgroup.procs migration and error cases.

Changes:

  • Track per-process cgroup v2 membership (cgroup_id) and implement cgroup.procs write-to-attach behavior.
  • Add /proc/.../cgroup in procfs backed by kernel cgroup path rendering.
  • Extend the cgroup basic test to validate migration, /proc/self/cgroup, and expected errors.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
test-suit/starryos/normal/qemu-smp1/cgroup-basic/c/src/main.c Adds helper assertions and new migration/error tests for cgroup v2.
os/StarryOS/kernel/src/task/mod.rs Adds cgroup_id to process state and hooks registration/unregistration.
os/StarryOS/kernel/src/pseudofs/proc.rs Exposes a new cgroup procfs file returning cgroup membership text.
os/StarryOS/kernel/src/cgroup/mod.rs Implements process attach, cgroup.procs parsing/writes, and procfs text helpers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test-suit/starryos/normal/qemu-smp1/cgroup-basic/c/src/main.c
Comment thread test-suit/starryos/normal/qemu-smp1/cgroup-basic/c/src/main.c
Comment thread os/StarryOS/kernel/src/cgroup/mod.rs
Comment thread os/StarryOS/kernel/src/cgroup/mod.rs
Comment thread os/StarryOS/kernel/src/task/mod.rs Outdated
Make cgroup live-process accounting fail loudly on inconsistent counts instead of masking underflow, and register new processes against their initialized cgroup id.

Inherit the parent's cgroup id for non-thread clone/fork paths so children do not escape a migrated cgroup.

Harden the cgroup-basic helpers for explicit string termination and partial/EINTR writes, and add a fork-inheritance regression while the child remains live.

Validation: git diff --check. Docker-based fmt, clippy, and QEMU tests were not run in this environment and should be run by the submitter.
@LetsWalkInLine

Copy link
Copy Markdown
Contributor Author

新commit处理copilot以及本地review发现的问题:

  • 已修复 Copilot 提到的 C 测试字符串终止、write() partial/EINTR、live count underflow/overflow、初始 cgroup 注册问题。
  • 已修复非 CLONE_THREAD fork/clone 继承父进程 cgroup,并加了 child live 时的 cgroup.procs 回归覆盖。
  • Copilot 的 procs_text() 性能建议暂不处理:当前 slice 按计划从进程表动态生成,避免引入 per-cgroup PID 状态。

Release cgroup live-count membership when wait removes a reaped process from the process table instead of waiting for ProcessData to drop.

Keep Drop as a one-shot fallback so stale task references cannot double-decrement the cgroup live count.

This keeps cgroup.procs visibility and rmdir busy checks in sync after a forked child exits and is waited.

Validation: git diff --check. Docker-based fmt, clippy, and QEMU tests were not run by the committer.
Include the rustfmt-only line wrapping change for the cgroup membership activation store.

Validation: not run; formatting change only.

@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 实现 cgroup2 进程 membership 核心功能,包括:

  • ProcessData 新增 cgroup_id(AtomicU64)和 cgroup_membership_active(AtomicBool),追踪进程当前所属 cgroup。
  • cgroup core 新增 register_process/unregister_process/attach_process API,支持 cgroup.procs 写入迁移和 live count 计数。
  • /proc/[pid]/cgroup/proc/self/cgroup 动态输出当前 cgroup hierarchy path(0::<path>\n)。
  • fork/clone 继承父进程 cgroup id(通过 clone.rs 传递 old_proc_data.cgroup_id())。
  • ProcessData::Dropremove_process() 中通过 cgroup_membership_active flag 确保 live count 只注销一次。

实现逻辑评审

实现设计正确,source of truth 在 cgroup core tree 和 ProcessData 中:

  1. 锁序安全attach_process()CGROUP_TREE lock 内完成旧/新 cgroup 的 live count 更新和 cgroup_id 原子写入,避免多锁嵌套。procs_text() 先确认 node 存在再释放 tree lock,然后扫描 process table,避免 cgroup tree lock 与 process table lock 反向嵌套。
  2. 计数正确性register_process 使用 checked_addunregister_process 使用 checked_subdebug_assert!,不会静默吞掉下溢。attach_process 同时更新 old/target live count,在同一临界区内完成。
  3. 单次注销cgroup_membership_active 使用 swap(false, AcqRel) 保证 release_cgroup_membership_if_needed() 只执行一次,Dropremove_process 两个调用点不会双重递减。
  4. fork 继承clone.rs 中非 CLONE_THREAD 路径将父进程 cgroup id 传给子进程 ProcessData::new(),子进程自动注册到同一 cgroup。
  5. 输入解析parse_procs_pid 正确处理空白、非数字、溢出、pid=0,分别返回 EINVAL/ESRCH

验证结果

命令 结果
cargo fmt --check 通过
cargo xtask clippy --package starry-kernel 13/13 通过
cargo xtask starry test qemu --arch x86_64 -g normal -c cgroup-basic 70 pass, 0 fail

作者声称的 aarch64/riscv64/loongarch64 验证已在 PR body 中记录,本地确认 x86_64 QEMU 通过。

CI 状态

当前无 CI 失败报告(作者触发了 CI rerun commit)。

重复/重叠分析

  • base branch:dev 分支无已有的 cgroup process membership 实现,本 PR 是新增功能。
  • PR #1031(claw-code):PR #1031 也在 pseudofs/proc.rs 中添加了 cgroup 条目,但实现为静态 "0::/\n"。本 PR 的实现是动态的(基于 proc_data.cgroup_id() 生成实际路径)。两者在 proc.rs 的 ThreadDir entries()get() 匹配分支以及 task/mod.rs 中存在合并冲突风险。建议:先合并本 PR,PR #1031 在合并时更新 proc.rs 中的 cgroup handler 为本 PR 的实现(或直接删除其静态版本)。作者已在 PR body 中提到此冲突风险。

Review 线程状态

  • Copilot 的 expect_file_equals NUL 终止、write() partial/EINTR、saturating_subchecked_subroot_id() 硬编码共 4 个线程已在后续 commit 中修复,已 resolve。
  • procs_text() O(n log n) 性能建议线程保留为 open:作者明确说明当前 slice 按计划从进程表动态生成,避免引入 per-cgroup PID 状态。当前进程数规模下可接受。

测试覆盖评估

新增测试覆盖了:

  • /proc/self/cgroup 初始值和迁移后更新
  • cgroup.procs 写入错误码(EINVAL/ESRCH)
  • 进程迁移进/出 child cgroup
  • fork 子进程继承 cgroup(通过 pipe 同步验证)
  • populated cgroup rmdir 返回 EBUSY
  • cgroup.controllers 写入返回 EACCES

测试充分覆盖了本轮实现的 migration 语义,fork 继承测试设计良好(使用 ready/release pipe 避免竞态)。

未覆盖/后续工作

PR body 已明确列出后续 slice(CLONE_NEWCGROUPCLONE_INTO_CGROUP、cgroup namespace、controller 支持等),当前 slice 边界合理。

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.

先 request changes,阻塞点只有一个:cgroup.procs 迁移和 remove_process()/reap 的 membership 释放之间还缺同步,可能破坏 cgroup live-count。细节见 inline comment。

本地验证:

  • git diff --check $(git merge-base HEAD origin/dev)..HEAD:通过
  • cargo fmt --check:通过
  • cargo xtask clippy --package starry-kernel:13/13 checks passed
  • cargo xtask starry test qemu --arch x86_64 -g normal -c cgroup-basic:通过,DONE: 70 pass, 0 failresult: 1/1 case(s) passed

CI 状态:当前 head ab2283a82a7aa5cc68f0af3cb960cca8cf7ce2da 上 GitHub checks 未看到失败项;format、clippy、Starry x86_64/aarch64/riscv64/loongarch64 QEMU container jobs 均为 success。

重复/重叠分析:当前 origin/dev 只有 cgroup2 hierarchy/interface stubs,没有 process membership 的重复实现。open PR #1031 也添加 /proc/[pid]/cgroup,但它是静态返回 0::/\n,与本 PR 的动态 membership 视图属于 conflict-risk;建议以本 PR 的动态实现为准,#1031 后续 rebase 时删除/替换它的静态 handler。

已有 review thread 中,Copilot 的 NUL 终止、partial write/EINTR、live-count checked_sub、root_id 硬编码问题已修复;剩余 procs_text() O(n log n) 属性能建议,对当前 slice 不作为阻塞。另一个小点:PR body 的“尚未实现 fork/clone 继承”已经和当前代码/测试不一致,建议修正描述。

Comment thread os/StarryOS/kernel/src/cgroup/mod.rs
@ZR233
ZR233 requested review from AsakuraMizu and yfblock June 2, 2026 04:24
Route cgroup membership release through the cgroup tree lock so attach and reap share the same synchronization point.

Have cgroup.procs attach reject inactive memberships with ESRCH before changing live counts or cgroup_id, preventing reaped ProcessData handles from corrupting cgroup accounting.

Validation: cargo fmt; cargo xtask clippy --package starry-kernel; git diff --check.
@LetsWalkInLine

Copy link
Copy Markdown
Contributor Author

新commit修复review提出的竞态问题, 修复点:attach_process() 和 remove_process()/Drop 的 membership release 现在都通过 CGROUP_TREE lock 串行化。attach_process() 会在同一锁内检查 membership 是否仍 active,已被 reap/release 的 pid 直接返回 ESRCH,不会再修改 live count 或 cgroup_id。同时更新了PR body的内容。

@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.

先继续 request changes,之前 attach_process()remove_process()/reap 的同步问题在最新 commit 里已经修好,我也已把旧线程 resolve 了。

新的阻塞点在 fork 继承 cgroup id:clone.rs 先读父进程 cgroup_id,稍后 ProcessData::new() 再注册 child membership,这两个动作没有和 cgroup tree 的迁移/删除共用临界区。父进程在这个窗口被迁出旧 cgroup、旧 cgroup 随后被 rmdir 后,child 会用已经不存在的 id 注册并触发 expect("initial process cgroup must exist") panic。细节见 inline comment。

本地验证:

  • git diff --check origin/dev...HEAD:通过
  • cargo fmt --check:通过
  • cargo xtask clippy --package starry-kernel:13/13 checks passed
  • cargo xtask starry test qemu --arch x86_64 -g normal -c cgroup-basic:通过,DONE: 70 pass, 0 failresult: 1/1 case(s) passed

CI 状态:当前 head 54ca664a23dd9e7fb305b42877c3625beb08b909 上未看到失败项;format、clippy、Starry x86_64/aarch64/riscv64/loongarch64 QEMU container jobs 均为 success。

重复/重叠分析:origin/dev 仍没有重复的 process membership 实现。open PR #1031 也改 /proc/[pid]/cgrouptask/mod.rs,属于合并冲突/语义重叠风险;建议以本 PR 的动态 /proc/[pid]/cgroup 为准,#1031 后续 rebase 时删除或替换其静态 cgroup 输出。

Comment thread os/StarryOS/kernel/src/syscall/task/clone.rs Outdated
Register forked children through a cgroup helper that reads the parent membership and increments the child live count while holding the cgroup tree lock.

Return clone errors instead of panicking if inherited membership cannot be registered, so a stale parent cgroup id cannot survive deletion between the read and child registration.

Validation: cargo fmt; cargo xtask clippy --package starry-kernel; git diff --check.
Build /proc/[pid]/cgroup text while holding the cgroup tree lock so migration followed by cgroup deletion cannot make a live process path lookup fail on a stale id.

Filter inactive process memberships from cgroup.procs snapshots to avoid reporting processes whose cgroup live-count membership has already been released.

Validation: cargo fmt; cargo xtask clippy --package starry-kernel; git diff --check.
Take the process table snapshot before locking the cgroup tree, then filter active membership while holding the tree lock. This keeps cgroup.procs reads aligned with attach/release membership state without introducing a reverse process-table/tree lock order.

Validation: cargo fmt; git diff --check origin/dev...HEAD; cargo xtask clippy --package starry-kernel; cargo xtask starry test qemu --arch x86_64 -g normal -c cgroup-basic.
@LetsWalkInLine

LetsWalkInLine commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

感谢review,对新的同步问题采取的修复方式:

新增 cgroup::register_fork_child(parent)
在 CGROUP_TREE 锁内完成:检查父 membership active、读取父当前 cgroup id、递增 child live count
ProcessData::new() 改为 AxResult<Arc>,用 ProcessCgroupInit::Root/Inherit 传递初始化意图
clone.rs 不再提前读裸 cgroup_id,而是通过 helper 继承;失败时返回 syscall error,不 panic

同时在本地review了一遍,也发现了两个并发竞态问题,均已做出修复:

第一个问题

/proc/[pid]/cgroup 之前在 CGROUP_TREE 锁外先读 proc_data.cgroup_id(),再进锁生成 path。可能出现:读到旧 cgroup id --> 进程被迁到 root --> 旧 cgroup live_count 归零并被 rmdir 删除 --> 再用旧 id 生成 path -> NotFound

这会让 live process 的 /proc/[pid]/cgroup 并发读取异常。

修复:

现在改成在 CGROUP_TREE 锁内同时完成 active 检查、读取 cgroup id、生成 path。另外顺手让 cgroup.procs 快照过滤 inactive membership,避免并发 reap 后仍把已释放 membership 的旧 ProcessData 打出来

第二个问题

位置在 cgroup/mod.rsprocs_text()

原实现先 ensure_node_exists(id) 后释放 CGROUP_TREE 锁,再扫描 process table 并读取每个进程的 cgroup_membership_activecgroup_id。但新 状态 已经把 attach/release/fork 注册这些写侧状态转移都串到 CGROUP_TREE 锁下了,读侧如果不拿同一把锁,就可能在并发迁移或 release 时看到一个不一致快照,比如 live count 已经从旧 cgroup 扣掉了,但 cgroup.procs 仍瞬时把该进程打印在旧 cgroup 里。

修复:

先拿 process table 快照,避免在 CGROUP_TREE 锁内反向获取 process table 锁;然后在 CGROUP_TREE 锁内完成 node 存在性检查和 active + cgroup_id 过滤。这样读侧和 attach/release 的 membership 状态观察对齐,同时不引入新的锁序风险。

@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 实现 cgroup2 进程 membership 核心功能(Slice 3),包括 ProcessData 新增 cgroup_id 和 cgroup_membership_active 追踪进程所属 cgroup,新增 register_process/register_fork_child/release_process_membership/attach_process API 支持 cgroup.procs 迁移和 live count 计数,/proc/[pid]/cgroup 动态输出当前 cgroup path,fork/clone 通过 ProcessCgroupInit::Inherit 在 CGROUP_TREE 锁内完成 membership 注册避免竞态,Drop 和 remove_process 通过 deactivate_cgroup_membership swap 保证只注销一次。

实现逻辑

实现设计正确,锁序安全:attach_process 在 CGROUP_TREE lock 内完成旧/新 live count 更新和 active 检查;procs_text 先扫描 process table 再取 tree lock;register_fork_child 在锁内同时检查父进程 membership active、读取 cgroup_id、确认 node 存在并递增 live count,与 release_process_membership 和 attach_process 完全串行化。checked_add/checked_sub 加 debug_assert 保证计数正确性。

验证结果

  • cargo fmt --check: 通过
  • cargo xtask clippy --package starry-kernel: 13/13 通过
  • cargo xtask starry test qemu --arch x86_64 -g normal -c cgroup-basic: 70 pass, 0 fail

作者声称的 aarch64/riscv64/loongarch64 验证已在 PR body 中记录。

CI 状态

当前 head 上 GitHub Actions 无失败项。

重复/重叠分析

origin/dev 无已有 cgroup process membership 实现。open PR #1031 也添加 /proc/[pid]/cgroup(静态 0::/\n),与本 PR 动态实现存在合并冲突风险。建议以本 PR 为准,#1031 rebase 时删除/替换其静态 handler。

Review 线程状态

全部 7 个 review thread 已 resolved。ZR233 的 attach/release 同步和 fork 继承竞态两个阻塞点均已修复。

结论

无阻塞问题。实现设计正确、同步安全、测试充分、CI 无失败。

Powered by mimo-v2.5-pro

@LetsWalkInLine

Copy link
Copy Markdown
Contributor Author

目前存在增量功能更多更完整的PR #1107 ,因此本PR转为draft暂不追求合并

@LetsWalkInLine
LetsWalkInLine marked this pull request as draft June 4, 2026 13:00
SongShiQ pushed a commit to SongShiQ/tgoskits that referenced this pull request Jun 4, 2026
SongShiQ pushed a commit to SongShiQ/tgoskits that referenced this pull request Jun 4, 2026
…ructure

- cgroupfs.rs: use get_pids_state/get_bandwidth_state helpers to
  read/write actual controller state from the global tree
- cpu.rs: wrap BandwidthState in Arc, use cgroup_id to look up state
- mod.rs: add get_pids_state/get_bandwidth_state helper functions
- rename core.rs to cgroup_core.rs (avoid conflict with Rust core)

Remaining: register_process/unregister_process need to update pids.current

@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 4293db10670a064fbe2b21aefe872fe5b562edf7。这是 draft PR,但当前 diff 里的代码级阻塞点已经处理掉,可以从技术实现角度批准;是否转 ready/合入仍由作者和 maintainer 按 draft 状态决定。

上次 ZR233 阻塞的 fork 继承竞态已修复:现在非 CLONE_THREAD fork/clone 通过 ProcessCgroupInit::Inherit(parent) 调用 register_fork_child(),在 CGROUP_TREE 锁内同时检查父进程 membership active、读取父 cgroup id、确认 node 存在并递增 child live count;注册失败会从 ProcessData::new(...)? 返回错误,不再对已删除 cgroup id expect panic。

实现逻辑复核:

  • attach_process()register_fork_child()release_process_membership() 都以 CGROUP_TREE 作为同步点,避免 attach/reap/fork 与 cgroup 删除之间留下 stale membership。
  • remove_process() 在 wait reap 时释放 membership,ProcessData::Drop 作为 one-shot fallback,deactivate_cgroup_membership() 防止重复扣减 live count。
  • /proc/[pid]/cgroup 在 tree lock 下渲染当前 path;cgroup.procs 读取先拿 process snapshot,再在 tree lock 下过滤 active membership,避免反向锁序。
  • cgroup.procs 输入解析覆盖空白、非数字、溢出、pid 0、missing TGID;当前 Slice 3 明确暂不实现 pid 0 shorthand,这一点 PR 说明里也写清楚了。

本地验证:

  • git diff --check origin/dev...HEAD 通过
  • cargo fmt --check 通过
  • cargo xtask clippy --package starry-kernel 通过,13/13 checks passed
  • cargo xtask starry test qemu --test-group normal --arch x86_64 -c cgroup-basic 通过,guest 输出 DONE: 70 pass, 0 fail,包含 /proc/self/cgroupcgroup.procs 迁移、fork child 继承迁移后 cgroup、root/child procs 列表过滤、wait 后 membership 释放和 populated rmdir EBUSY 覆盖

CI:当前 head 的 workflow 26886210864 没有失败项;formatting、sync-lint、clippy、Starry x86_64/aarch64/riscv64/loongarch64 QEMU、std、ArceOS/Axvisor QEMU 以及相关 self-hosted board host/container sibling jobs 均为 success 或预期 skipped。

重复/重叠检查:origin/dev 只有 cgroup2 skeleton 和旧 cgroup-basic 覆盖,没有 process membership 实现。open PR 中 #1031 也触及 /proc/[pid]/cgroup 和 task/procfs 附近代码,但它是 namespace/procfs 方向,当前主要是 merge/语义顺序风险:应以本 PR 的动态 /proc/[pid]/cgroup 为准,#1031 后续 rebase 时删除或替换静态 0::/ 输出。#1042/#1034 只是搜索词命中相关 syscall/epoll 方向,不是同一 cgroup membership 实现。

Reviewer 请求:现有 yfblockAsakuraMizu review request 已覆盖 Starry/process/procfs 方向,本次不新增 reviewer。

@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.

请求修改:TOCTOU 问题和锁序文档缺失(Draft PR)。

本 PR 为 StarryOS 实现了 cgroup2 进程迁移,包括写 cgroup.procs、fork 继承、/proc/[pid]/cgroup 和进程退出时的成员资格释放。整体设计良好,C 测试套件全面。但有几个需要关注的问题。

阻塞问题:(1) write_procsensure_node_exists(id)? 在独立的锁获取中获取并释放 CGROUP_TREE 锁,然后 attach_process 稍后再次获取,两者之间目标 cgroup 可能被 rmdir。attach_process 已在锁下验证 nodes.contains_key,所以 ensure_node_exists 是冗余的,建议移除以避免不必要的锁抖动。(2) 锁顺序依赖(PROCESS_TABLE -> CGROUP_TREE)应在代码注释中文档化,防止未来代码路径以相反顺序获取导致 AB/BA 死锁。

注:作者表示更完整的后续 PR #1107 正在准备中,建议优先审查该后续 PR。

Remove the redundant target cgroup existence precheck in write_procs so attach_process performs the authoritative check under the cgroup tree lock.

Document the PROCESS_TABLE snapshot before CGROUP_TREE lock order used by cgroup.procs reads to avoid future AB/BA deadlocks with remove_process membership release.

Validation: git diff --check. Docker-based cargo fmt and clippy were not run because docker is unavailable in this environment.
@LetsWalkInLine

Copy link
Copy Markdown
Contributor Author

当前保持 draft, 不过本 PR 当前收到的 request changes 我会继续处理并保持分支在技术上的ready状态。

针对新的review建议做出了简单的修复:

  • 移除 write_procs 的冗余目标 cgroup 预检查,让 attach_processCGROUP_TREE 锁内做权威检查
  • procs_text 前补充锁序注释,避免未来引入 AB/BA 死锁。

SongShiQ added a commit to SongShiQ/tgoskits that referenced this pull request Jun 5, 2026
- Remove duplicate 'cgroup' match arm in proc.rs (already present in dev from PR rcore-os#1045)
- Remove redundant closure in task/mod.rs unwrap_or_else
SongShiQ added a commit to SongShiQ/tgoskits that referenced this pull request Jun 5, 2026
- Remove duplicate 'cgroup' match arm in proc.rs (already present in dev from PR rcore-os#1045)
- Remove redundant closure in task/mod.rs unwrap_or_else
SongShiQ added a commit to SongShiQ/tgoskits that referenced this pull request Jun 5, 2026
- Remove duplicate 'cgroup' match arm in proc.rs (already present in dev from PR rcore-os#1045)
- Remove redundant closure in task/mod.rs unwrap_or_else
@LetsWalkInLine
LetsWalkInLine deleted the feat/support-cgroup2-process-migration branch June 7, 2026 05:30
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.

3 participants