Linux scheduler sync

sched discussion

[PATCH 1/2] sched/fair: Fix stale comment on task_is_ineligible_on_dst_cpu()

其它分析 success2026/08/28 16:362 封邮件2 个 patch
0/2 已展开

LLM 分析

sched/fair: Fix stale/incomplete comments in load balancing

系列概况

  • 标题: 系列围绕"修复 load balance 路径上的过时/不完整注释"，由两枚独立 patch 组成（邮件正文无单独 cover letter）
    • PATCH 1/2: sched/fair: Fix stale comment on task_is_ineligible_on_dst_cpu()
    • PATCH 2/2: sched/fair: Complete the bail-out list in can_migrate_task()
  • 作者: Zhan Xusheng zhanxusheng@xiaomi.com
  • 版本: v1（Subject 中未出现 vN 标记）
  • 规模: 2 个 patch
  • 修改文件: kernel/sched/fair.c（仅此一个文件）
  • 代码统计:
    • PATCH 1 共 11 行 diff（+6/-5），全部是注释
    • PATCH 2 共 10 行 diff（+6/-4），全部是注释
  • Message-ID:
    • 20260828083628.1406519-1-zhanxusheng@xiaomi.com
    • 20260828083628.1406519-2-zhanxusheng@xiaomi.com
  • 完整性: 完整，系列共 2 patch，输入包含全部 2 封

补丁目的

这是两枚纯注释维护型 patch：不改动任何运行时行为，只把 kernel/sched/fair.c 中两处早已与代码脱钩的注释校正为与代码一致：

  1. task_is_ineligible_on_dst_cpu() 上方注释：仍以 PLACE_LAG 时代措辞写 dst_cfs_rq->nr_queued > 1，但代码早已切到 root cfs_rq 上的 h_nr_queued，判定对象也从 task_cfs_rq(p) 换成 task_rq(p)->cfs。
  2. can_migrate_task() 头部 bail-out 概要：旧注释只列 6 条，实际代码里有 8 条筛选项；缺了 kthread_is_per_cpu() 和 eligibility 两条，且顺序与代码 if 链对不上。

旧流程的问题

  • 873199d27bb2 (sched/core: Prioritize migrating eligible tasks in sched_balance_rq()) 引入 task_is_ineligible_on_dst_cpu() 时注释就与实际判定条件略有出入。
  • 85570f10a4c6 (sched/eevdf: Move to a single runqueue) 把 load balance 改到 root cfs_rq 上跑，注释还停留在 task group cfs_rq 的世界观。
  • 9bcb959d05ee (sched/fair: Ignore percpu threads for imbalance pulls) 给 can_migrate_task() 加上 kthread_is_per_cpu()，873199d27bb2 又加 eligibility 判断，两次都没回头更新函数顶部概要。
  • 后果：新人按注释反推代码会得出错误结论，code reviewer 也容易被注释带偏。

新流程

完全保留代码可执行路径，只重写注释：

  • PATCH 1 把判定描述改成"在源 runqueue 上判 @p 是否 ineligible；只有目标 runqueue 非空时才有意义"，并把"为什么在源 runqueue 上判"——place_entity() 会保持 lag——写进注释。
  • PATCH 2 把 bail-out 列表补齐到 8 条，并按代码 if 链的真实顺序重新排序。

Patch 概览

  • PATCH 1/2 sched/fair: Fix stale comment on task_is_ineligible_on_dst_cpu()
    重写 task_is_ineligible_on_dst_cpu() 上方注释。
  • PATCH 2/2 sched/fair: Complete the bail-out list in can_migrate_task()
    补全并重排 can_migrate_task() 顶部注释。

关键实现

PATCH 1/2 关键 diff（纯注释，无逻辑变更）：

/*
 * Check whether @p would be ineligible if migrated to @dest_cpu.
 *
 * Eligibility is evaluated on the source runqueue, because place_entity()
 * preserves the lag over a migration: a task that is ineligible on the
 * source stays ineligible on the destination. It only matters when the
 * destination has something queued, as a task joining an empty runqueue is
 * eligible either way.
 */
static inline int task_is_ineligible_on_dst_cpu(struct task_struct *p,
                                                int dest_cpu)

PATCH 2/2 关键 diff（纯注释，无逻辑变更）：

/*
 * We do not migrate tasks that are:
 *  1) delayed dequeued unless we migrate load, or
 *  2) target cfs_rq is in throttled hierarchy, or
 *  3) ineligible, while this domain has not failed to balance yet, or
 *  4) per-CPU kthreads, or
 *  5) blocked on mutexes (if SCHED_PROXY_EXEC is enabled), or
 *  6) cannot be migrated to this CPU due to cpus_ptr, or
 *  7) running (obviously), or
 *  8) are cache-hot on their current CPU
 */
int can_migrate_task(struct task_struct *p, struct lb_env *env)

类比

  • 像给一栋老旧办公楼重新贴"楼层功能指引"——墙、楼梯、房间没动（代码逻辑 0 改动），但每层门口的牌子三年前装修后没人更新过，结果把机房写成仓库。新贴的指引要按现在的真实用途写。
  • can_migrate_task() 的注释像医院挂号大厅的"今日拒诊科室"清单：实际医生有 8 个拒诊理由（旧清单只列了 6 个），名单顺序和叫号顺序也对不上。这个 patch 就是把清单补齐、并按真正的初诊顺序排好。
  • 分类上：这是 cleanup 类文档维护 patch，不属于 RFC、也不是修复运行时缺陷的 bugfix，归为 other（clean-up/refactor）。
+----------------------------------------------------------+
| Old comment: nr_queued > 1 (PLACE_LAG wording)           |
| can_migrate_task bail-out list: 6 cases                  |
+--------------------------+-------------------------------+
                           |
                           |  873199d27bb2: add eligibility check
                           |  85570f10a4c6: switch to root cfs_rq / h_nr_queued
                           |  9bcb959d05ee: add per-CPU kthread check
                           v
+----------------------------------------------------------+
| Current code: h_nr_queued != 0, evaluate on src rq       |
| can_migrate_task bail-out: actually 8 cases              |
| Current comment: stale (does not match code)             |
+--------------------------+-------------------------------+
                           |
                           |  PATCH 1/2: rewrite eligibility comment
                           |  PATCH 2/2: complete bail-out list
                           v
+----------------------------------------------------------+
| New comment: matches code if chain, ready as baseline    |
+----------------------------------------------------------+

Highlight：风险与注意点

  • 注释 patch 容易被维护者当成 trivial 改动搁置，但它们位于 load balance 关键路径，对新接触 EEVDF / sched_balance_rq 的人是第一手材料，建议合入而非堆栈化。
  • PATCH 2 新加的 "ineligible, while this domain has not failed to balance yet" 这一条措辞要再 review：它与后续 if (...) return 0; 中使用的 lb_env->flags/LBF_* 是否字面对应，审阅者需要顺一遍具体 flag。
  • PATCH 1 解释了"为什么在源 runqueue 上判"，但没写出具体判定的字段（p->se.avg_vruntime 还是其他）。如果未来还要补一版注释，建议把"实际读的是哪个 cfs_rq 字段"也明确出来，避免下一轮同样的脱钩。
  • 风险点（无运行时风险）：base-commit 1b78070aaef63512688aebfbc82365ef9d6660f1 需要确认仍是当前 sched/fair 树上的有效提交，否则 patch 可能因上下文偏差被退回。

版本变化

仅有 v1，无后续版本。

一句话总结

Zhan Xusheng 提交两枚纯注释补丁，把 task_is_ineligible_on_dst_cpu() 与 can_migrate_task() 上方早已与 EEVDF/PLACE_LAG 时代脱钩的注释修正为与代码 if 链一一对应。