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

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

Patch Review

**Overall: Good.** This is the core infrastructure patch and is well-designed.

**`disable_trace_on_warning()` move in `lib/bug.c`:**

The patch moves `disable_trace_on_warning()` to after the suppression check and after the flag extraction block:

```c
+#ifdef CONFIG_KUNIT
+	if (warning && kunit_is_suppressed_warning(true))
+		return BUG_TRAP_TYPE_WARN;
+#endif
+
+	disable_trace_on_warning();
+
 	if (warning && once) {
```

This is correct: suppressed warnings should not disable tracing, and importantly the comment says the check is placed "before the once logic so suppressed warnings do not consume the single-fire budget of WARN_ON_ONCE()." Good design choice.

However, the `disable_trace_on_warning()` move also affects non-KUnit code paths. Previously it ran unconditionally early; now it runs after the flag extraction. This is a minor semantic change for BUG (non-warning) traps — `disable_trace_on_warning()` now runs slightly later but still before any output. Should be harmless but worth noting.

**`#ifdef CONFIG_KUNIT` in `lib/bug.c`:**

```c
+#ifdef CONFIG_KUNIT
+	if (warning && kunit_is_suppressed_warning(true))
+		return BUG_TRAP_TYPE_WARN;
+#endif
```

Using `#ifdef CONFIG_KUNIT` rather than `IS_ENABLED(CONFIG_KUNIT)` is slightly inconsistent with the inline in `test-bug.h` which uses `IS_ENABLED`. However, since `kunit_is_suppressed_warning()` isn't available without the header, and `test-bug.h` provides a stub when `!CONFIG_KUNIT`, this is fine — the `#ifdef` avoids the header include cost for the `!CONFIG_KUNIT` case. But the `#include <kunit/test-bug.h>` is already added unconditionally at the top of `lib/bug.c`, so the `#ifdef` guard is redundant — you could just use the inline stub. This is a style nit, not a bug.

**Race in `kunit_start_suppress_warning`:**

```c
+	if (kunit_has_active_suppress_warning()) {
+		KUNIT_FAIL(test, "Another suppression block is already active");
+		return NULL;
+	}
```

The check-then-act between `kunit_has_active_suppress_warning()` and `list_add_rcu()` is not atomic, but this is fine in practice because KUnit tests run in their own kthread and nesting is explicitly disallowed — two concurrent calls for the same task can't happen.

**`kunit_has_active_suppress_warning()` is exported but not declared in `test.h`:**

The function is declared in `test.h` (line 597 in the patch). Confirmed, this is fine.

**Lifetime management is correct:** `get_task_struct()` pins the task to prevent ABA reuse, and `kunit_add_action_or_reset` ensures cleanup. The `kunit_end_suppress_warning` -> `kunit_release_action` path removes from the list and `put_task_struct`. Good.

**`__kunit_suppress_auto_cleanup` accessing `(*wp)->test`:**

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

This works because `w->test` is set during `kunit_start_suppress_warning` and remains valid — the test struct outlives the suppression handle. Clean.

**No `EXPORT_SYMBOL_GPL` for `__kunit_is_suppressed_warning_impl`:** This function is not exported, which is correct since it's only called through the hook table indirection. The hook pointer is set in `kunit_install_hooks()` which runs at module init.

---
Generated by Claude Code Patch Reviewer