Repository navigation
Conversation
…entinel The R16 investigation lost two days to console output placed inside atomic regions: printk/console writes take milliseconds while holding the runqueue lock or with interrupts disabled, which is itself the defect that produced the runqueue self_lock spinout and the timer storms. master has none of those probes, so the durable part of that lesson is (a) a runtime sentinel that flags a long rq critical section and (b) probe code that cannot print by default. - enqueue_task_on_cpu(): keep the rq lock guard alive explicitly, time the critical section with rdtsc and report after the unlock only. Reporting requires interrupts to be enabled, so a wakeup from IRQ context can never print inside the region it is measuring. - CompletelyFairScheduler::set_next_task(): drop the runtime deadline re-arm. pick_next_task() already calls refresh_runtime_event() right after set_next_task(), so this was a duplicate arm(now+remaining) on every switch; Linux arms runtime exhaustion at accounting points (update_curr/tick), not at switch-in. - apic timer: add the TMICT sampling probe with two R16 corrections - every sampling baseline is per-CPU (a shared baseline mixed samples across CPUs and produced a bogus 9.1GHz TSC reading) and PMTMR is extended to 64 bits per CPU because the raw counter is 24 bits and wraps every ~4.7ms. Printing is behind a compile-time switch that defaults to off, since the probe runs in the deadline queue's IRQ-off region.
|
I reviewed the rebased patch and its call paths with an independent subagent. I am skipping this PR because the proposed instrumentation adds hot-path overhead and unreliable measurements, while the original debug-probe removal is already satisfied on master:
A narrowly validated duplicate-arm optimization could be submitted separately. No local rebase was pushed and this PR was not merged. |
fslongjin
left a comment
fslongjin
left a comment
There was a problem hiding this comment.
Changes requested. Please separate the underlying production problems into independent issues and focused PRs, and implement each feature or root-cause fix completely with concrete acceptance criteria. The debug probes described in the motivation are absent from master, so first identify the actual remaining defect or measured performance deficiency in the current tree.
A redundant-runtime-arm optimization needs a complete audit of scheduling, policy/nice, and cgroup-transition callers, with validation that every required event is still programmed. Diagnostic instrumentation should be a separate feature with correct measurements, bounded overhead, safe output contexts, and correct PM-timer wrap handling. The current global hot-path counters, lock timing that includes acquisition/interference, and sparse wrap sampling do not provide reliable evidence for the claimed fixes.
pick_next_task() already calls refresh_runtime_event() unconditionally right after set_next_task(). Arming again from set_next_task() re-arms the same deadline at now+remaining, delaying it on every switch. The authoritative arm points remain pick_next_task (all policy/nice and cgroup-transition paths go through it) and SchedClass::tick (after real accounting). Drop the diagnostic probes from this change; they are tracked separately. Refs: DragonOS-Community#2514, DragonOS-Community#2545
|
已按 review 意见收敛为单一调度修复：
生产问题的拆分与后续验收项见 #2545。 |
pick_next_task() already calls refresh_runtime_event() unconditionally right after set_next_task(). Arming again from set_next_task() re-arms the same deadline at now+remaining, delaying it on every switch. The authoritative arm points remain pick_next_task (all policy/nice and cgroup-transition paths go through it) and SchedClass::tick (after real accounting). Add a selftest-only invariant that every positive runtime refresh actually arms the event at the requested deadline, so a missed arm is caught instead of silently disabling throttling. Validated with the cgroup CPU bandwidth suite (20 tests). Refs: DragonOS-Community#2514, DragonOS-Community#2545
5d28f3a to
41a1384
Compare
|
已按 review 重做并补上验证：
|
|
Reviewed the latest, narrowed version (41a1384), not the earlier instrumentation patch, against current master ca991aa and Linux 6.6.139, with independent review. The duplicate runtime-event arm was a valid issue in the older implementation. It has now been superseded by #2573 (9894665): master already removes the call from I also inspected the failed LoongArch CI job (121062613318): it is the heap-layout E0080 documented here, already fixed by #2567 (32ef7a5), which aligns eligibility with actual virtual-heap backends. No additional source fix is needed for that historical failure. Skipping this PR as superseded. No remote branch changes were pushed; this assessment does not claim a new build or guest test run. |
根因
R16 排查最贵的一课：console 输出被放进了原子区。
enqueue_task_on_cpu()在持 rq 锁（self_lock()，1e6 次自旋忍耐度）的临界区里用串口级打印输出探针，单行数 ms，串口忙时更久；program_deadline_delta()/ deadline IRQ 路径在 IF=0 区间直写串口；self_lockspinout panic；定时器武装路径被拖慢 → 武装风暴与BADHALT（idle 在nr_running>0时仍进入 halt）。master 并不包含这些探针（因此"删除探针"在 master 上已自然满足）。本 PR 落地的是一份可长期保留的净增量：一个只会在安全上下文发声的回归哨兵 + 默认静默的采样探针。
修复要点
rq 锁临界区哨兵（
sched/mod.rs）enqueue_task_on_cpu()记录rdtsc，显式drop(guard)之后才统计并告警；阈值 1ms（正常入队是微秒级）。is_irq_enabled()：从 IRQ 里唤醒任务时只计数不打印——绝不把输出放回它正在测量的区域。set_next_task 去掉重复的 runtime 武装（
sched/fair.rs）pick_next_task()在set_next_task()之后已经无条件调用refresh_runtime_event()；CompletelyFairScheduler::set_next_task()里再调一次等于每次切换把同一个 deadline 事件重复arm(now+remaining)。SchedClass::tick（真实计费之后），与 Linux 在update_curr()/tick 点检查 runtime 耗尽一致——Linux 没有任何"随切换武装"的 runtime 定时器。TMICT 采样探针（默认静默）（
arch/x86_64/driver/apic/apic_timer.rs）static记录窗口/基线，被两个 CPU 交替swap，相邻采样跨 CPU 混合，差分失真（曾因此误判tsc_hz≈9.1GHz，把一个采样缺陷当成系统缺陷追了很久）。tsc_hz = Δtsc/Δpmtmr × 3.579545e6）。TMICT_PRINT = false：探针由program_deadline_delta()在 IF=0 临界区调用，默认绝不输出；即使打开也有 1s 窗口/2048 次事件节流与 64 行总量上限。Linux 对齐点
rq_lock内只用SCHED_WARN_ON/lockdep这类无输出原语，输出统一在__schedule()之后。account_cfs_rq_runtime()/check_enqueue_throttle()在计费点武装 hrtimer，切换路径（set_next_task_fair()）只做set_next_entity()，不武装定时器。实测证据
print_critical!探针位于 rq 锁内/IF=0 区时，docker load在 ~4 分钟处必挂，串口出现 cpu1self_lockspinout panic 与BADHALT洪泛。docker load全程（>13 分钟）零 panic、零sched: long rq section、零 rq spinout；Loaded image/LOAD_RC=0达成。tsc_hz交叉定频回到真实量级；此前的 9.1GHz 结论被判定为采样缺陷而非系统缺陷。docs/ocmanager-porting/status-overview.md§3.5（"探针放入 IRQ 关闭 + 持 rq 锁的临界区"）、§3.6 第 5 项。make fmt（kernel rustfmt + clippy--all-features）通过、make kernelBUILD=0。拆分说明 / 交叉引用
fix/apic-timer-delta-warn）的内容：那个 PR 修改program_deadline_delta()的开头（raw/告警），本 PR 只在同一函数的末尾追加tmict_probe(0, ...)，两者行不重叠，可任意顺序合入。fix/sched-idle-safe-halt）：那个 PR 负责 idleSTI;HLT与enqueue的Arc::ptr_eq守卫；本 PR 只动enqueue_task_on_cpu()的计时与set_next_task()的重复武装，避免同一函数被两个 PR 改同一行。debug/task_dump.rs的 stall watchdog 与time/deadline.rs的 dlprobe：它们依赖尚未合入的 critical 输出通道（printk.rs的print_critical!），本 PR 不引入该基础设施；TLB 侧的上报已由 fix(kernel): preserve user-return work and retire stopped TLB targets #2511 自包含实现。Review 后更新（最终方案）
set_next_task()重复武装 runtime event 的根因修复：pick_next_task()已在set_next_task()后无条件武装。remaining > 0的refresh_runtime_event()必须成功 arm，且queued_deadline() == now + remaining；漏武装会在测试构建中直接失败。normal/cgroup_cpu20 个测试（runtime 计费、quota、period、ancestor/sibling、cgroup transition）本地通过；sched_*、test_tlb_shootdown通过。