selftests/sched_ext: 修复 prog_run 早期返回导致的 bpf_link 泄漏

系列基线信息

标题: [PATCH] selftests/sched_ext: Fix bpf_link leak on early return in prog_run
作者: Liang Luo luoliang@kylinos.cn
版本: 单 patch，非系列（total: null）
规模: 1 file changed, 26 insertions(+), 8 deletions(-)
首封 Message-ID: 20260709100340.706670-1-luoliang@kylinos.cn
来源频道: sched-ext（LORE 列表）
后续回复: 2 封（Andrea Righi、Tejun Heo）
最终落点: Tejun Heo 已 apply 到 sched_ext/for-7.3 分支，并补上 Fixes: a5db7817af78 ("sched_ext: Add selftests") 标签，预定合入 7.3

明确目的

这个 patch 的目标非常聚焦：修一个 sched_ext selftest 的资源泄漏 bug。具体地说，在 prog_run 这个测试用例的 run() 函数里，测试一开始就把一个 BPF struct_ops scheduler 通过 bpf_map__attach_struct_ops() 挂到内核上，拿到一个 bpf_link。原代码只在「所有断言都通过」的成功路径上才调用 bpf_link__destroy(link) 来卸载 scheduler。

但是，在 attach 和 destroy 之间有三个 SCX_EQ 断言。SCX_EQ 是一个宏，失败时会直接 return SCX_TEST_FAIL，根本走不到 destroy。这意味着：

一旦某个断言失败，BPF scheduler 不会被卸下，仍然占着内核的调度器位置；
这个测试是 selftest 套件中的一个用例，跑完一个用例接着跑下一个。下一个 selftest 想 attach 自己的 scheduler 时会失败，因为系统当前并不是 SCX_ENABLED/SCX_DISABLED 的初始状态；
结果：单点失败会污染整轮测试，错误地把"没 bug 的代码"也报告成 fail。

遍历代码

改动前（旧逻辑）

run(ctx):
link = attach_struct_ops(...) // 1. 挂载 BPF scheduler
err = bpf_prog_test_run_opts(...) // 2. 跑一次 BPF 程序
spin-wait uei.kind 写入
close(prog_fd)

SCX_EQ(err, 0); // 失败 → return SCX_TEST_FAIL
// → 不会执行下面的 destroy
SCX_EQ(uei.kind == SCX_EXIT_UNREG_BPF);
SCX_EQ(uei.exit_code == 0xdeadbeef);

bpf_link__destroy(link); // 仅成功路径才到这里
return SCX_TEST_PASS;


### 改动后（新逻辑）

- 把三个 `SCX_EQ` 全部换成「显式 if 比较 + 设置 `status = SCX_TEST_FAIL` + `goto out`」的形式；
- 新增一个 `out:` 标签，里面对 `link` 做 NULL 检查后无条件 `bpf_link__destroy(link)`；
- 函数的最终 `return status;` 统一在 out 之后；
- `setup()` 中把 `link` 初始化为 `NULL`，避免"未 attach 就被 destroy"的潜在空指针。

run(ctx):
link = NULL
status = SCX_TEST_PASS

link = attach_struct_ops(...)
if (!link) { SCX_ERR(...); return FAIL; } // 早期 bail out，没泄漏

err = bpf_prog_test_run_opts(...)
spin-wait uei.kind

if (err) { SCX_ERR(...); status=FAIL; goto out; }

if (uei.kind != SCX_EXIT_UNREG_BPF) {
SCX_ERR(...); status=FAIL; goto out;
}
if (uei.exit_code != 0xdeadbeef) {
SCX_ERR(...); status=FAIL; goto out;
}

out:
if (link) bpf_link__destroy(link); // ★ 不管哪条路径都走这里
return status;


这种"单出口 + goto cleanup"是 C 里常见的健壮写法，作者在 commit message 里也明确指出要参考 `cyclic_kick_wait.c` 这个同目录的兄弟测试，保持套件内风格一致。

### reply 1 — Andrea Righi

Andrea Righi 回了什么内容从原文里只能看到 patch 头部的引用块（`>` 引用），原始回复 body 在抓取里被截掉了。但从 Tejun Heo 的下一封回复「with the Fixes tag Andrea suggested」可以反推：Andrea 建议补一个 `Fixes:` 标签，指向最初引入这个 selftest 的提交 `a5db7817af78 ("sched_ext: Add selftests")`。这是标准做法：让 `git blame` 和 stable 回溯能精确定位到引入 bug 的 commit。

### reply 2 — Tejun Heo

Tejun 简短确认：「Applied to sched_ext/for-7.3, with the Fixes tag Andrea suggested: `Fixes: a5db7817af78 ("sched_ext: Add selftests")` Thanks. -- tejun」。明确表示 patch 已合入 7.3 准备发版窗口。

## ASCII 流程图

### 修复前后的控制流对比

          BEFORE (有泄漏)                         AFTER (无泄漏)
        ┌──────────────────┐                  ┌──────────────────┐
        │   run() entry    │                  │   run() entry    │
        │  link=attach()   │                  │ link=NULL;st=PASS│
        └────────┬─────────┘                  └────────┬─────────┘
                 │                                     │
        ┌────────▼─────────┐                  ┌────────▼─────────┐
        │ bpf_prog_test_run│                  │ bpf_prog_test_run│
        └────────┬─────────┘                  └────────┬─────────┘
                 │                                     │
    ┌────────────┼────────────┐               ┌────────┼────────┐
    │            │            │               │  if err  │  ok   │

┌────▼────┐ ┌────▼────┐ ┌────▼────┐ │ status │ │
│SCX_EQ 1 │ │SCX_EQ 2 │ │SCX_EQ 3 │ │ =FAIL │ │
│ fail? │ │ fail? │ │ fail? │ │ goto out │ │
└────┬────┘ └────┬────┘ └────┬────┘ └────┬─────┘ ┌───▼───┐
│ yes │ yes │ yes │ │ more │
┌────▼────────────▼────────────▼────┐ │ │checks │
│ return SCX_TEST_FAIL │ ┌────▼─────────▼────┐ │
│ bpf_link 永远不会被 destroy ★ │ │ out: │◄┘
│ → 后续测试 attach 失败 │ │ if(link) destroy │
└──────────────────────────────────┘ │ return status │
└───────────────────┘


### 全局调度器状态影响

┌────────────────┐
│ SCX_DISABLED │ (系统默认调度器)
└────────┬───────┘
│ prog_run 测试开始
▼
┌────────────────┐
│ SCX_ENABLED │ ← attach struct_ops 成功
│ (BPF sched) │
└────────┬───────┘
│ 旧代码：断言失败 → 直接 return
▼
┌────────────────┐
│ SCX_ENABLED │ ← 没 destroy，调度器卡死在这 ★
│ (BPF sched) │ 下一测试 attach 冲突 → fail
└────────────────┘
│ 新代码：goto out → 总是 destroy
▼
┌────────────────┐
│ SCX_DISABLED │ ✓ 干净回到初始态
└────────────────┘


## 概念类比

把这个 bug 想象成 **酒店客房的迷你吧（mini-bar）流程**：

- 客人 check-in 的时候，前台给你一张"迷你吧使用卡"（`bpf_link`），相当于把 BPF 调度器挂上房间。
- 退房时前台**只在所有消费金额都对得上的情况下**才回收这张卡。
- 但住客只要"金额对不上"前台就直接报警退房（`return SCX_TEST_FAIL`），**完全忘了收卡**。
- 下一次清洁工（下一个 selftest）想进这间房做"标准打扫"（attach 自己的调度器）时，发现门卡还插着、电也被人占着，根本进不去。

修复就是：不管结账金额对不对，前台都先走到 "out" 出口，把卡**一定**收回来、把电**一定**恢复，然后再决定是给客人办"顺利退房"还是"异常退房"。

再换一个角度：这也是为什么 C 语言里大家推崇 `goto cleanup` 模式——它像函数末尾的"finally"块，保证清理动作**与中间任何 return 路径正交**。

## Highlight 突出问题

1. **测试污染（test pollution）风险**：原作者没有跑过"第一个测试 fail 后第二个测试是否还能继续"这条路径，否则这个 bug 当初就抓到了。社区后续可以加一条 "在 ASSERT 失败路径下断言全局 SCX 状态为 DISABLED" 的 meta-check，避免类似回归。
2. **`SCX_EQ` 宏的语义陷阱**：这个宏的失败行为是"直接 return"，对写 `setup`/`cleanup` 配对或资源句柄的代码其实并不友好。维护者可以评估是否要新增一个 `SCX_EQ_GOTO(label)` 或让 `SCX_EQ` 默认带 `goto out` 行为，但**不在本 patch 范围内**。
3. **不变量同步**：`bpf_link` 一定要先初始化为 `NULL`，否则在 `setup()` 阶段 attach 失败后，`cleanup()` 里 destroy 一个未初始化的栈变量会读垃圾值。patch 顺手把这块也改了。
4. **Fixes 标签**：`a5db7817af78 ("sched_ext: Add selftests")` 是 sched_ext selftest 套件首次合入的提交（v6.9 时代），意味着这个 bug 跟着整套 selftest 一起存在了多个 release。stable 维护者可能会希望这个 patch 沿 stable queue 回溯到所有受影响的 stable 内核。
5. **API 一致性提示**：作者在 commit message 里直接点名 `cyclic_kick_wait.c` 作为参考，说明 sched_ext selftests 套件里其他兄弟用例已经采用 `goto out` 写法，prog_run 是漏网之鱼——这是一个"风格统一"型 cleanup，code review 容易快速通过。

## 版本演进

本讨论是单 patch，没有 v1→v2 演进。但有两个**隐式演进**：
- **隐含 v0 → v1（这个 patch 本身）**：从"散乱 SCX_EQ 失败即 return"重构为"统一 goto out cleanup"模式。
- **应用阶段补丁**：Tejun Heo 在 apply 时把 `Fixes:` 标签补上（来自 Andrea 建议），把 commit chain 接到 `a5db7817af78`，让 stable 回溯有据可查。这不是技术变更，是流程性变更。

## 与其他相关 patch 系列的关联

- **`sched_ext: Add selftests` (`a5db7817af78`)**：被 Fixes 标签引用的根提交，sched_ext selftest 套件最初的合入点。prog_run.c 的资源管理 bug 自此引入。
- **`cyclic_kick_wait.c` 风格**：本 patch 显式对齐这个兄弟测试文件的 cleanup 模式。如果未来要把整个 sched_ext selftests 套件的所有 `run()` 都统一成"goto out"模式，这个 patch 可以作为模板。
- **`sched_ext/for-7.3` 分支**：Tejun 的 sched_ext 维护分支，7.3 merge window 即将合入的目标。说明 sched_ext 子系统对资源管理 bug 修复的优先级别是高的（fix 直接入 rc 而非 -next）。

## 一句话总结

Liang Luo 把 sched_ext selftest 的 `prog_run` 用例里"断言失败就提前 return"的写法改成统一的 `goto out` 清理模式，修了 `bpf_link` 泄漏导致的"单点失败污染整轮 selftest"问题，Tejun 已在 7.3 窗口收下。

[PATCH] selftests/sched_ext: Fix bpf_link leak on early return in prog_run

LLM 分析

selftests/sched_ext: 修复 prog_run 早期返回导致的 bpf_link 泄漏

系列基线信息

明确目的

遍历代码

改动前（旧逻辑）