Claude review: bug/kunit: Core support for suppressing warning backtraces

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

Patch Review

**Design and correctness look good.** The hook integration is clean and follows the existing pattern exactly.

**1. Suppression check placement in `__report_bug()` is well-placed:**

```c
+	/*
+	 * Before the once logic so suppressed warnings do not consume
+	 * the single-fire budget of WARN_ON_ONCE().
+	 */
+	if (warning && kunit_is_suppressed_warning(true))
+		return BUG_TRAP_TYPE_WARN;
+
+	disable_trace_on_warning();
```

Good decision to return before both the `once` flag consumption and `disable_trace_on_warning()`. A suppressed warning shouldn't eat the `WARN_ON_ONCE()` budget or disable tracing.

**2. `kunit_start_suppress_warning` uses plain `kzalloc` (not `kunit_kzalloc`):**

```c
+	w = kzalloc(sizeof(*w), GFP_KERNEL);
```

This is correct since the lifetime is manually managed via `call_rcu` -> `kfree`, but it's worth noting the cover letter says "dynamically allocated via kunit_kzalloc()" which is inaccurate — only the overall cover letter, not the patch 1 commit message which correctly says `kzalloc()`.

**3. The `kunit_is_suppressed_warning` inline checks the function pointer before calling:**

```c
+	return kunit_hooks.is_suppressed_warning &&
+	       kunit_hooks.is_suppressed_warning(count);
```

This NULL check is correct — the hook may not be installed if the kunit module hasn't loaded yet but `kunit_running` was somehow set. Defensive and appropriate.

**4. `kunit_end_suppress_warning` silently returns on NULL:**

```c
+void kunit_end_suppress_warning(struct kunit *test,
+				struct kunit_suppressed_warning *w)
+{
+	if (!w)
+		return;
+	kunit_release_action(test, kunit_suppress_warning_cleanup, w);
+}
```

This makes the direct API forgiving when `kunit_start_suppress_warning` returned NULL on failure. That's the right choice since `kunit_start_suppress_warning` already calls `KUNIT_FAIL`.

**5. The `__kunit_suppress_auto_cleanup` accesses `(*wp)->test`:**

```c
+void __kunit_suppress_auto_cleanup(struct kunit_suppressed_warning **wp)
+{
+	if (*wp)
+		kunit_end_suppress_warning((*wp)->test, *wp);
+}
```

This is fine — the `__cleanup` attribute fires on scope exit, before the action teardown, so the handle is still valid at this point.

**6. Minor: `__kunit_is_suppressed_warning_impl` is not exported:**

```c
+bool __kunit_is_suppressed_warning_impl(bool count)
+{
```

No `EXPORT_SYMBOL_GPL`. This is correct since it's only called indirectly via the hook function pointer, and `kunit_has_active_suppress_warning()` (which calls it) IS exported. But `kunit_has_active_suppress_warning` is defined in `bug.c` and calls `__kunit_is_suppressed_warning_impl` directly, so they're in the same compilation unit — no export needed.

---
Generated by Claude Code Patch Reviewer