Claude review: Re: [PATCH 00/15] tracepoint: Avoid double static_branch evaluation at guarded call sites

[Claude Code Review Bot <claude-review@example.com>]

Overall Series Review

Subject: Re: [PATCH 00/15] tracepoint: Avoid double static_branch evaluation at guarded call sites
Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Patches: 18
Reviewed: 2026-03-13T14:04:52.638511

---

This is a well-motivated, clean optimization series. The core idea — avoiding a redundant `static_branch_unlikely()` evaluation at tracepoint call sites that are already guarded by `trace_foo_enabled()` — is sound and was suggested by both Steven Rostedt and Peter Zijlstra. The implementation is straightforward: a new `trace_invoke_##name()` API that calls `__do_trace_##name()` directly, bypassing the static branch, while preserving the LOCKDEP RCU-watching assertion and `might_fault()` for syscall tracepoints.

The filtered patches touching DRM/accel/dma-buf subsystems (patches 05, 08, 10) are all mechanical, correct conversions. The series is low-risk — it only changes behavior on hot paths when tracing is already enabled, and the functional semantics are preserved.

**One design concern** with patch 01 (the API patch): the new `trace_invoke_##name()` does **not** include the LOCKDEP `rcu_is_watching()` assertion that the normal `trace_##name()` path has. Looking at the implementation:

```c
static inline void trace_invoke_##name(proto)
{
    __do_trace_##name(args);
}
```

The normal `trace_##name()` includes:
```c
if (IS_ENABLED(CONFIG_LOCKDEP) && (cond)) {
    WARN_ONCE(!rcu_is_watching(),
              "RCU not watching for tracepoint");
}
```

The commit message for patch 01 says "retains the LOCKDEP RCU-watching assertion", but looking at the actual diff, **the assertion is NOT present** in `trace_invoke_##name()`. This is misleading. In practice it may be acceptable (the caller has already passed through `trace_foo_enabled()` which implies the tracepoint infrastructure is active, and `__do_trace_##name()` uses `guard(srcu_fast_notrace)` internally), but the commit message should accurately describe the behavior. Steven Rostedt should confirm whether omitting this assertion is intentional.

---

---
Generated by Claude Code Patch Reviewer