Test sched family#969
Conversation
There was a problem hiding this comment.
Review 总结
本 PR 已关闭(未合并),且已被同一作者的新 PR #970 取代。#970 以单个干净 commit 重新提交了几乎相同的改动(550 additions / 9 changed files vs 本 PR 的 563 / 9),建议转至 #970 继续审阅。
以下为本 PR 的代码质量观察,供 #970 参考:
内核改动 (schedule.rs)
-
_reset_on_fork计算后未使用:sys_sched_setscheduler中解析了SCHED_RESET_ON_FORK标志并赋值给_reset_on_fork,但没有任何后续逻辑使用该值。Linux 语义要求在 fork 时将子进程策略重置为SCHED_NORMAL、优先级重置为 0。当前实现只是静默接受了该标志,未真正实现重置行为。如果暂不实现,建议至少在注释中说明。 -
双重
get_task_by_sched_pid查找:check_sched_permission内部调用了get_task_by_sched_pid(pid),而调用方(sys_sched_setaffinity、sys_sched_setscheduler)在权限检查之后再次调用相同函数。这是不必要的重复查找,建议check_sched_permission返回查找到的 task 引用,调用方直接复用。 -
sys_sched_getparam使用原始指针转换:当前通过core::slice::from_raw_parts+vm_write_slice来写入用户空间,风格与文件中其他vm_write模式不一致。可以考虑用更简洁的方式(如直接构造字节切片或使用现有的 VM write helper)。 -
sched_policy初始值为 0:task.rs中sched_policy: AtomicI32::new(0)依赖于SCHED_NORMAL == 0的巧合。虽然 Linux 确实如此定义,但建议显式使用SCHED_NORMAL as i32以提高可读性。
测试 (test-sched-family)
-
硬编码 syscall 号:
test_framework.h按架构硬编码SYS_SCHED_*号码,而非使用<sys/syscall.h>提供的SYS_*宏。虽然 StarryOS 的用户态可能没有完整的 libc 头文件,但这种方式在架构支持变化时容易遗漏。当前 x86_64 的号码正确(143/144/145 对应 getparam/setcheduler/getscheduler)。 -
测试覆盖范围良好:覆盖了 setaffinity/getaffinity 正常路径、EFAULT/EINVAL/EPERM/ESRCH 错误路径、setscheduler 策略和优先级校验、getparam 边界条件、getpriority 三种 which 值,以及 RESET_ON_FORK 标志接受性。建议 #970 补充 fork 后 RESET_ON_FORK 实际重置行为的验证。
-
fail_regex包含(?m)FAIL:这会在任何输出包含 "FAIL" 字符串时触发失败,包括测试框架的 PASS/FAIL 行中的失败报告。这与success_regex的0 fail一起形成了双重保障,设计合理。
重复/重叠分析
- PR #970:同一作者的取代版本,已重新整理为单个 commit,正在审阅中。本 PR (#969) 已被取代(superseded)。
- base 分支:
dev上的sys_sched_getscheduler硬编码返回SCHED_RR,sys_sched_setscheduler和sys_sched_getparam都是空壳(直接返回 0)。本 PR 正确填补了这些空白。 - PR #823(sync_file_range):不相关,无重叠。
结论
本 PR 已关闭并取代,无需进一步操作。建议在 #970 中处理上述代码质量问题。
Powered by glm-5.1
No description provided.