Skip to content

fix(irqsnoop): print per-interval summary tables instead of raw event flood - #162

Open
yuKing123-king wants to merge 1 commit into
DKapture:mainfrom
yuKing123-king:fix/irqsnoop-interval-times-semantics
Open

fix(irqsnoop): print per-interval summary tables instead of raw event flood#162
yuKing123-king wants to merge 1 commit into
DKapture:mainfrom
yuKing123-king:fix/irqsnoop-interval-times-semantics

Conversation

@yuKing123-king

@yuKing123-king yuKing123-king commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@yuKing123-king yuKing123-king changed the title fix(irqsnoop): run interval windows by wall-clock time instead of pol… fix(irqsnoop): run interval windows by wall-clock time instead of poll counts Oct 8, 2026
@yuKing123-king
yuKing123-king force-pushed the fix/irqsnoop-interval-times-semantics branch from 7eed47b to 49d4af0 Compare October 8, 2026 09:05
@yuKing123-king yuKing123-king changed the title fix(irqsnoop): run interval windows by wall-clock time instead of poll counts fix(irqsnoop): print per-interval summary tables instead of raw event flood Oct 8, 2026
@yuKing123-king

yuKing123-king commented Oct 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

问题

  1. interval/count 参数不生效：ringbuffer_worker 原实现从不使用 env.interval，
    --env.times != 0 的语义也偏移（实际输出 times-1 个周期），doc 承诺的
    irqsnoop 1 10（1 秒汇总 × 10 次）无法工作。
  2. 事件洪流：无过滤地逐条打印 irq/softirq 事件，高负载下输出不可用。
  3. 选项失效：argp 提供 -N（纳秒）/-C（次数列）/-T（时间戳）选项，代码从未使用：
    -N 被硬编码 usec 覆盖，-T/-C 完全无实现。
  4. 参数校验不对称：interval 有 <=0 校验而 times 没有——irqsnoop 5 -1
    因 env.times-- 恒真陷入无限窗口，irqsnoop 5 0 静默零输出退出。
  5. 窗口计时用墙钟：time(NULL) 受 NTP 校时/手动改时影响；秒级粒度 + 固定 500ms
    poll 超时使实际窗口最长超出 interval 约 0.5s。

修复

  • 窗口语义：interval 真正生效（每窗口末打印汇总），env.times-- 修正次数；
    times 补 <=0 校验，与 interval 对称。
  • live/summary 分离：无参数时保留原始逐条输出（旧行为）；带 interval/count
    时静默累计，按 (type, vec) 汇总次数与总时长（u64 组合键），窗口末打印汇总表。
  • 选项生效：-N 切换 ns/usec 单位（表头同步），-C 增加 count 列，
    -T 输出前缀时间戳（live 行内前缀 / 汇总块首单独一行）。
  • 单调时钟窗口：改用 CLOCK_MONOTONIC（免校时跳变），poll 超时动态取
    min(500, 剩余ms)，窗口时长精确、无 overshoot。

@yuKing123-king
yuKing123-king force-pushed the fix/irqsnoop-interval-times-semantics branch 2 times, most recently from 66ea7ff to 38e430c Compare October 10, 2026 02:32
xu-lang
xu-lang reviewed Oct 19, 2026
View reviewed changes
Comment thread observe/irqsnoop.cpp Outdated
stat.total_time += e->delta;
// 只处理IRQ类型事件
if (e->type == IRQ)
if (env.live)

@xu-lang xu-lang Oct 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (!env.live)
return 0;

简化处理

@xu-lang xu-lang Oct 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

另一个问题，为什么加这个？同步有问题？，这个也保证不了不同

@yuKing123-king yuKing123-king Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

加这个是为了让./build/observe/irqsnoop无参数时正常逐条打印
[SOFTIRQ] pid=3134715 tid=3134729 comm=CPU 1/TCG vec=rcu delta=627ns ret=0
[SOFTIRQ] pid=3134715 tid=3134729 comm=CPU 1/TCG vec=rcu delta=1844ns ret=0然后ctrl C 汇总
让sudo ./build/observe/irqsnoop 3 10 ，有参数时，每三秒汇总，总共打印10次。就不逐条打印信息了，直接打印汇总的信息。
Tracing IRQ/SoftIRQ events... Hit Ctrl-C to end.
type vec time(usec)
softirq hi 15
softirq timer 51
softirq net_rx 167
softirq block 22
softirq tasklet 0
softirq sched 2323
softirq rcu 554
irq irq127 9
irq irq132 26
irq irq133 20
irq irq134 5
irq irq135 246
type vec time(usec)
softirq hi 41
softirq timer 76
softirq net_rx 623
softirq sched 3286
softirq rcu 734
irq irq124 0
irq irq125 29
irq irq128 11
irq irq129 1
irq irq132 2332357390194
irq irq134 16
irq irq135 469
^Ctype vec time(usec)
softirq timer 54
softirq net_rx 291
softirq tasklet 2
softirq sched 1441
softirq rcu 315
irq irq124 70
irq irq125 8
irq irq126 12
irq irq127 3
irq irq131 15
irq irq132 31
irq irq135 204

Comment thread observe/irqsnoop.cpp Outdated
{
return 0;
}
auto &stat = summary_map[{(int)e->type, (int)e->vec_nr}];

@xu-lang xu-lang Oct 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

写法优化下，不要用pair，两个int，可以直接组装long作为key

Comment thread observe/irqsnoop.cpp
return;
}
}
#ifndef BUILTIN

@xu-lang xu-lang Oct 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

怎么还有builtin的代码

@yuKing123-king yuKing123-king Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个工具之前就有builtin的部分了，这里ringbuffer_worker是公共区，但是print_summary();又是非BUILTIN的所以得隔开一下

@yuKing123-king
yuKing123-king force-pushed the fix/irqsnoop-interval-times-semantics branch from 38e430c to d137acb Compare October 19, 2026 07:15
xu-lang
xu-lang reviewed Oct 22, 2026
View reviewed changes
Comment thread observe/irqsnoop.cpp Outdated
e->name,
e->delta,
e->ret
"[IRQ] pid=%d tid=%d comm=%s irq=%d name=%s delta=%lluns "

@xu-lang xu-lang Oct 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

怎么还有这种问题呢，无关改动，不要引入

xu-lang
xu-lang reviewed Oct 22, 2026
View reviewed changes
Comment thread observe/irqsnoop.cpp Outdated
}
// 只处理IRQ类型事件
unsigned long long key =
((unsigned long long)(unsigned int)e->type << 32) |

@xu-lang xu-lang Oct 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(unsigned long long)(unsigned int)e->type ？？？？两次强转何意味

@xu-lang

xu-lang commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

AI review
一、最关键：描述与实际改动不符，根因没修
PR 正文详细描述了 bpf/observe/irqsnoop.bpf.c 的 start percpu 槽缺陷（共享槽、exit 不清槽、跨 CPU 读不到 entry）以及修法（max_entries 1→2、按 type 分槽、exit 加 if(!delta) return）。但实际 commit 没有动任何 BPF 文件：
git diff --stat origin/main...pr-162
observe/irqsnoop.cpp | 115 ++++++++++++++++++++++++------
也就是说 delta ≈ 开机时长 的天文数字根本没被消除，只是不再逐条刷屏，却照样进汇总。作者自己在评论里贴的输出 irq irq132 2332357390194 就是原 bug。结论："fix ... delta" 的标题/提交信息不成立，改动实际只是"把逐条输出改成区间汇总"。
二、汇总本身的正确性问题

  • 单位不处理选项：print_summary() 固定 total_time / 1000 硬编码为 usec（observe/irqsnoop.cpp:196），完全忽略 -N（env.nanoseconds）；-d/-T/-C 也全部失效。这些 argp 选项对外宣传却不起作用。
  • 无异常值过滤：没有对 delta 做任何合理性裁剪，一个 stale 事件即可污染整行区间结果。
  • time(NULL) 秒级窗口（:253-254）：interval<1s 无法精确，且受系统时钟跳变影响；配合 ring_buffer__poll(rb,500)，实际窗口会超出 interval 至多约 0.5s。
  • live 与标题自相矛盾：无参数时仍逐条打印（env.live=true，:222），只是 Ctrl-C 时额外打一份汇总。"instead of raw event flood" 与代码不符。
    三、确凿的逻辑 bug
  • else if (pos_args == 1 || env.times <= 0)（:101）：env.times<=0 只可能在 pos_args>=2 时成立，等于让 irqsnoop 1 0 20 这类多余位置参数被静默当作 times 接受，破坏了原来"多余参数报错"的行为。
  • 只给 env.interval 加了 <=0 校验（:96），times 没有，不对称。

@yuKing123-king
yuKing123-king force-pushed the fix/irqsnoop-interval-times-semantics branch 3 times, most recently from e8690f8 to b1186ec Compare October 23, 2026 05:25
Wang Yu
fix(irqsnoop): print per-interval summary tables instead of raw event…
6c689ca 
… flood

Signed-off-by: Wang Yu <wangyu6@uniontech.com>
@yuKing123-king
yuKing123-king force-pushed the fix/irqsnoop-interval-times-semantics branch from b1186ec to 6c689ca Compare October 23, 2026 05:43

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xu-lang xu-lang xu-lang left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@yuKing123-king @xu-lang