Linux scheduler sync

sched-ext discussion

[PATCH] sched_ext: don't BUG_ON a destroyed DSQ in process_deferred_reenq_users

Bugfix分析 success2026/08/15 08:013 封邮件1 个 patch
0/3 已展开

LLM 分析

sched_ext：避免在 process_deferred_reenq_users 中对已销毁 DSQ 触发 BUG_ON

系列概况

  • 标题: [PATCH] sched_ext: don't BUG_ON a destroyed DSQ in process_deferred_reenq_users
  • 作者: Tao Cui cuitao@kylinos.cn
  • 版本: v1（讨论中由 Tejun Heo 要求出 v2，作者已确认会发 v2）
  • 规模: 单补丁，1 file, 3 insertions(+), 1 deletion(-)
  • 修改文件: kernel/sched/ext/ext.c
  • 代码统计: 净增 2 行；用精确 if (continue) 替换宽泛的 BUG_ON
  • Message-ID: 20260811075913.344033-1-cui.tao@linux.dev
  • 完整性: 包含 --- 提交说明、Fixes tag、Signed-off-by，diff 完整可解析

补丁目的

scx_bpf_dsq_reenq() 会把一个 deferred reenq（dru）排到队列里，由 run_deferred() 在后续 ops.dispatch() 上下文之外执行，而不是 ops.dispatch() 本身。如果在 dru 真正运行之前，对应的用户 DSQ 已经被 destroy_dsq() 释放掉，那么 process_deferred_reenq_users() 看到的 dsq->id 已经是 SCX_DSQ_INVALID，走到 BUG_ON(dsq->id & SCX_DSQ_FLAG_BUILTIN) 就会触发内核崩溃。destroy_dsq() 不会等待/flush 挂起的 drus，因此这里本来就不能假设 DSQ 仍然有效。补丁把对这一特定条件的 fatal check 改为温和的 continue 跳过。

旧流程的问题

旧流程在遍历挂起的 dru 时，对每个 dsq 都直接断言 dsq->id 不带 SCX_DSQ_FLAG_BUILTIN。一旦 destroy_dsq() 在 dru 待运行窗口里把 DSQ 标记成 SCX_DSQ_INVALID，这条断言就会被点燃，把整个系统拉下水。这是一种典型的「用 BUG_ON 表达不变量，但实际只是合法竞态」的误用——把偶然可达的状态当作永不该出现来处理。

新流程

把这条 fatal 断言换成对 SCX_DSQ_INVALID 的精确判断，仅在这一种情况下 continue 跳过；其它任何带 builtin flag 的状态仍由 BUG_ON 守护，这样既消除了已知竞态，又保留了对应不应出现状态的告警能力。

Patch 概览

  • kernel/sched/ext/ext.c::process_deferred_reenq_users()：循环开头加 if (unlikely(dsq->id == SCX_DSQ_INVALID)) continue;，保留后续 BUG_ON(dsq->id & SCX_DSQ_FLAG_BUILTIN);，标题与 tag 排版按规范微调。

关键实现

static void process_deferred_reenq_users(struct rq *rq)
{
    /* destroy_dsq() may have raced and invalidated @dsq, nothing to reenq */
    if (unlikely(dsq->id == SCX_DSQ_INVALID))
        continue;

    BUG_ON(dsq->id & SCX_DSQ_FLAG_BUILTIN);
    /* see schedule_dsq_reenq() */
    smp_mb();
    reenq_user(rq, dsq, reenq_flags);
}

要点：

  • 用 SCX_DSQ_INVALID（精确值）替代 SCX_DSQ_FLAG_BUILTIN（mask）。Builtin DSQ 是 scheduler 内部预留的，不应出现在 dru 列表里，因此只有「已被销毁的 user DSQ」是唯一例外。
  • unlikely() 提示这是冷路径，正常执行时 dru 列表里所有 dsq 都还活着。
  • 把 BUG_ON 留在后面，意味着如果用户 BPF 程序胡乱塞进 builtin DSQ 的引用，仍然能在第一时间暴露。

类比

想象餐厅服务员在排队叫号：

  • 你把一张「延迟叫号小票」（dru）放进托盘，等轮到你再喊这位客人回到队列。
  • 但客人已经吃完走人（DSQ 被 destroy_dsq() 销毁），托盘上那张小票指向的座位号已经作废。
  • 旧做法：服务员拿着作废小票去前台，前台直接掀桌子报警（BUG_ON）。
  • v1：服务员看一眼发现号是回收号就跳过，但前台本应能识别「其他异常号码也要掀桌子」，被一起放过了。
  • 修正后的 v2：服务员只对「明确作废号」跳过，其它任何异常号码（说明系统出大问题了）依然掀桌子报警。

Highlight：风险与注意点

  • 范围误判：v1 的 if (dsq->id & SCX_DSQ_FLAG_BUILTIN) continue 覆盖了所有 builtin flag 状态，会吞掉本来应当 BUG_ON 抓到的非 INVALID 错误。Tejun 明确指出「swallows states which can never occur legitimately」。
  • destroy_dsq 没有 flush dru：根本上是个设计权衡——destroy 选择不等待挂起的 drus。补丁只是把「崩内核」变成「安静跳过」，但这意味着 destroy 的语义要靠调用者自觉（确保 BPF 侧不会在 destroy 之后还想 reenq）。
  • Commit message 表达：标题大小写、Tags 之间的空行都应在 v2 中修正，符合 Documentation/process/submitting-patches.rst 的惯例。
  • 后续观察点：是否需要从 destroy_dsq() 反向避免留下 dru（譬如在 destroy 之前 flush、或复用 SCX_DSQ_INVALID 做哨兵）？这是个值得在 v2+/follow-up 讨论的接口设计问题。

版本变化

  • v1 → v2（讨论中预计）：将 if (unlikely(dsq->id & SCX_DSQ_FLAG_BUILTIN)) continue; 改为 if (unlikely(dsq->id == SCX_DSQ_INVALID)) continue;，紧跟原 BUG_ON；标题从 don't 改为 Don't；删除 Fixes/Signed-off-by 之外的空行。Tao 已在第三封邮件确认将按此调整出 v2。

一句话总结

scx_bpf_dsq_reenq() 排出的 dru 会与 destroy_dsq() 竞态，把那个会被无害触发的 BUG_ON 改成只对 SCX_DSQ_INVALID 的精确 continue，保留对其他异常状态的内核崩溃报警。

+---------------------------------------------------------------+
|  BPF: scx_bpf_dsq_reenq(dsq, ...)                            |
|     -> enqueue a deferred reenq (dru) on the per-rq list     |
+---------------------------+-----------------------------------+
 |
                            v
+---------------------------------------------------------------+
|  ... time passes; dsq may be destroyed before dru runs ... |
|                                                               |
|  destroy_dsq(): dsq->id := SCX_DSQ_INVALID  (no dru flush)  |
+---------------------------+-----------------------------------+
                            |
                            v
+---------------------------------------------------------------+
|  run_deferred() -> process_deferred_reenq_users(rq)           |
| |
|    v1 (too broad):                                           |
|        if (dsq->id & SCX_DSQ_FLAG_BUILTIN) continue;<-- WRONG: swallows every builtin state|        BUG_ON(...);   (removed)                               |
| |
|    v2 (review fix):                                          |
|        if (unlikely(dsq->id == SCX_DSQ_INVALID)) continue; <-- skip ONLY the destroyed one
|        BUG_ON(dsq->id & SCX_DSQ_FLAG_BUILTIN);          <-- still catch real bugs
|        smp_mb(); |
|        reenq_user(rq, dsq, reenq_flags);                     |
+---------------------------------------------------------------+