| 0/0 |
2026/08/04 02:19 |
flow |
assessment-kcsan |
29m
Results: map[Benign:false Explanation:### Race Summary
`ring_buffer_unlock_commit` vs `ring_buffer_unlock_commit` on
`buffer->irq_work.waiters_pending` (and similarly on
`cpu_buffer->irq_work.waiters_pending` and `full_waiters_pending`).
### Synchronization Context
The ring buffer commit path (`ring_buffer_unlock_commit`) is designed to be
highly concurrent and lockless. When multiple CPUs concurrently write events to
the ring buffer, they concurrently execute `rb_wakeups()` to check if there are
any pending readers/waiters to wake up. Because these checks are performed
without mutual exclusion, concurrent reads and writes to `waiters_pending`
occur.
---
### Final Classification
**HARMFUL** (Marking Required for Correctness)
---
### Failure Reasoning
The `waiters_pending` flag is a fast-path flag used to notify writers that a
reader has gone to sleep and needs to be woken up via `irq_work`.
While the algorithm is logically sound and tolerant of spurious wakeups, the
lack of data-race annotations (`READ_ONCE()` / `WRITE_ONCE()`) allows the
compiler to perform optimizations that break the algorithm. Specifically, the
compiler is permitted to optimize the conditional write in `rb_wakeups()`:
```c
if (buffer->irq_work.waiters_pending) {
buffer->irq_work.waiters_pending = false;
rb_irq_work_queue(&buffer->irq_work);
}
```
into an unconditional write to avoid a branch (store hoisting / branch
elimination):
```c
bool temp = buffer->irq_work.waiters_pending;
buffer->irq_work.waiters_pending = false; // Unconditional write!
if (temp) {
rb_irq_work_queue(&buffer->irq_work);
}
```
If this optimization is performed, a concurrent reader setting `waiters_pending
= true` can have its write silently overwritten and lost, leading to a permanent
hang (lost wakeup).
#### Failure Sequence (Lost Wakeup / Hang)
```text
CPU 0 (Writer 1) CPU 1 (Reader)
// Writer 1 commits an event
// and enters rb_wakeups()
// Compiler-optimized rb_wakeups():
// 1. Read waiters_pending into register
reg = buffer->irq_work.waiters_pending // reg = false
// Reader enters ring_buffer_wait()
// and sets waiters_pending to true
buffer->irq_work.waiters_pending = true
// 2. Unconditionally write false
buffer->irq_work.waiters_pending = false
// (This overwrites Reader's 'true'!)
// Reader checks for data
rb_watermark_hit() // returns false
// Reader goes to sleep
schedule()
// 3. Check register and skip queueing
if (reg) // false, so skip rb_irq_work_queue
```
Subsequently, when Writer 2 commits another event (which now hits the reader's
watermark), it enters `rb_wakeups()`, reads `waiters_pending` as `false` (since
it was overwritten by Writer 1), and does not queue the wakeup. The reader
sleeps indefinitely.
---
### Recommended Fix
To prevent the compiler from performing unsafe optimizations such as store
hoisting or merging, all accesses to `waiters_pending` and
`full_waiters_pending` must be annotated with `READ_ONCE()` and `WRITE_ONCE()`.
For example, in `rb_wakeups()`:
```c
static __always_inline void
rb_wakeups(struct trace_buffer *buffer, struct ring_buffer_per_cpu *cpu_buffer)
{
if (READ_ONCE(buffer->irq_work.waiters_pending)) {
WRITE_ONCE(buffer->irq_work.waiters_pending, false);
/* irq_work_queue() supplies its own memory barriers */
rb_irq_work_queue(&buffer->irq_work);
}
if (READ_ONCE(cpu_buffer->irq_work.waiters_pending)) {
WRITE_ONCE(cpu_buffer->irq_work.waiters_pending, false);
/* irq_work_queue() supplies its own memory barriers */
rb_irq_work_queue(&cpu_buffer->irq_work);
}
if (cpu_buffer->last_pages_touch ==
local_read(&cpu_buffer->pages_touched))
return;
if (cpu_buffer->reader_page == cpu_buffer->commit_page)
return;
if (!READ_ONCE(cpu_buffer->irq_work.full_waiters_pending))
return;
cpu_buffer->last_pages_touch = local_read(&cpu_buffer->pages_touched);
if (!full_hit(buffer, cpu_buffer->cpu, cpu_buffer->shortest_full))
return;
cpu_buffer->irq_work.wakeup_full = true;
WRITE_ONCE(cpu_buffer->irq_work.full_waiters_pending, false);
/* irq_work_queue() supplies its own memory barriers */
rb_irq_work_queue(&cpu_buffer->irq_work);
}
```
And similarly, use `WRITE_ONCE()` when setting these flags to `true` in
`rb_wait_cond()` and `ring_buffer_poll_wait()`, and `READ_ONCE()` /
`WRITE_ONCE()` in `rb_wake_up_waiters()`.]
|
| 1/1 |
2026/08/04 02:19 |
action |
kernel-checkouter |
1m
Results: map[KernelSrc:/app/workdir/cache/src/0bd482cc9fd6a5c3cdf3e7f636a3906397462859]
|
| 2/1 |
2026/08/04 02:20 |
action |
kernel-builder |
8m
Results: map[KernelObj:/app/workdir/cache/build/c74dae594c3a60776c3332c6f0edd4bae8630b5f]
|
| 3/1 |
2026/08/04 02:29 |
action |
codesearch-prepare |
6m
Results: map[Index:codesearch-index]
|
| 4/1 |
2026/08/04 02:36 |
agent |
expert |
12m
Model:
Results:
Instruction: # KCSAN Data Race Severity Analysis Protocol
You are an expert Linux kernel concurrency engineer analyzing a Kernel
Concurrency Sanitizer (KCSAN) data race report to determine if it is
**BENIGN** or **HARMFUL**.
## 1. CLASSIFICATIONS
### **BENIGN (Truly Benign)**
The logic is sound and structurally tolerant to compiler optimizations or
stale/torn reads.
- **Diagnostics/Stats:** Reads used only for `/proc`, `/sys`, counters, or
`pr_info`.
- **Heuristic Hints:** A "hint" flag where an old value only causes a
slightly delayed update or a sub-optimal but safe fast-path.
- **Single-Writer Flag Updates:** A single writer updating flags where the
concurrent read is a simple bitwise check (e.g., `flags & MASK`). These are
historically tolerated, assuming neither "Fused Accesses" nor "Ordering
Violations" are relevant in this context.
- **Marked Reloads:** A load feeding into a `cmpxchg()` loop or checked
against a later `READ_ONCE()` reload.
- **Safe Overwrites:** Writing the same value already present.
### **HARMFUL (Logic Bug or Marking Required)**
The race causes incorrect behavior due to a synchronization failure or
because missing annotations allow the compiler to break the algorithm.
**Marking Required for Correctness:**
The algorithm is logically sound but requires annotations (`READ_ONCE()`,
`WRITE_ONCE()`, `smp_load_acquire()`, `smp_store_release()`, etc.) to be safe.
- **Fused Accesses:** The compiler might merge accesses or hoist a load out
of a loop, breaking polling/wait loops (livelocks).
- **Torn Accesses:** A large access (e.g., 64-bit on 32-bit arch) might be
split into multiple non-atomic accesses. Note that `READ_ONCE()` does **not**
guarantee atomicity for 64-bit variables on 32-bit architectures.
- **Ordering Violations:** The race breaks a "happens-before" relationship
(requires primitives with implied or explicit memory barriers).
**Logic Bugs:**
A fundamental synchronization failure. Marking accesses will **not** fix it;
the logic itself must change.
- **Pointers/Lifecycle:** The racing variable is a pointer being dereferenced
or a refcount governing object lifecycle (Use-After-Free risk).
- **Control Flow:** The variable guards a critical section, memory allocation,
or hardware command.
- **Bitfields:** Concurrent writes to different bits in the same word.
Compilers often use non-atomic read-modify-write sequences, meaning a
write to `bit_A` can "clobber" a concurrent write to `bit_B`. However,
do not blindly assume all bitfield accesses are harmful; you must prove
that a concurrent write actually clobbers another in a way that breaks
logic.
- **Complex Structures:** Races on shared lists, trees, or hashmaps.
- **Lossy Updates:** Concurrent plain RMW operations (e.g., `var++`) on
non-diagnostic variables where every increment must be preserved.
- **State Machines:** Races allowing a state machine to bypass transitions
or enter an invalid state.
- **Adjacent Unsynchronized Operations:** Consider races happening at the
same time. For example, if both threads execute `struct->has_elements = true;
list_add(node, &struct->list);`, the race on `has_elements` implies an
adjacent race on `list_head`, which is HARMFUL.
## 2. RESEARCH & ANALYSIS WORKFLOW
1. **Locate the Race:** Find the exact variables and functions in the stack
traces. Do not speculate about hypothetical compiler
behaviors or theoretical dangers (e.g., dismissing something as
"fundamentally unsafe") without tracing the actual data flow to a crash.
2. **Contextualize:** Identify held locks, RCU sections, or interrupt
contexts. Explain why the race is possible (e.g., "Thread A holds `lock`,
but Thread B is a lockless reader").
3. **Data Flow:** Follow the racing variable into its subsequent uses. If
the reader sees a stale or torn value, what branch is taken? Does it lead
to a pointer dereference or `BUG_ON()`? **You must prove harm via this
data flow.**
4. **Failure Sequencing (if HARMFUL):** Construct a two-column execution trace
demonstrating the failure.
*Example format:*
```text
CPU0 CPU1
function_a()
// lockless read
ptr = global_ptr
<PREEMPT>
function_b()
lock(&my_lock)
global_ptr = NULL
unlock(&my_lock)
kfree(ptr)
if (ptr)
*ptr = 1; // -> Use-After-Free!
```
## 3. OUTPUT FORMAT
- **Race Summary:** `[Function A]` vs `[Function B]` on `[Variable/Field]`.
- **Synchronization context:** List held locks and explain the lack of mutual
exclusion.
- **Final Classification:** **BENIGN** or **HARMFUL**.
**If BENIGN:**
- **Reasoning:** Briefly explain why the race is structurally tolerant to stale
or torn reads.
- **Recommended Annotations:** Suggest appropriate annotations (e.g.,
`READ_ONCE()`, `data_race()`).
**If HARMFUL:**
- **Failure Reasoning:** Explain and provide a two-column interleaving showing
exactly how the race leads to a failure such as corruption or kernel crash.
- **Recommended Fix (ONLY if fix is trivial):** Suggest a structural fix (e.g.,
"Hold `mapping->i_pages` lock", "Convert to `atomic_t`") or required memory
ordering annotations (e.g., "Wrap in `READ_ONCE()`", "Use
`smp_load_acquire()`").
Don't make assumptions about the kernel source code (it may be different from what you assume it is).
Extensively use the provided code access tools (codesearch-*, git-*, grepper, etc)
to examine the actual source code, and confirm any assumptions.
Prefer calling several tools at the same time to save round-trips.
Use set-results tool to provide results of the analysis.
It must be called exactly once before the final reply.
Ignore results of this tool.
Prompt:
The data race report is:
==================================================================
BUG: KCSAN: data-race in ring_buffer_unlock_commit / ring_buffer_unlock_commit
write to 0xffff888100074cb4 of 1 bytes by task 8271 on cpu 1:
rb_wakeups kernel/trace/ring_buffer.c:4295 [inline]
ring_buffer_unlock_commit+0x6a/0x440 kernel/trace/ring_buffer.c:4495
__buffer_unlock_commit kernel/trace/trace.h:1697 [inline]
trace_buffer_unlock_commit_regs+0x43/0x310 kernel/trace/trace.c:2049
trace_event_buffer_commit+0x149/0x590 kernel/trace/trace.c:2024
do_trace_event_raw_event_hrtimer_setup include/trace/events/timer.h:193 [inline]
trace_event_raw_event_hrtimer_setup+0xc9/0x170 include/trace/events/timer.h:193
__do_trace_hrtimer_setup include/trace/events/timer.h:193 [inline]
trace_hrtimer_setup include/trace/events/timer.h:193 [inline]
debug_setup_on_stack kernel/time/hrtimer.c:523 [inline]
hrtimer_setup_sleeper_on_stack+0xd1/0xf0 kernel/time/hrtimer.c:2366
schedule_hrtimeout_range_clock+0x71/0x1a0 kernel/time/sleep_timeout.c:211
schedule_hrtimeout_range+0x28/0x40 kernel/time/sleep_timeout.c:263
poll_schedule_timeout fs/select.c:241 [inline]
do_select+0xed3/0xfa0 fs/select.c:603
core_sys_select+0x453/0x730 fs/select.c:677
do_pselect fs/select.c:770 [inline]
__do_sys_pselect6 fs/select.c:809 [inline]
__se_sys_pselect6+0x23f/0x2a0 fs/select.c:800
__x64_sys_pselect6+0x78/0x90 fs/select.c:800
x64_sys_call+0x8d5/0x3020 arch/x86/include/generated/asm/syscalls_64.h:271
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x136/0x3c0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
read to 0xffff888100074cb4 of 1 bytes by task 8264 on cpu 0:
rb_wakeups kernel/trace/ring_buffer.c:4294 [inline]
ring_buffer_unlock_commit+0x54/0x440 kernel/trace/ring_buffer.c:4495
__buffer_unlock_commit kernel/trace/trace.h:1697 [inline]
trace_buffer_unlock_commit_regs+0x43/0x310 kernel/trace/trace.c:2049
trace_event_buffer_commit+0x149/0x590 kernel/trace/trace.c:2024
do_trace_event_raw_event_sock_msg_length include/trace/events/sock.h:267 [inline]
trace_event_raw_event_sock_msg_length+0x10f/0x1c0 include/trace/events/sock.h:267
__do_trace_sock_recv_length include/trace/events/sock.h:304 [inline]
trace_sock_recv_length include/trace/events/sock.h:304 [inline]
call_trace_sock_recv_length+0xa2/0xc0 net/socket.c:1120
sock_recvmsg_nosec+0xdf/0xf0 net/socket.c:1131
____sys_recvmsg+0x26f/0x280 net/socket.c:2902
___sys_recvmsg+0x11f/0x3a0 net/socket.c:2946
do_recvmmsg+0x1e5/0x560 net/socket.c:3041
__sys_recvmmsg net/socket.c:3115 [inline]
__do_sys_recvmmsg net/socket.c:3138 [inline]
__se_sys_recvmmsg net/socket.c:3131 [inline]
__x64_sys_recvmmsg+0xe5/0x170 net/socket.c:3131
x64_sys_call+0x80f/0x3020 arch/x86/include/generated/asm/syscalls_64.h:300
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x136/0x3c0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
value changed: 0x01 -> 0x00
Reported by Kernel Concurrency Sanitizer on:
CPU: 0 UID: 0 PID: 8264 Comm: syz.5.1654 Not tainted syzkaller #0 PREEMPT(lazy)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/24/2026
==================================================================
Reply: ### Race Summary
`ring_buffer_unlock_commit` vs `ring_buffer_unlock_commit` on `buffer->irq_work.waiters_pending` (and similarly on `cpu_buffer->irq_work.waiters_pending` and `full_waiters_pending`).
### Synchronization Context
The ring buffer commit path (`ring_buffer_unlock_commit`) is designed to be highly concurrent and lockless. When multiple CPUs concurrently write events to the ring buffer, they concurrently execute `rb_wakeups()` to check if there are any pending readers/waiters to wake up. Because these checks are performed without mutual exclusion, concurrent reads and writes to `waiters_pending` occur.
---
### Final Classification
**HARMFUL** (Marking Required for Correctness)
---
### Failure Reasoning
The `waiters_pending` flag is a fast-path flag used to notify writers that a reader has gone to sleep and needs to be woken up via `irq_work`.
While the algorithm is logically sound and tolerant of spurious wakeups, the lack of data-race annotations (`READ_ONCE()` / `WRITE_ONCE()`) allows the compiler to perform optimizations that break the algorithm. Specifically, the compiler is permitted to optimize the conditional write in `rb_wakeups()`:
```c
if (buffer->irq_work.waiters_pending) {
buffer->irq_work.waiters_pending = false;
rb_irq_work_queue(&buffer->irq_work);
}
```
into an unconditional write to avoid a branch (store hoisting / branch elimination):
```c
bool temp = buffer->irq_work.waiters_pending;
buffer->irq_work.waiters_pending = false; // Unconditional write!
if (temp) {
rb_irq_work_queue(&buffer->irq_work);
}
```
If this optimization is performed, a concurrent reader setting `waiters_pending = true` can have its write silently overwritten and lost, leading to a permanent hang (lost wakeup).
#### Failure Sequence (Lost Wakeup / Hang)
```text
CPU 0 (Writer 1) CPU 1 (Reader)
// Writer 1 commits an event
// and enters rb_wakeups()
// Compiler-optimized rb_wakeups():
// 1. Read waiters_pending into register
reg = buffer->irq_work.waiters_pending // reg = false
// Reader enters ring_buffer_wait()
// and sets waiters_pending to true
buffer->irq_work.waiters_pending = true
// 2. Unconditionally write false
buffer->irq_work.waiters_pending = false
// (This overwrites Reader's 'true'!)
// Reader checks for data
rb_watermark_hit() // returns false
// Reader goes to sleep
schedule()
// 3. Check register and skip queueing
if (reg) // false, so skip rb_irq_work_queue
```
Subsequently, when Writer 2 commits another event (which now hits the reader's watermark), it enters `rb_wakeups()`, reads `waiters_pending` as `false` (since it was overwritten by Writer 1), and does not queue the wakeup. The reader sleeps indefinitely.
---
### Recommended Fix
To prevent the compiler from performing unsafe optimizations such as store hoisting or merging, all accesses to `waiters_pending` and `full_waiters_pending` must be annotated with `READ_ONCE()` and `WRITE_ONCE()`.
For example, in `rb_wakeups()`:
```c
static __always_inline void
rb_wakeups(struct trace_buffer *buffer, struct ring_buffer_per_cpu *cpu_buffer)
{
if (READ_ONCE(buffer->irq_work.waiters_pending)) {
WRITE_ONCE(buffer->irq_work.waiters_pending, false);
/* irq_work_queue() supplies its own memory barriers */
rb_irq_work_queue(&buffer->irq_work);
}
if (READ_ONCE(cpu_buffer->irq_work.waiters_pending)) {
WRITE_ONCE(cpu_buffer->irq_work.waiters_pending, false);
/* irq_work_queue() supplies its own memory barriers */
rb_irq_work_queue(&cpu_buffer->irq_work);
}
if (cpu_buffer->last_pages_touch == local_read(&cpu_buffer->pages_touched))
return;
if (cpu_buffer->reader_page == cpu_buffer->commit_page)
return;
if (!READ_ONCE(cpu_buffer->irq_work.full_waiters_pending))
return;
cpu_buffer->last_pages_touch = local_read(&cpu_buffer->pages_touched);
if (!full_hit(buffer, cpu_buffer->cpu, cpu_buffer->shortest_full))
return;
cpu_buffer->irq_work.wakeup_full = true;
WRITE_ONCE(cpu_buffer->irq_work.full_waiters_pending, false);
/* irq_work_queue() supplies its own memory barriers */
rb_irq_work_queue(&cpu_buffer->irq_work);
}
```
And similarly, use `WRITE_ONCE()` when setting these flags to `true` in `rb_wait_cond()` and `ring_buffer_poll_wait()`, and `READ_ONCE()` / `WRITE_ONCE()` in `rb_wake_up_waiters()`.
|
| 5/2 |
2026/08/04 02:36 |
llm |
expert |
0m
Model:
Tokens:
input: 6637
output: 37
thoughts: 189
Thoughts: **Analyzing the KCSAN Report: A Data Race Dive**
Okay, let's break down this KCSAN report. It's flagging a data race, specifically between two instances of `ring_buffer_unlock_commit`. The race is on a 1-byte variable, located at `0xffff888100074cb4`. The report points to the `rb_wakeups` variable.
The interesting part is that the conflict is happening within `kernel/trace/ring_buffer.c`, specifically around lines 4294 and 4295. Line 4295 shows a write operation and 4294 shows a read operation. The value observed changing from `0x01` to `0x00`.
Now, I need to understand what's happening with `rb_wakeups` in that exact context. I'll need to use my preferred tools: either I'll jump straight into `codesearch-definition-source` to get the definition of `rb_wakeups` or if I don't have that I will `read-file` to get the surrounding code. Then I can figure out the context of the race and its impact on the system. It should be a pretty straightforward analysis from there.
|
| 6/2 |
2026/08/04 02:36 |
tool |
codesearch-definition-source |
0m
Args: map[ContextFile:kernel/trace/ring_buffer.c Name:rb_wakeups]
Results: map[SourceCode:4291: static __always_inline void
4292: rb_wakeups(struct trace_buffer *buffer, struct ring_buffer_per_cpu *cpu_buffer)
4293: {
4294: if (buffer->irq_work.waiters_pending) {
4295: buffer->irq_work.waiters_pending = false;
4296: /* irq_work_queue() supplies it's own memory barriers */
4297: rb_irq_work_queue(&buffer->irq_work);
4298: }
4299:
4300: if (cpu_buffer->irq_work.waiters_pending) {
4301: cpu_buffer->irq_work.waiters_pending = false;
4302: /* irq_work_queue() supplies it's own memory barriers */
4303: rb_irq_work_queue(&cpu_buffer->irq_work);
4304: }
4305:
4306: if (cpu_buffer->last_pages_touch == local_read(&cpu_buffer->pages_touched))
4307: return;
4308:
4309: if (cpu_buffer->reader_page == cpu_buffer->commit_page)
4310: return;
4311:
4312: if (!cpu_buffer->irq_work.full_waiters_pending)
4313: return;
4314:
4315: cpu_buffer->last_pages_touch = local_read(&cpu_buffer->pages_touched);
4316:
4317: if (!full_hit(buffer, cpu_buffer->cpu, cpu_buffer->shortest_full))
4318: return;
4319:
4320: cpu_buffer->irq_work.wakeup_full = true;
4321: cpu_buffer->irq_work.full_waiters_pending = false;
4322: /* irq_work_queue() supplies it's own memory barriers */
4323: rb_irq_work_queue(&cpu_buffer->irq_work);
4324: }
SourceFile:kernel/trace/ring_buffer.c]
|
| 7/2 |
2026/08/04 02:36 |
llm |
expert |
0m
Model:
Tokens:
input: 7751
output: 43
thoughts: 225
Thoughts: **Analyzing a Potential Race Condition**
Okay, so I'm looking at this snippet of C code and my immediate thought is "Ah!". I see a potential synchronization issue here. Specifically, at line 4294, I'm reading `buffer->irq_work.waiters_pending`, and then at line 4295, it's being written to `false`. That's the key: read and then write.
My expert intuition kicks in. This screams race condition if not handled carefully! I need to understand what's happening here. The critical path is around `waiters_pending` flag, which sounds like it could be protected by `irq_work_queue()`.
My next thought process is to understand the context. What's the type of `buffer->irq_work`? Is it `struct trace_buffer` or `struct ring_buffer_per_cpu`, or something similar? I need to understand what `irq_work` is and how it relates to this flag. I need to know where and how `irq_work_queue` is used.
I'm thinking, "Let's find the definition of those structures." I need to find the definition of `struct trace_buffer` or `struct ring_buffer_per_cpu` and `irq_work` in my code base. Alternatively, I will search for usages of those structures, or the `waiters_pending` variable. That will tell me everything I need to know to know to determine if this code is safe, and understand where that flag is being set in the first place.
|
| 8/2 |
2026/08/04 02:36 |
tool |
codesearch-find-references |
0m
Error:
Args: map[ContextFile:kernel/trace/ring_buffer.c IncludeSnippetLines:5 Name:waiters_pending]
Results: map[References:<nil> TruncatedOutput:false]
|
| 9/2 |
2026/08/04 02:36 |
llm |
expert |
0m
Model:
Tokens:
input: 8384
output: 28
thoughts: 17
Thoughts: Okay, here's my interpretation of that thought process, as if it were my own:
**Investigating Waiter Queueing**
Alright, I need to get to the bottom of this "waiters_pending" issue. My gut tells me there's a bottleneck somewhere related to waiter processes. Rather than manually sifting through logs or code, I'm going to leverage my expertise in using command-line tools for efficient investigation. Specifically, I'm thinking `grepper`. I'll use it to quickly search my codebase and configuration files for any instances of "waiters_pending". This is a fast and effective way to identify relevant code sections, configuration parameters, or even documentation that directly mentions or handles this specific metric or state. Hopefully, this will give me a clear starting point for my diagnosis, and reveal how this is being handled in the codebase. From there, I can start to form hypotheses about what's *causing* the accumulation of pending waiters. Grepping is the fastest way to get a focused overview of where this is defined and how its being used. Let's see what it turns up!
|
| 10/2 |
2026/08/04 02:36 |
tool |
grepper |
0m
Args: map[Expression:waiters_pending PathPrefix:kernel/trace/]
Results: map[Output:kernel/trace/ring_buffer.c=437=struct rb_irq_work {
--
kernel/trace/ring_buffer.c-441- atomic_t seq;
kernel/trace/ring_buffer.c:442: bool waiters_pending;
kernel/trace/ring_buffer.c:443: bool full_waiters_pending;
kernel/trace/ring_buffer.c-444- bool wakeup_full;
--
kernel/trace/ring_buffer.c=816=static void rb_wake_up_waiters(struct irq_work *work)
--
kernel/trace/ring_buffer.c-823- wake_up_all(&rbwork->waiters);
kernel/trace/ring_buffer.c:824: if (rbwork->full_waiters_pending || rbwork->wakeup_full) {
kernel/trace/ring_buffer.c-825- /* Only cpu_buffer sets the above flags */
--
kernel/trace/ring_buffer.c-831- rbwork->wakeup_full = false;
kernel/trace/ring_buffer.c:832: rbwork->full_waiters_pending = false;
kernel/trace/ring_buffer.c-833-
--
kernel/trace/ring_buffer.c=914=rb_wait_cond(struct rb_irq_work *rbwork, struct trace_buffer *buffer,
--
kernel/trace/ring_buffer.c-943- if (full)
kernel/trace/ring_buffer.c:944: rbwork->full_waiters_pending = true;
kernel/trace/ring_buffer.c-945- else
kernel/trace/ring_buffer.c:946: rbwork->waiters_pending = true;
kernel/trace/ring_buffer.c-947-
--
kernel/trace/ring_buffer.c=1039=__poll_t ring_buffer_poll_wait(struct trace_buffer *buffer, int cpu,
--
kernel/trace/ring_buffer.c-1061- /*
kernel/trace/ring_buffer.c:1062: * Only allow full_waiters_pending update to be seen after
kernel/trace/ring_buffer.c-1063- * the shortest_full is set (in rb_watermark_hit). If the
kernel/trace/ring_buffer.c:1064: * writer sees the full_waiters_pending flag set, it will
kernel/trace/ring_buffer.c-1065- * compare the amount in the ring buffer to shortest_full.
--
kernel/trace/ring_buffer.c-1070- * the below smp_mb() makes sure that the update to
kernel/trace/ring_buffer.c:1071: * full_waiters_pending doesn't leak up into the above.
kernel/trace/ring_buffer.c-1072- */
kernel/trace/ring_buffer.c-1073- smp_mb();
kernel/trace/ring_buffer.c:1074: rbwork->full_waiters_pending = true;
kernel/trace/ring_buffer.c-1075- return 0;
--
kernel/trace/ring_buffer.c-1078- poll_wait(filp, &rbwork->waiters, poll_table);
kernel/trace/ring_buffer.c:1079: rbwork->waiters_pending = true;
kernel/trace/ring_buffer.c-1080-
kernel/trace/ring_buffer.c-1081- /*
kernel/trace/ring_buffer.c:1082: * There's a tight race between setting the waiters_pending and
kernel/trace/ring_buffer.c:1083: * checking if the ring buffer is empty. Once the waiters_pending bit
kernel/trace/ring_buffer.c-1084- * is set, the next event will wake the task up, but we can get stuck
--
kernel/trace/ring_buffer.c=4292=rb_wakeups(struct trace_buffer *buffer, struct ring_buffer_per_cpu *cpu_buffer)
kernel/trace/ring_buffer.c-4293-{
kernel/trace/ring_buffer.c:4294: if (buffer->irq_work.waiters_pending) {
kernel/trace/ring_buffer.c:4295: buffer->irq_work.waiters_pending = false;
kernel/trace/ring_buffer.c-4296- /* irq_work_queue() supplies it's own memory barriers */
--
kernel/trace/ring_buffer.c-4299-
kernel/trace/ring_buffer.c:4300: if (cpu_buffer->irq_work.waiters_pending) {
kernel/trace/ring_buffer.c:4301: cpu_buffer->irq_work.waiters_pending = false;
kernel/trace/ring_buffer.c-4302- /* irq_work_queue() supplies it's own memory barriers */
--
kernel/trace/ring_buffer.c-4311-
kernel/trace/ring_buffer.c:4312: if (!cpu_buffer->irq_work.full_waiters_pending)
kernel/trace/ring_buffer.c-4313- return;
--
kernel/trace/ring_buffer.c-4320- cpu_buffer->irq_work.wakeup_full = true;
kernel/trace/ring_buffer.c:4321: cpu_buffer->irq_work.full_waiters_pending = false;
kernel/trace/ring_buffer.c-4322- /* irq_work_queue() supplies it's own memory barriers */
]
|
| 11/2 |
2026/08/04 02:36 |
llm |
expert |
0m
Model:
Tokens:
input: 10126
output: 37
thoughts: 32
Thoughts: Okay, here's my take on those thoughts, framed as an expert's internal monologue:
**Diving into the Ring Buffer Implementation**
Alright, let's get down to brass tacks. I need to refresh my memory on how this ring buffer is handling interrupt-related work. I'm going to jump into `kernel/trace/ring_buffer.c`, specifically around line 437. I seem to recall that's where the definition for `struct rb_irq_work` is located. I'll take a look and confirm that's the structure used to manage deferred actions related to interrupt handling within the ring buffer itself. Understanding this structure is key to grasping how tracing events are managed in a potentially high-interrupt environment. I expect to find details on how interrupts are masked, work is queued and dequeued, and how potential race conditions are addressed to ensure data integrity and avoid impacting system latency. Let's see what the code reveals.
|
| 12/2 |
2026/08/04 02:36 |
tool |
read-file |
0m
Args: map[File:kernel/trace/ring_buffer.c FirstLine:430 LineCount:20]
Results: map[Contents: 430:
431: dpage = page_address(page);
432: rb_init_data_page(dpage);
433:
434: return dpage;
435: }
436:
437: struct rb_irq_work {
438: struct irq_work work;
439: wait_queue_head_t waiters;
440: wait_queue_head_t full_waiters;
441: atomic_t seq;
442: bool waiters_pending;
443: bool full_waiters_pending;
444: bool wakeup_full;
445: };
446:
447: /*
448: * Structure to hold event state and handle nested events.
449: */
]
|
| 13/2 |
2026/08/04 02:36 |
llm |
expert |
0m
Model:
Tokens:
input: 10658
output: 37
thoughts: 53
Thoughts: Okay, here's my take on those code locations, thinking through it like I would myself:
**Diving into the Ring Buffer Implementation**
Alright, let's get down to the nitty-gritty. I want to refresh my memory on the synchronization mechanisms within the ring buffer code, specifically how threads wait for events. I'm going to jump into `kernel/trace/ring_buffer.c`, focusing on the `rb_wait_cond` function around line 914. I need to understand the specifics of that wait condition. I want to see precisely what triggers a thread to wake up. Is it new data arriving, the buffer becoming available, or something else entirely? Then, I also want to follow up and see how it is used in `ring_buffer_poll_wait` around line 1039 to understand how a poll waits for data. The `poll` system call is critical for efficient user-space interaction, so a solid grasp here is essential. I need to understand how those two functions interact. What specific condition triggers the wake up here, compared to the generic wait? This is probably related to file descriptors being used. I need to refresh the key concepts here to quickly recall them.
|
| 14/2 |
2026/08/04 02:36 |
tool |
read-file |
0m
Args: map[File:kernel/trace/ring_buffer.c FirstLine:914 LineCount:80]
Results: map[Contents: 914: rb_wait_cond(struct rb_irq_work *rbwork, struct trace_buffer *buffer,
915: int cpu, int full, ring_buffer_cond_fn cond, void *data)
916: {
917: if (rb_watermark_hit(buffer, cpu, full))
918: return true;
919:
920: if (cond(data))
921: return true;
922:
923: /*
924: * The events can happen in critical sections where
925: * checking a work queue can cause deadlocks.
926: * After adding a task to the queue, this flag is set
927: * only to notify events to try to wake up the queue
928: * using irq_work.
929: *
930: * We don't clear it even if the buffer is no longer
931: * empty. The flag only causes the next event to run
932: * irq_work to do the work queue wake up. The worse
933: * that can happen if we race with !trace_empty() is that
934: * an event will cause an irq_work to try to wake up
935: * an empty queue.
936: *
937: * There's no reason to protect this flag either, as
938: * the work queue and irq_work logic will do the necessary
939: * synchronization for the wake ups. The only thing
940: * that is necessary is that the wake up happens after
941: * a task has been queued. It's OK for spurious wake ups.
942: */
943: if (full)
944: rbwork->full_waiters_pending = true;
945: else
946: rbwork->waiters_pending = true;
947:
948: return false;
949: }
950:
951: struct rb_wait_data {
952: struct rb_irq_work *irq_work;
953: int seq;
954: };
955:
956: /*
957: * The default wait condition for ring_buffer_wait() is to just to exit the
958: * wait loop the first time it is woken up.
959: */
960: static bool rb_wait_once(void *data)
961: {
962: struct rb_wait_data *rdata = data;
963: struct rb_irq_work *rbwork = rdata->irq_work;
964:
965: return atomic_read_acquire(&rbwork->seq) != rdata->seq;
966: }
967:
968: /**
969: * ring_buffer_wait - wait for input to the ring buffer
970: * @buffer: buffer to wait on
971: * @cpu: the cpu buffer to wait on
972: * @full: wait until the percentage of pages are available, if @cpu != RING_BUFFER_ALL_CPUS
973: * @cond: condition function to break out of wait (NULL to run once)
974: * @data: the data to pass to @cond.
975: *
976: * If @cpu == RING_BUFFER_ALL_CPUS then the task will wake up as soon
977: * as data is added to any of the @buffer's cpu buffers. Otherwise
978: * it will wait for data to be added to a specific cpu buffer.
979: */
980: int ring_buffer_wait(struct trace_buffer *buffer, int cpu, int full,
981: ring_buffer_cond_fn cond, void *data)
982: {
983: struct ring_buffer_per_cpu *cpu_buffer;
984: struct wait_queue_head *waitq;
985: struct rb_irq_work *rbwork;
986: struct rb_wait_data rdata;
987: int ret = 0;
988:
989: /*
990: * Depending on what the caller is waiting for, either any
991: * data in any cpu buffer, or a specific buffer, put the
992: * caller on the appropriate wait queue.
993: */
]
|
| 15/2 |
2026/08/04 02:36 |
llm |
expert |
0m
Model:
Tokens:
input: 12258
output: 37
thoughts: 28
Thoughts: Here's my take on the thought process, as requested:
**Diving Deeper into `rbwork->waiters_pending`**
Okay, so we're looking at `ring_buffer_wait` and its implementation now. I need to understand exactly how it's leveraging `rbwork->waiters_pending`. My initial assumption is it's handling some form of synchronization or signaling mechanism related to waiting consumers and producers on the ring buffer, but I need to pinpoint the *how*. I want to see *exactly* how the `waiters_pending` field is being used. Let's see how it's being set, cleared, tested. I expect it'll be fundamental to ensuring correct data flow and preventing race conditions around the circular buffer.
|
| 16/2 |
2026/08/04 02:36 |
tool |
read-file |
0m
Args: map[File:kernel/trace/ring_buffer.c FirstLine:994 LineCount:50]
Results: map[Contents: 994: if (cpu == RING_BUFFER_ALL_CPUS) {
995: rbwork = &buffer->irq_work;
996: /* Full only makes sense on per cpu reads */
997: full = 0;
998: } else {
999: if (!cpumask_test_cpu(cpu, buffer->cpumask))
1000: return -ENODEV;
1001: cpu_buffer = buffer->buffers[cpu];
1002: rbwork = &cpu_buffer->irq_work;
1003: }
1004:
1005: if (full)
1006: waitq = &rbwork->full_waiters;
1007: else
1008: waitq = &rbwork->waiters;
1009:
1010: /* Set up to exit loop as soon as it is woken */
1011: if (!cond) {
1012: cond = rb_wait_once;
1013: rdata.irq_work = rbwork;
1014: rdata.seq = atomic_read_acquire(&rbwork->seq);
1015: data = &rdata;
1016: }
1017:
1018: ret = wait_event_interruptible((*waitq),
1019: rb_wait_cond(rbwork, buffer, cpu, full, cond, data));
1020:
1021: return ret;
1022: }
1023:
1024: /**
1025: * ring_buffer_poll_wait - poll on buffer input
1026: * @buffer: buffer to wait on
1027: * @cpu: the cpu buffer to wait on
1028: * @filp: the file descriptor
1029: * @poll_table: The poll descriptor
1030: * @full: wait until the percentage of pages are available, if @cpu != RING_BUFFER_ALL_CPUS
1031: *
1032: * If @cpu == RING_BUFFER_ALL_CPUS then the task will wake up as soon
1033: * as data is added to any of the @buffer's cpu buffers. Otherwise
1034: * it will wait for data to be added to a specific cpu buffer.
1035: *
1036: * Returns EPOLLIN | EPOLLRDNORM if data exists in the buffers,
1037: * zero otherwise.
1038: */
1039: __poll_t ring_buffer_poll_wait(struct trace_buffer *buffer, int cpu,
1040: struct file *filp, poll_table *poll_table, int full)
1041: {
1042: struct ring_buffer_per_cpu *cpu_buffer;
1043: struct rb_irq_work *rbwork;
]
|
| 17/2 |
2026/08/04 02:36 |
llm |
expert |
0m
Model:
Tokens:
input: 13277
output: 38
thoughts: 16
Thoughts: Okay, here's my interpretation of that thought process, assuming I'm already deep in the weeds of kernel development or a similar low-level system.
**Diving Deeper into the Ring Buffer**
Alright, that's a good starting point. `ring_buffer_poll_wait`... that function name suggests interaction with a polling mechanism, probably involving a wait queue. I need to understand exactly how this ring buffer is interacting with waiting processes. What's the context? Is it related to interrupt handling, data transfer between kernel and userspace, or something else entirely? Time to dig into the rest of the code for `ring_buffer_poll_wait`. I'm hoping to see the specific conditions that trigger a wait, the wakeup mechanisms used, and how the buffer's state is modified during those operations. I'll be looking for things like mutexes, spinlocks, and other synchronization primitives – gotta make sure we're avoiding race conditions. Let's get to it.
|
| 18/2 |
2026/08/04 02:36 |
tool |
read-file |
0m
Args: map[File:kernel/trace/ring_buffer.c FirstLine:1044 LineCount:60]
Results: map[Contents:1044:
1045: if (cpu == RING_BUFFER_ALL_CPUS) {
1046: rbwork = &buffer->irq_work;
1047: full = 0;
1048: } else {
1049: if (!cpumask_test_cpu(cpu, buffer->cpumask))
1050: return EPOLLERR;
1051:
1052: cpu_buffer = buffer->buffers[cpu];
1053: rbwork = &cpu_buffer->irq_work;
1054: }
1055:
1056: if (full) {
1057: poll_wait(filp, &rbwork->full_waiters, poll_table);
1058:
1059: if (rb_watermark_hit(buffer, cpu, full))
1060: return EPOLLIN | EPOLLRDNORM;
1061: /*
1062: * Only allow full_waiters_pending update to be seen after
1063: * the shortest_full is set (in rb_watermark_hit). If the
1064: * writer sees the full_waiters_pending flag set, it will
1065: * compare the amount in the ring buffer to shortest_full.
1066: * If the amount in the ring buffer is greater than the
1067: * shortest_full percent, it will call the irq_work handler
1068: * to wake up this list. The irq_handler will reset shortest_full
1069: * back to zero. That's done under the reader_lock, but
1070: * the below smp_mb() makes sure that the update to
1071: * full_waiters_pending doesn't leak up into the above.
1072: */
1073: smp_mb();
1074: rbwork->full_waiters_pending = true;
1075: return 0;
1076: }
1077:
1078: poll_wait(filp, &rbwork->waiters, poll_table);
1079: rbwork->waiters_pending = true;
1080:
1081: /*
1082: * There's a tight race between setting the waiters_pending and
1083: * checking if the ring buffer is empty. Once the waiters_pending bit
1084: * is set, the next event will wake the task up, but we can get stuck
1085: * if there's only a single event in.
1086: *
1087: * FIXME: Ideally, we need a memory barrier on the writer side as well,
1088: * but adding a memory barrier to all events will cause too much of a
1089: * performance hit in the fast path. We only need a memory barrier when
1090: * the buffer goes from empty to having content. But as this race is
1091: * extremely small, and it's not a problem if another event comes in, we
1092: * will fix it later.
1093: */
1094: smp_mb();
1095:
1096: if ((cpu == RING_BUFFER_ALL_CPUS && !ring_buffer_empty(buffer)) ||
1097: (cpu != RING_BUFFER_ALL_CPUS && !ring_buffer_empty_cpu(buffer, cpu)))
1098: return EPOLLIN | EPOLLRDNORM;
1099: return 0;
1100: }
1101:
1102: /* buffer may be either ring_buffer or ring_buffer_per_cpu */
1103: #define RB_WARN_ON(b, cond) \
]
|
| 19/2 |
2026/08/04 02:36 |
llm |
expert |
0m
Model:
Tokens:
input: 14555
output: 41
thoughts: 967
Thoughts: **Analyzing a Data Race in the Ring Buffer Wakeup Mechanism**
Okay, let's break this down. I'm looking at the comments in `rb_wait_cond` and `ring_buffer_poll_wait`, and specifically, this comment in `rb_wait_cond` is intriguing: *"There's no reason to protect this flag either, as the work queue and irq_work logic will do the necessary synchronization for the wake ups. The only thing that is necessary is that the wake up happens after a task has been queued. It's OK for spurious wake ups."*
But then, KCSAN flagged a data race: a read/write conflict on `rb_wakeups` within `ring_buffer_unlock_commit`. That's unexpected. Both the read and the write are happening within the same function, which is unusual for a race condition. Let me look at the code for `rb_wakeups` more closely:
```c
static __always_inline void
rb_wakeups(struct trace_buffer *buffer, struct ring_buffer_per_cpu *cpu_buffer)
{
if (buffer->irq_work.waiters_pending) {
buffer->irq_work.waiters_pending = false;
/* irq_work_queue() supplies it's own memory barriers */
rb_irq_work_queue(&buffer->irq_work);
}
}
```
Okay, I see the problem. If two CPUs are executing `ring_buffer_unlock_commit` concurrently, they can both call `rb_wakeups`. CPU 0 reads `buffer->irq_work.waiters_pending` and sees it's true. CPU 1 reads the same flag and sees true, too. Then CPU 1 writes `buffer->irq_work.waiters_pending = false`, and CPU 0 *also* writes `buffer->irq_work.waiters_pending = false`.
Now, if both see `true`, both will call `rb_irq_work_queue(&buffer->irq_work)`. My immediate concern is that this double call will then cause some issue. So, let's see what `rb_irq_work_queue` does, and specifically if it handles these concurrent calls correctly. I need to understand what `irq_work_queue` actually does. Is it designed to handle this kind of race condition, or is the comment I read earlier inaccurate? I need to verify that this potential double queuing isn't causing a real problem.
|
| 20/2 |
2026/08/04 02:36 |
tool |
codesearch-definition-source |
0m
Args: map[ContextFile:kernel/trace/ring_buffer.c Name:rb_irq_work_queue]
Results: map[SourceCode:4274: static bool
4275: rb_irq_work_queue(struct rb_irq_work *irq_work)
4276: {
4277: int cpu;
4278:
4279: /* irq_work_queue_on() is not NMI-safe */
4280: if (unlikely(in_nmi()))
4281: return irq_work_queue(&irq_work->work);
4282:
4283: /*
4284: * If CPU isolation is not active, cpu is always the current
4285: * CPU, and the following is equivallent to irq_work_queue().
4286: */
4287: cpu = housekeeping_any_cpu(HK_TYPE_KERNEL_NOISE);
4288: return irq_work_queue_on(&irq_work->work, cpu);
4289: }
SourceFile:kernel/trace/ring_buffer.c]
|
| 21/2 |
2026/08/04 02:36 |
llm |
expert |
0m
Model:
Tokens:
input: 16403
output: 36
thoughts: 159
Thoughts: **Analyzing Concurrency in irq_work_queue**
Okay, so `irq_work_queue` and `irq_work_queue_on` are designed to be called concurrently, which makes sense. They use atomic operations internally, right? That should handle the basic race conditions around queuing the work. Still, I should double-check. Just because it *should* work doesn't mean it *does* in the wild. I'm especially wary of compiler optimizations that could introduce a subtle data race, or some other benign race.
First things first, let's look closely at how the internal mechanics work. Let's start with `waiters_pending`. That looks like a boolean, right? Yes, confirmed! `bool waiters_pending;` in `struct rb_irq_work`. Okay, that's good.
But where else is that thing being used? More specifically, where is `waiters_pending` written to, or read from? Gotta cover all angles. Time to dig into the codebase. Let's see... I'll check the git log of `kernel/trace/ring_buffer.c` and search for both `waiters_pending` and `rb_wakeups` for clues. This should show me any recent changes, any potential areas of concern, and give me a good overview of how the variable is being used.
|
| 22/2 |
2026/08/04 02:36 |
tool |
git-log |
0m
Args: map[CodeRegexp:waiters_pending PathPrefix:kernel/trace/ring_buffer.c]
Results: map[Output:7af9ded0c2ca ring-buffer: Use wait_event_interruptible() in ring_buffer_wait()
e36f19a6457b ring-buffer: Reuse rb_watermark_hit() for the poll logic
8145f1c35fa6 ring-buffer: Fix full_waiters_pending in poll
68282dd930ea ring-buffer: Fix resetting of shortest_full
b3594573681b ring-buffer: Fix waking up ring buffer readers
42fb0a1e84ff tracing/ring-buffer: Have polling block on watermark
ec0bbc5ec566 ring-buffer: Check pending waiters when doing wake ups as well
03329f993978 tracing: Add tracefs file buffer_percentage
d90fd77402d3 ring-buffer: Reorganize function locations
1e0d6714aceb ring-buffer: Do not wake up a splice waiter when page is not full
]
|
| 23/2 |
2026/08/04 02:36 |
llm |
expert |
0m
Model:
Tokens:
input: 17143
output: 23
thoughts: 43
Thoughts: Here's my analysis of the provided text, rendered in the first person, assuming I'm an expert in the field:
**Diving into the `waiters_pending` Implementation**
Okay, so I need to get a handle on the design and evolution of `waiters_pending`. The fastest way to get up to speed here is to trace the commits. Looks like `ec0bbc5ec566` and `b3594573681b` should provide the key context. I'll start with those and see how the `waiters_pending` mechanism was introduced and then how it's evolved. I'm expecting these commits to reveal details about the initial requirements that drove the design, any trade-offs made, and, importantly, what problem this component was originally intended to solve. From there, I'll be able to quickly grasp the implications for the overall architecture and how `waiters_pending` integrates with other subsystems. I'll be looking for things like mutexes, condition variables, atomic operations... the usual suspects when dealing with concurrent state management. I'm especially keen to spot any potential bottlenecks or areas that might require optimization down the line. Let's see what those commits reveal.
|
| 24/2 |
2026/08/04 02:37 |
tool |
git-show |
0m
Args:
Results: map[Output:commit ec0bbc5ec5664dcee344f79373852117dc672c86
Author: Steven Rostedt (Google) <rostedt@goodmis.org>
Date: Tue Sep 27 19:15:25 2022 -0400
ring-buffer: Check pending waiters when doing wake ups as well
The wake up waiters only checks the "wakeup_full" variable and not the
"full_waiters_pending". The full_waiters_pending is set when a waiter is
added to the wait queue. The wakeup_full is only set when an event is
triggered, and it clears the full_waiters_pending to avoid multiple calls
to irq_work_queue().
The irq_work callback really needs to check both wakeup_full as well as
full_waiters_pending such that this code can be used to wake up waiters
when a file is closed that represents the ring buffer and the waiters need
to be woken up.
Link: https://lkml.kernel.org/r/20220927231824.209460321@goodmis.org
Cc: stable@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Fixes: 15693458c4bc0 ("tracing/ring-buffer: Move poll wake ups into ring buffer code")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 02db92c9eb1b..5a7d818ca3ea 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -917,8 +917,9 @@ static void rb_wake_up_waiters(struct irq_work *work)
struct rb_irq_work *rbwork = container_of(work, struct rb_irq_work, work);
wake_up_all(&rbwork->waiters);
- if (rbwork->wakeup_full) {
+ if (rbwork->full_waiters_pending || rbwork->wakeup_full) {
rbwork->wakeup_full = false;
+ rbwork->full_waiters_pending = false;
wake_up_all(&rbwork->full_waiters);
}
}
]
|
| 25/2 |
2026/08/04 02:37 |
llm |
expert |
0m
Model:
Tokens:
input: 18132
output: 27
thoughts: 19
Thoughts: Okay, let's dive into commit `b3594573681b`. I need to understand what changed here, and the context around it. My initial thought process is this:
**Analyzing Commit b3594573681b**
First, I'll quickly check the diff. What files were touched? Are we talking about a core algorithm, a UI tweak, a configuration change, or documentation? The filenames will guide my initial understanding. I'll need to pay close attention to the specific lines added, removed, or modified. Are we introducing new functionality, fixing a bug, refactoring code for clarity, or optimizing performance?
If it's code, I'll look for the logic. Is there a clear pattern? Did they add a new module or class, modify an existing function, or rewrite something significant? I'll be looking for changes in data structures, control flow, and any potential side effects. Any new dependencies introduced? Are there any obvious performance implications of the changes?
If it's a bug fix, I'll need to understand the original problem. Does the commit message provide enough context? If not, I'll need to review the related issues or pull requests. I'll check if any tests were added to confirm the fix and prevent regressions.
For a refactoring, I'm expecting improved readability, maintainability, and possibly performance. I'll need to assess if the changes make the code easier to understand and evolve. Does it introduce any new abstractions or design patterns?
In general, I'll be looking for an understanding of the impact to the system as a whole. What is the overall architectural impact of the change? Do the changes impact security, or performance? I will compare it to any knowledge I already have, or can quickly recall, about the project.
|
| 26/2 |
2026/08/04 02:37 |
tool |
git-show |
0m
Args:
Results: map[Output:commit b3594573681b53316ec0365332681a30463edfd6
Author: Steven Rostedt (Google) <rostedt@goodmis.org>
Date: Fri Mar 8 15:24:03 2024 -0500
ring-buffer: Fix waking up ring buffer readers
A task can wait on a ring buffer for when it fills up to a specific
watermark. The writer will check the minimum watermark that waiters are
waiting for and if the ring buffer is past that, it will wake up all the
waiters.
The waiters are in a wait loop, and will first check if a signal is
pending and then check if the ring buffer is at the desired level where it
should break out of the loop.
If a file that uses a ring buffer closes, and there's threads waiting on
the ring buffer, it needs to wake up those threads. To do this, a
"wait_index" was used.
Before entering the wait loop, the waiter will read the wait_index. On
wakeup, it will check if the wait_index is different than when it entered
the loop, and will exit the loop if it is. The waker will only need to
update the wait_index before waking up the waiters.
This had a couple of bugs. One trivial one and one broken by design.
The trivial bug was that the waiter checked the wait_index after the
schedule() call. It had to be checked between the prepare_to_wait() and
the schedule() which it was not.
The main bug is that the first check to set the default wait_index will
always be outside the prepare_to_wait() and the schedule(). That's because
the ring_buffer_wait() doesn't have enough context to know if it should
break out of the loop.
The loop itself is not needed, because all the callers to the
ring_buffer_wait() also has their own loop, as the callers have a better
sense of what the context is to decide whether to break out of the loop
or not.
Just have the ring_buffer_wait() block once, and if it gets woken up, exit
the function and let the callers decide what to do next.
Link: https://lore.kernel.org/all/CAHk-=whs5MdtNjzFkTyaUy=vHi=qwWgPi0JgTe6OYUYMNSRZfg@mail.gmail.com/
Link: https://lore.kernel.org/linux-trace-kernel/20240308202431.792933613@goodmis.org
Cc: stable@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linke li <lilinke99@qq.com>
Cc: Rabin Vincent <rabin@rab.in>
Fixes: e30f53aad2202 ("tracing: Do not busy wait in buffer splice")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 0699027b4f4c..3400f11286e3 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -384,7 +384,6 @@ struct rb_irq_work {
struct irq_work work;
wait_queue_head_t waiters;
wait_queue_head_t full_waiters;
- long wait_index;
bool waiters_pending;
bool full_waiters_pending;
bool wakeup_full;
@@ -798,14 +797,40 @@ void ring_buffer_wake_waiters(struct trace_buffer *buffer, int cpu)
rbwork = &cpu_buffer->irq_work;
}
- rbwork->wait_index++;
- /* make sure the waiters see the new index */
- smp_wmb();
-
/* This can be called in any context */
irq_work_queue(&rbwork->work);
}
+static bool rb_watermark_hit(struct trace_buffer *buffer, int cpu, int full)
+{
+ struct ring_buffer_per_cpu *cpu_buffer;
+ bool ret = false;
+
+ /* Reads of all CPUs always waits for any data */
+ if (cpu == RING_BUFFER_ALL_CPUS)
+ return !ring_buffer_empty(buffer);
+
+ cpu_buffer = buffer->buffers[cpu];
+
+ if (!ring_buffer_empty_cpu(buffer, cpu)) {
+ unsigned long flags;
+ bool pagebusy;
+
+ if (!full)
+ return true;
+
+ raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
+ pagebusy = cpu_buffer->reader_page == cpu_buffer->commit_page;
+ ret = !pagebusy && full_hit(buffer, cpu, full);
+
+ if (!cpu_buffer->shortest_full ||
+ cpu_buffer->shortest_full > full)
+ cpu_buffer->shortest_full = full;
+ raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
+ }
+ return ret;
+}
+
/**
* ring_buffer_wait - wait for input to the ring buffer
* @buffer: buffer to wait on
@@ -821,7 +846,6 @@ int ring_buffer_wait(struct trace_buffer *buffer, int cpu, int full)
struct ring_buffer_per_cpu *cpu_buffer;
DEFINE_WAIT(wait);
struct rb_irq_work *work;
- long wait_index;
int ret = 0;
/*
@@ -840,81 +864,54 @@ int ring_buffer_wait(struct trace_buffer *buffer, int cpu, int full)
work = &cpu_buffer->irq_work;
}
- wait_index = READ_ONCE(work->wait_index);
-
- while (true) {
- if (full)
- prepare_to_wait(&work->full_waiters, &wait, TASK_INTERRUPTIBLE);
- else
- prepare_to_wait(&work->waiters, &wait, TASK_INTERRUPTIBLE);
-
- /*
- * The events can happen in critical sections where
- * checking a work queue can cause deadlocks.
- * After adding a task to the queue, this flag is set
- * only to notify events to try to wake up the queue
- * using irq_work.
- *
- * We don't clear it even if the buffer is no longer
- * empty. The flag only causes the next event to run
- * irq_work to do the work queue wake up. The worse
- * that can happen if we race with !trace_empty() is that
- * an event will cause an irq_work to try to wake up
- * an empty queue.
- *
- * There's no reason to protect this flag either, as
- * the work queue and irq_work logic will do the necessary
- * synchronization for the wake ups. The only thing
- * that is necessary is that the wake up happens after
- * a task has been queued. It's OK for spurious wake ups.
- */
- if (full)
- work->full_waiters_pending = true;
- else
- work->waiters_pending = true;
-
- if (signal_pending(current)) {
- ret = -EINTR;
- break;
- }
-
- if (cpu == RING_BUFFER_ALL_CPUS && !ring_buffer_empty(buffer))
- break;
-
- if (cpu != RING_BUFFER_ALL_CPUS &&
- !ring_buffer_empty_cpu(buffer, cpu)) {
- unsigned long flags;
- bool pagebusy;
- bool done;
-
- if (!full)
- break;
-
- raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
- pagebusy = cpu_buffer->reader_page == cpu_buffer->commit_page;
- done = !pagebusy && full_hit(buffer, cpu, full);
+ if (full)
+ prepare_to_wait(&work->full_waiters, &wait, TASK_INTERRUPTIBLE);
+ else
+ prepare_to_wait(&work->waiters, &wait, TASK_INTERRUPTIBLE);
- if (!cpu_buffer->shortest_full ||
- cpu_buffer->shortest_full > full)
- cpu_buffer->shortest_full = full;
- raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
- if (done)
- break;
- }
+ /*
+ * The events can happen in critical sections where
+ * checking a work queue can cause deadlocks.
+ * After adding a task to the queue, this flag is set
+ * only to notify events to try to wake up the queue
+ * using irq_work.
+ *
+ * We don't clear it even if the buffer is no longer
+ * empty. The flag only causes the next event to run
+ * irq_work to do the work queue wake up. The worse
+ * that can happen if we race with !trace_empty() is that
+ * an event will cause an irq_work to try to wake up
+ * an empty queue.
+ *
+ * There's no reason to protect this flag either, as
+ * the work queue and irq_work logic will do the necessary
+ * synchronization for the wake ups. The only thing
+ * that is necessary is that the wake up happens after
+ * a task has been queued. It's OK for spurious wake ups.
+ */
+ if (full)
+ work->full_waiters_pending = true;
+ else
+ work->waiters_pending = true;
- schedule();
+ if (rb_watermark_hit(buffer, cpu, full))
+ goto out;
- /* Make sure to see the new wait index */
- smp_rmb();
- if (wait_index != work->wait_index)
- break;
+ if (signal_pending(current)) {
+ ret = -EINTR;
+ goto out;
}
+ schedule();
+ out:
if (full)
finish_wait(&work->full_waiters, &wait);
else
finish_wait(&work->waiters, &wait);
+ if (!ret && !rb_watermark_hit(buffer, cpu, full) && signal_pending(current))
+ ret = -EINTR;
+
return ret;
}
]
|
| 27/2 |
2026/08/04 02:37 |
llm |
expert |
0m
Model:
Tokens:
input: 21451
output: 39
thoughts: 931
Thoughts: **Analyzing Potential Compiler Optimization Issues with a Flag in a Concurrency Context**
Right, let's break this down. That comment in the commit diff, the one talking about `waiters_pending` in `rb_wait_cond`, essentially says that there's *no need* to protect this flag because the work queue and `irq_work` logic handle synchronization. Spurious wakeups are acceptable as long as the wakeup happens *after* a task is queued. Okay, I get the intention... but is this *completely* bulletproof? My expert intuition tells me to dig a little deeper.
My immediate concern is: Could a compiler optimization trip us up here? Specifically, could optimizations applied to `buffer->irq_work.waiters_pending` lead to a bug in concurrent access scenarios? We are talking about flags that are read and written to concurrently, so any potential compiler reordering or atomicity issues must be carefully considered.
Let's look at `rb_wakeups`. We have this:
```c
static __always_inline void
rb_wakeups(struct trace_buffer *buffer, struct ring_buffer_per_cpu *cpu_buffer)
{
if (buffer->irq_work.waiters_pending) {
buffer->irq_work.waiters_pending = false;
/* irq_work_queue() supplies it's own memory barriers */
rb_irq_work_queue(&buffer->irq_work);
}
```
My first concern is regarding the read and write of the `waiters_pending` flag. Could the compiler, without explicit memory barriers, reorder these operations in a way that breaks our expected behavior? More specifically, could the compiler split the write, or could it do a torn write?
The flag is a `bool` (1 byte), right? On x86 and most architectures, a single byte write is atomic. This is critical. A torn write is not possible.
However, I'm thinking about "Fused Accesses" and "Control Flow" and Bitfields here. Are there any gotchas that I'm missing? Is `waiters_pending` part of a bitfield? Let's verify by looking again at the struct.
```c
struct rb_irq_work {
struct irq_work work;
wait_queue_head_t waiters;
wait_queue_head_t full_waiters;
atomic_t seq;
bool waiters_pending;
bool full_waiters_pending;
bool wakeup_full;
};
```
Okay, there are three separate `bool` fields here. `waiters_pending`, `full_waiters_pending`, and `wakeup_full`. The question is, are they separate bytes in memory, or is the compiler packing them somehow?
In C, unless they are declared as bitfields (e.g., `bool waiters_pending:1;`), separate `bool` fields within a struct are allocated as separate bytes. These are simply declared as `bool waiters_pending;`.
Therefore, each is a separate byte. And the single byte write is atomic.
Now, to make *absolutely sure* let's double check this using `codesearch-struct-layout` or `codeexpert` to confirm the struct layout of `rb_irq_work`. Just to be 100% sure.
|
| 28/2 |
2026/08/04 02:37 |
tool |
codesearch-struct-layout |
0m
Args: map[ContextFile:kernel/trace/ring_buffer.c Name:rb_irq_work]
Results: map[Fields:[map[Name:work OffsetBits:0 SizeBits:256] map[Name:waiters OffsetBits:256 SizeBits:192] map[Name:full_waiters OffsetBits:448 SizeBits:192] map[Name:seq OffsetBits:640 SizeBits:32] map[Name:waiters_pending OffsetBits:672 SizeBits:8] map[Name:full_waiters_pending OffsetBits:680 SizeBits:8] map[Name:wakeup_full OffsetBits:688 SizeBits:8]]]
|
| 29/2 |
2026/08/04 02:37 |
llm |
expert |
0m
Model:
Tokens:
input: 23332
output: 21
thoughts: 3391
Thoughts: **Analyzing a Potential Data Race in `ring_buffer` Wakeup Mechanism**
Okay, so I'm looking at these three variables in the `ring_buffer` implementation: `waiters_pending`, `full_waiters_pending`, and `wakeup_full`. They are distinct bytes, which, on the surface, seems fine. But a compiler could potentially mess with this by performing optimizations. My mind immediately jumps to potential compiler optimizations; like a write to one variable clobbering another. Since the variables are bytes, the compiler will use byte-sized instructions like `movb`, but even on some architectures, byte writes could be implemented as a read-modify-write on a larger word.
The crux of my concern revolves around KCSAN detecting data races on concurrent unsynchronized accesses. Is the data race *benign* here, or does it potentially lead to missing wakeups? Let's trace the execution path within the `ring_buffer_wait()` and `rb_wakeups()` functions. We have a reader going to sleep with `waiters_pending = true`, and a writer calling `rb_wakeups()`, which sets `waiters_pending = false` and queues irq work.
My first thought is, what if two writers concurrently write? This is where the issues can emerge. I have to analyze the scenario where one reader and two writers race. Even if both queue irq work it still seems fine. But then I get to scenarios where the reader has slept, a writer sets `waiters_pending = false`, then a new reader sets it to true, and then another writer sets it back to false. I have to trace how the `waiters_pending` variable works in these scenarios, what happens when it's read and set to true, and when it's set to false.
Thinking about the code, my concern then becomes the standard race between reader sleeping and writer waking up. I wonder if there is a memory barrier on the writer side. It turns out that there isn't one. The comment in `ring_buffer_poll_wait` explicitly states that this is because of performance reasons, and they're accepting an extremely small race. The comment says that if one more event comes in, it will be woken up!
But the KCSAN report isn't about the missing memory barrier between data write and the `waiters_pending` read; it's about a data race on `waiters_pending` itself, between *two* concurrent writers, which has to be an issue! I start to wonder if the data race on `waiters_pending` is harmful. If they both call `rb_wakeups` concurrently, and both see waiters pending as true, then they will both set it to false and both queue the irq work. This could still be fine! But my biggest concern is about *compiler optimizations*. Could the compiler change the following:
```c
if (buffer->irq_work.waiters_pending) {
buffer->irq_work.waiters_pending = false;
/* irq_work_queue() supplies it's own memory barriers */
rb_irq_work_queue(&buffer->irq_work);
}
```
into:
```c
bool pending = buffer->irq_work.waiters_pending;
buffer->irq_work.waiters_pending = false;
if (pending) {
rb_irq_work_queue(&buffer->irq_work);
}
```
Wait, no! It could become this, where there is an unconditional write:
```c
buffer->irq_work.waiters_pending = false;
```
This *would* be a problem. This means that, even if `waiters_pending` was `false`, the writer would unconditionally write `false`, potentially overwriting a reader's `true`. If the reader's `true` is overwritten with `false`, then the next writer won't wake up the reader, which means that the reader could sleep forever!
Wait, is this even a real thing? Yes! The compiler is allowed to do this, because it assumes single threaded execution, but with a data race, it is a problem. The LKMM explicitly warns about these compiler optimizations on non-atomic variables: merging, conditional write becoming unconditional write, splitting, and reordering.
Now I need to check if there are any KCSAN reports or patches for this type of issue, so I need to search the git log! I need to search for `waiters_pending` in git commits or mailing lists.
|
| 30/2 |
2026/08/04 02:37 |
tool |
git-log |
0m
Args: map[MessageRegexps:[waiters_pending]]
Results: map[Output:7af9ded0c2ca ring-buffer: Use wait_event_interruptible() in ring_buffer_wait()
8145f1c35fa6 ring-buffer: Fix full_waiters_pending in poll
ec0bbc5ec566 ring-buffer: Check pending waiters when doing wake ups as well
76835b0ebf8a futex: Ensure get_futex_key_refs() always implies a barrier
4ce97dbf5024 trace: Fix epoll hang when we race with new entries
11d4616bd07f futex: revert back to the explicit waiter counting code
]
|
| 31/2 |
2026/08/04 02:37 |
llm |
expert |
0m
Model:
Tokens:
input: 27898
output: 21
thoughts: 15
Thoughts: Here's my thought process, summarized as you requested:
**Investigating Ruby's Wakeup Behavior**
Okay, so I'm trying to get to the bottom of something related to Ruby's performance, likely at a low level. The initial thought is to zero in on `rb_wakeups`. That function, or more precisely instances of it in the Ruby core source, is a pretty good indicator of where the runtime is being nudged to wake up threads. This smells like a synchronization issue or perhaps contention within the interpreter. I'll start by checking the git log to see where this function has been modified or introduced. Those changes will give me context around its purpose and usage. Maybe there are some clues there about potential bottlenecks, unexpected behavior, or even planned optimizations related to thread management.
|
| 32/2 |
2026/08/04 02:38 |
tool |
git-log |
10m
Error:
Args: map[CodeRegexp:rb_wakeups]
Results:
|
| 33/2 |
2026/08/04 02:48 |
llm |
expert |
0m
Model:
Tokens:
input: 28135
output: 36
thoughts: 28
Thoughts: Okay, here's my interpretation of that thought process, as if it were my own:
**Refining the Probe Point Specification**
Right, let's zero in on exactly *where* in the kernel we're interested in the wakeups related to the ring buffer. The general probe point `rb_wakeups` is useful, but it could potentially trigger from many places. I need to narrow that down. Let's specify the file; that should dramatically reduce the noise. Ah, yes, setting `PathPrefix: "kernel/trace/ring_buffer.c"` for that `rb_wakeups` probe point should be perfect. That'll give me a much cleaner, more relevant data set to work with and I should be able to get what I need in a more focused manner.
|
| 34/2 |
2026/08/04 02:48 |
tool |
git-log |
0m
Args: map[CodeRegexp:rb_wakeups PathPrefix:kernel/trace/ring_buffer.c]
Results: map[Output:2e67fabd8b77 ring-buffer: Introduce ring-buffer remotes
13292494379f tracing: Make struct ring_buffer less ambiguous
d90fd77402d3 ring-buffer: Reorganize function locations
15693458c4bc tracing/ring-buffer: Move poll wake ups into ring buffer code
]
|
| 35/2 |
2026/08/04 02:48 |
llm |
expert |
0m
Model:
Tokens:
input: 28470
output: 26
thoughts: 18
Thoughts: Okay, here's my interpretation of what's going through my mind when I encounter this prompt, tailored for an expert audience:
**Analyzing the Commit Context**
Alright, show me commit `15693458c4bc`. My immediate instinct is to contextualize this. I'm going to quickly recall the typical workflow I use when presented with a commit hash. I need to understand *why* this commit is relevant. Is it a bug fix I'm tracking? A feature implementation I need to review? Part of a broader refactoring effort?
Based on the hash alone, I have to assume the requestor either knows or can readily provide the context of the commit. I'm going to expect, in other words, that the actual meaning and significance of the commit will be readily apparent in the following discussion.
I'm mentally running through the probable methods I'll use to actually *look* at the code. Are we talking `git show`, a full `git diff`, a visual diff tool, or something more sophisticated depending on the project setup? And of course, I'm thinking about the repository. Is this a code I'm familiar with, or something entirely new? My approach will be very different depending on the project's complexity and my own familiarity.
This is all about efficient knowledge acquisition: understanding the change, its impact, and its potential implications within the system, *quickly*. It's about getting the *gist* of the commit so I can address whatever follow up might need to be taken.
|
| 36/2 |
2026/08/04 02:48 |
tool |
git-show |
0m
Args:
Results: map[Output:commit 15693458c4bc0693fd63a50d60f35b628fcf4e29
Author: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
Date: Thu Feb 28 19:59:17 2013 -0500
tracing/ring-buffer: Move poll wake ups into ring buffer code
Move the logic to wake up on ring buffer data into the ring buffer
code itself. This simplifies the tracing code a lot and also has the
added benefit that waiters on one of the instance buffers can be woken
only when data is added to that instance instead of data added to
any instance.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
diff --git a/include/linux/ring_buffer.h b/include/linux/ring_buffer.h
index 1342e69542f3..d69cf637a15a 100644
--- a/include/linux/ring_buffer.h
+++ b/include/linux/ring_buffer.h
@@ -4,6 +4,7 @@
#include <linux/kmemcheck.h>
#include <linux/mm.h>
#include <linux/seq_file.h>
+#include <linux/poll.h>
struct ring_buffer;
struct ring_buffer_iter;
@@ -96,6 +97,11 @@ __ring_buffer_alloc(unsigned long size, unsigned flags, struct lock_class_key *k
__ring_buffer_alloc((size), (flags), &__key); \
})
+void ring_buffer_wait(struct ring_buffer *buffer, int cpu);
+int ring_buffer_poll_wait(struct ring_buffer *buffer, int cpu,
+ struct file *filp, poll_table *poll_table);
+
+
#define RING_BUFFER_ALL_CPUS -1
void ring_buffer_free(struct ring_buffer *buffer);
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 7244acde77b0..56b6ea32d2e7 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -8,6 +8,7 @@
#include <linux/trace_clock.h>
#include <linux/trace_seq.h>
#include <linux/spinlock.h>
+#include <linux/irq_work.h>
#include <linux/debugfs.h>
#include <linux/uaccess.h>
#include <linux/hardirq.h>
@@ -442,6 +443,12 @@ int ring_buffer_print_page_header(struct trace_seq *s)
return ret;
}
+struct rb_irq_work {
+ struct irq_work work;
+ wait_queue_head_t waiters;
+ bool waiters_pending;
+};
+
/*
* head_page == tail_page && head == tail then buffer is empty.
*/
@@ -476,6 +483,8 @@ struct ring_buffer_per_cpu {
struct list_head new_pages; /* new pages to add */
struct work_struct update_pages_work;
struct completion update_done;
+
+ struct rb_irq_work irq_work;
};
struct ring_buffer {
@@ -495,6 +504,8 @@ struct ring_buffer {
struct notifier_block cpu_notify;
#endif
u64 (*clock)(void);
+
+ struct rb_irq_work irq_work;
};
struct ring_buffer_iter {
@@ -506,6 +517,118 @@ struct ring_buffer_iter {
u64 read_stamp;
};
+/*
+ * rb_wake_up_waiters - wake up tasks waiting for ring buffer input
+ *
+ * Schedules a delayed work to wake up any task that is blocked on the
+ * ring buffer waiters queue.
+ */
+static void rb_wake_up_waiters(struct irq_work *work)
+{
+ struct rb_irq_work *rbwork = container_of(work, struct rb_irq_work, work);
+
+ wake_up_all(&rbwork->waiters);
+}
+
+/**
+ * ring_buffer_wait - wait for input to the ring buffer
+ * @buffer: buffer to wait on
+ * @cpu: the cpu buffer to wait on
+ *
+ * If @cpu == RING_BUFFER_ALL_CPUS then the task will wake up as soon
+ * as data is added to any of the @buffer's cpu buffers. Otherwise
+ * it will wait for data to be added to a specific cpu buffer.
+ */
+void ring_buffer_wait(struct ring_buffer *buffer, int cpu)
+{
+ struct ring_buffer_per_cpu *cpu_buffer;
+ DEFINE_WAIT(wait);
+ struct rb_irq_work *work;
+
+ /*
+ * Depending on what the caller is waiting for, either any
+ * data in any cpu buffer, or a specific buffer, put the
+ * caller on the appropriate wait queue.
+ */
+ if (cpu == RING_BUFFER_ALL_CPUS)
+ work = &buffer->irq_work;
+ else {
+ cpu_buffer = buffer->buffers[cpu];
+ work = &cpu_buffer->irq_work;
+ }
+
+
+ prepare_to_wait(&work->waiters, &wait, TASK_INTERRUPTIBLE);
+
+ /*
+ * The events can happen in critical sections where
+ * checking a work queue can cause deadlocks.
+ * After adding a task to the queue, this flag is set
+ * only to notify events to try to wake up the queue
+ * using irq_work.
+ *
+ * We don't clear it even if the buffer is no longer
+ * empty. The flag only causes the next event to run
+ * irq_work to do the work queue wake up. The worse
+ * that can happen if we race with !trace_empty() is that
+ * an event will cause an irq_work to try to wake up
+ * an empty queue.
+ *
+ * There's no reason to protect this flag either, as
+ * the work queue and irq_work logic will do the necessary
+ * synchronization for the wake ups. The only thing
+ * that is necessary is that the wake up happens after
+ * a task has been queued. It's OK for spurious wake ups.
+ */
+ work->waiters_pending = true;
+
+ if ((cpu == RING_BUFFER_ALL_CPUS && ring_buffer_empty(buffer)) ||
+ (cpu != RING_BUFFER_ALL_CPUS && ring_buffer_empty_cpu(buffer, cpu)))
+ schedule();
+
+ finish_wait(&work->waiters, &wait);
+}
+
+/**
+ * ring_buffer_poll_wait - poll on buffer input
+ * @buffer: buffer to wait on
+ * @cpu: the cpu buffer to wait on
+ * @filp: the file descriptor
+ * @poll_table: The poll descriptor
+ *
+ * If @cpu == RING_BUFFER_ALL_CPUS then the task will wake up as soon
+ * as data is added to any of the @buffer's cpu buffers. Otherwise
+ * it will wait for data to be added to a specific cpu buffer.
+ *
+ * Returns POLLIN | POLLRDNORM if data exists in the buffers,
+ * zero otherwise.
+ */
+int ring_buffer_poll_wait(struct ring_buffer *buffer, int cpu,
+ struct file *filp, poll_table *poll_table)
+{
+ struct ring_buffer_per_cpu *cpu_buffer;
+ struct rb_irq_work *work;
+
+ if ((cpu == RING_BUFFER_ALL_CPUS && !ring_buffer_empty(buffer)) ||
+ (cpu != RING_BUFFER_ALL_CPUS && !ring_buffer_empty_cpu(buffer, cpu)))
+ return POLLIN | POLLRDNORM;
+
+ if (cpu == RING_BUFFER_ALL_CPUS)
+ work = &buffer->irq_work;
+ else {
+ cpu_buffer = buffer->buffers[cpu];
+ work = &cpu_buffer->irq_work;
+ }
+
+ work->waiters_pending = true;
+ poll_wait(filp, &work->waiters, poll_table);
+
+ if ((cpu == RING_BUFFER_ALL_CPUS && !ring_buffer_empty(buffer)) ||
+ (cpu != RING_BUFFER_ALL_CPUS && !ring_buffer_empty_cpu(buffer, cpu)))
+ return POLLIN | POLLRDNORM;
+ return 0;
+}
+
/* buffer may be either ring_buffer or ring_buffer_per_cpu */
#define RB_WARN_ON(b, cond) \
({ \
@@ -1061,6 +1184,7 @@ rb_allocate_cpu_buffer(struct ring_buffer *buffer, int nr_pages, int cpu)
cpu_buffer->lock = (arch_spinlock_t)__ARCH_SPIN_LOCK_UNLOCKED;
INIT_WORK(&cpu_buffer->update_pages_work, update_pages_handler);
init_completion(&cpu_buffer->update_done);
+ init_irq_work(&cpu_buffer->irq_work.work, rb_wake_up_waiters);
bpage = kzalloc_node(ALIGN(sizeof(*bpage), cache_line_size()),
GFP_KERNEL, cpu_to_node(cpu));
@@ -1156,6 +1280,8 @@ struct ring_buffer *__ring_buffer_alloc(unsigned long size, unsigned flags,
buffer->clock = trace_clock_local;
buffer->reader_lock_key = key;
+ init_irq_work(&buffer->irq_work.work, rb_wake_up_waiters);
+
/* need at least two pages */
if (nr_pages < 2)
nr_pages = 2;
@@ -2610,6 +2736,22 @@ static void rb_commit(struct ring_buffer_per_cpu *cpu_buffer,
rb_end_commit(cpu_buffer);
}
+static __always_inline void
+rb_wakeups(struct ring_buffer *buffer, struct ring_buffer_per_cpu *cpu_buffer)
+{
+ if (buffer->irq_work.waiters_pending) {
+ buffer->irq_work.waiters_pending = false;
+ /* irq_work_queue() supplies it's own memory barriers */
+ irq_work_queue(&buffer->irq_work.work);
+ }
+
+ if (cpu_buffer->irq_work.waiters_pending) {
+ cpu_buffer->irq_work.waiters_pending = false;
+ /* irq_work_queue() supplies it's own memory barriers */
+ irq_work_queue(&cpu_buffer->irq_work.work);
+ }
+}
+
/**
* ring_buffer_unlock_commit - commit a reserved
* @buffer: The buffer to commit to
@@ -2629,6 +2771,8 @@ int ring_buffer_unlock_commit(struct ring_buffer *buffer,
rb_commit(cpu_buffer, event);
+ rb_wakeups(buffer, cpu_buffer);
+
trace_recursive_unlock();
preempt_enable_notrace();
@@ -2801,6 +2945,8 @@ int ring_buffer_write(struct ring_buffer *buffer,
rb_commit(cpu_buffer, event);
+ rb_wakeups(buffer, cpu_buffer);
+
ret = 0;
out:
preempt_enable_notrace();
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 3ec146c96df4..b5b25b6575a9 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -19,7 +19,6 @@
#include <linux/seq_file.h>
#include <linux/notifier.h>
#include <linux/irqflags.h>
-#include <linux/irq_work.h>
#include <linux/debugfs.h>
#include <linux/pagemap.h>
#include <linux/hardirq.h>
@@ -86,14 +85,6 @@ static int dummy_set_flag(u32 old_flags, u32 bit, int set)
*/
static DEFINE_PER_CPU(bool, trace_cmdline_save);
-/*
- * When a reader is waiting for data, then this variable is
- * set to true.
- */
-static bool trace_wakeup_needed;
-
-static struct irq_work trace_work_wakeup;
-
/*
* Kill all tracing for good (never come back).
* It is initialized to 1 but will turn to zero if the initialization
@@ -334,28 +325,12 @@ static inline void trace_access_lock_init(void)
#endif
-/* trace_wait is a waitqueue for tasks blocked on trace_poll */
-static DECLARE_WAIT_QUEUE_HEAD(trace_wait);
-
/* trace_flags holds trace_options default values */
unsigned long trace_flags = TRACE_ITER_PRINT_PARENT | TRACE_ITER_PRINTK |
TRACE_ITER_ANNOTATE | TRACE_ITER_CONTEXT_INFO | TRACE_ITER_SLEEP_TIME |
TRACE_ITER_GRAPH_TIME | TRACE_ITER_RECORD_CMD | TRACE_ITER_OVERWRITE |
TRACE_ITER_IRQ_INFO | TRACE_ITER_MARKERS;
-/**
- * trace_wake_up - wake up tasks waiting for trace input
- *
- * Schedules a delayed work to wake up any task that is blocked on the
- * trace_wait queue. These is used with trace_poll for tasks polling the
- * trace.
- */
-static void trace_wake_up(struct irq_work *work)
-{
- wake_up_all(&trace_wait);
-
-}
-
/**
* tracing_on - enable tracing buffers
*
@@ -763,36 +738,11 @@ update_max_tr_single(struct trace_array *tr, struct task_struct *tsk, int cpu)
static void default_wait_pipe(struct trace_iterator *iter)
{
- DEFINE_WAIT(wait);
-
- prepare_to_wait(&trace_wait, &wait, TASK_INTERRUPTIBLE);
-
- /*
- * The events can happen in critical sections where
- * checking a work queue can cause deadlocks.
- * After adding a task to the queue, this flag is set
- * only to notify events to try to wake up the queue
- * using irq_work.
- *
- * We don't clear it even if the buffer is no longer
- * empty. The flag only causes the next event to run
- * irq_work to do the work queue wake up. The worse
- * that can happen if we race with !trace_empty() is that
- * an event will cause an irq_work to try to wake up
- * an empty queue.
- *
- * There's no reason to protect this flag either, as
- * the work queue and irq_work logic will do the necessary
- * synchronization for the wake ups. The only thing
- * that is necessary is that the wake up happens after
- * a task has been queued. It's OK for spurious wake ups.
- */
- trace_wakeup_needed = true;
-
- if (trace_empty(iter))
- schedule();
+ /* Iterators are static, they should be filled or empty */
+ if (trace_buffer_iter(iter, iter->cpu_file))
+ return;
- finish_wait(&trace_wait, &wait);
+ ring_buffer_wait(iter->tr->buffer, iter->cpu_file);
}
/**
@@ -1262,11 +1212,6 @@ void
__buffer_unlock_commit(struct ring_buffer *buffer, struct ring_buffer_event *event)
{
__this_cpu_write(trace_cmdline_save, true);
- if (trace_wakeup_needed) {
- trace_wakeup_needed = false;
- /* irq_work_queue() supplies it's own memory barriers */
- irq_work_queue(&trace_work_wakeup);
- }
ring_buffer_unlock_commit(buffer, event);
}
@@ -3557,21 +3502,18 @@ static int tracing_release_pipe(struct inode *inode, struct file *file)
static unsigned int
trace_poll(struct trace_iterator *iter, struct file *filp, poll_table *poll_table)
{
- if (trace_flags & TRACE_ITER_BLOCK) {
+ /* Iterators are static, they should be filled or empty */
+ if (trace_buffer_iter(iter, iter->cpu_file))
+ return POLLIN | POLLRDNORM;
+
+ if (trace_flags & TRACE_ITER_BLOCK)
/*
* Always select as readable when in blocking mode
*/
return POLLIN | POLLRDNORM;
- } else {
- if (!trace_empty(iter))
- return POLLIN | POLLRDNORM;
- trace_wakeup_needed = true;
- poll_wait(filp, &trace_wait, poll_table);
- if (!trace_empty(iter))
- return POLLIN | POLLRDNORM;
-
- return 0;
- }
+ else
+ return ring_buffer_poll_wait(iter->tr->buffer, iter->cpu_file,
+ filp, poll_table);
}
static unsigned int
@@ -5701,7 +5643,6 @@ __init static int tracer_alloc_buffers(void)
#endif
trace_init_cmdlines();
- init_irq_work(&trace_work_wakeup, trace_wake_up);
register_tracer(&nop_trace);
]
|
| 37/2 |
2026/08/04 02:48 |
llm |
expert |
0m
Model:
Tokens:
input: 33559
output: 15
thoughts: 7536
Thoughts: **Compiler Optimization and Lost Wakeups in Ring Buffer Logic**
Right, let's break this down. The code in `ring_buffer_wait` and `rb_wakeups` uses a "hint" flag called `waiters_pending` to signal potential waiters needing a wakeup. The goal is to optimize wakeups, avoiding a full work queue check unless necessary. My initial thought was: is there a compiler optimization risk here? Let's trace it. The core of the problem lies in the following sequence: a reader (a waiting thread) sets `waiters_pending = true`. A writer then checks this flag and, if true, attempts a wakeup. If the writer sees `waiters_pending = false`, it skips the wakeup, optimizing performance.
Now, if a stale read (reading an outdated value) occurs where the writer reads `false` *after* the reader set the flag to `true`, a race condition arises. It's safe if data *written* to the ring buffer by the writer happens *before* the reader's check for the ring buffer's data. If we *miss* a wakeup, that is the problem.
This race is *potentially* exploitable. The comment in `ring_buffer_wait` is correct about synchronization *after* the `rb_watermark_hit()` call. However, a malicious compiler can *optimize* away the conditional check.
The classification guidelines are very helpful here: "Logic is sound" *but* requires annotations because compiler optimizations can break the algorithm. This is a crucial distinction. The logic *is* sound at a high level. However, if the compiler can transform `if (waiters_pending) { waiters_pending = false; ... }` into an unconditional `waiters_pending = false;`, we can have a lost wakeup. The compiler *is allowed* to do this because the standard C does not explicitly prevent the compiler from assuming no concurrent accesses.
Consider a scenario: Writer 1 sets `waiters_pending = false`. Reader sets `waiters_pending = true`. The compiler, in the writer's code, then reads the flag, sees *before* that it was false, unconditionally writes *false* to the variable, *overwriting* the reader's `true` with `false`, and skipping the wakeup. Later, another event fills the ring buffer, which should wake up the waiting thread. Because the flag was overwritten, and the compiler skips the check, the reader is now stuck! It's an optimization that breaks the algorithm's correctness.
The fix is straightforward, but it *is* required: explicitly prevent the compiler from making this optimization by wrapping the read and write operations with `READ_ONCE()` and `WRITE_ONCE()`. This forces the compiler to respect the data race and make sure the data is consistent at the read and write. These are the tools we have available. Let's make the changes and make sure this data race is handled.
The code in `rb_wakeups`, `rb_wait_cond`, and `ring_buffer_poll_wait`, and other places accessing the variables must be updated. This is not a structural change; it's a simple, targeted annotation.
|
| 38/2 |
2026/08/04 02:48 |
tool |
set-results |
0m
Args:
Results:
|
| 39/2 |
2026/08/04 02:48 |
llm |
expert |
0m
Model:
Tokens:
input: 41801
output: 1305
thoughts: 325
Thoughts: **Final Response Synthesis**
Okay, perfect, I'm ready to finalize this. Let's get this articulated clearly. I'll structure it like so:
1. **Race Summary:** The core issue revolves around a race condition between `ring_buffer_unlock_commit` and `ring_buffer_unlock_commit` specifically on the `buffer->irq_work.waiters_pending` variable.
2. **Synchronization Context:** I need to explicitly state that the function `ring_buffer_unlock_commit` can be invoked without a shared lock in certain trace scenarios, which means multiple CPUs could concurrently be reading and writing to `waiters_pending` in a completely unsynchronized fashion. This concurrent access is the heart of the problem.
3. **Final Classification:** This is definitely **HARMFUL**. This is a critical issue that *demands* immediate attention. The impact on correctness is undeniable.
4. **Failure Reasoning:**
* I'll break down the intended use of `waiters_pending`. It's essentially a crucial optimization, a fast path, designed to short-circuit the call to `irq_work_queue` when no waiting threads are present.
* The absence of proper memory barrier annotations (like `READ_ONCE` and `WRITE_ONCE`) is what exposes the vulnerability. The compiler, without these hints, is allowed to perform optimizations on the conditional write within `rb_wakeups`, specifically store-hoisting. This transformation causes the conditional write to be done every time.
* To make the problem clear, a detailed execution trace with two columns will be necessary. This will clearly illustrate how this optimization leads to a lost wakeup and ultimately results in a permanent hang or livelock condition for the reader.
5. **Recommended Fix:** The fix is clear: all accesses to `waiters_pending` and `full_waiters_pending` *must* be wrapped in `READ_ONCE()` and `WRITE_ONCE()` to ensure proper memory ordering and prevent the compiler from making unsafe optimizations.
|
| 40/1 |
2026/08/04 02:49 |
action |
format-explanation |
0m
Results: map[Explanation:### Race Summary
`ring_buffer_unlock_commit` vs `ring_buffer_unlock_commit` on
`buffer->irq_work.waiters_pending` (and similarly on
`cpu_buffer->irq_work.waiters_pending` and `full_waiters_pending`).
### Synchronization Context
The ring buffer commit path (`ring_buffer_unlock_commit`) is designed to be
highly concurrent and lockless. When multiple CPUs concurrently write events to
the ring buffer, they concurrently execute `rb_wakeups()` to check if there are
any pending readers/waiters to wake up. Because these checks are performed
without mutual exclusion, concurrent reads and writes to `waiters_pending`
occur.
---
### Final Classification
**HARMFUL** (Marking Required for Correctness)
---
### Failure Reasoning
The `waiters_pending` flag is a fast-path flag used to notify writers that a
reader has gone to sleep and needs to be woken up via `irq_work`.
While the algorithm is logically sound and tolerant of spurious wakeups, the
lack of data-race annotations (`READ_ONCE()` / `WRITE_ONCE()`) allows the
compiler to perform optimizations that break the algorithm. Specifically, the
compiler is permitted to optimize the conditional write in `rb_wakeups()`:
```c
if (buffer->irq_work.waiters_pending) {
buffer->irq_work.waiters_pending = false;
rb_irq_work_queue(&buffer->irq_work);
}
```
into an unconditional write to avoid a branch (store hoisting / branch
elimination):
```c
bool temp = buffer->irq_work.waiters_pending;
buffer->irq_work.waiters_pending = false; // Unconditional write!
if (temp) {
rb_irq_work_queue(&buffer->irq_work);
}
```
If this optimization is performed, a concurrent reader setting `waiters_pending
= true` can have its write silently overwritten and lost, leading to a permanent
hang (lost wakeup).
#### Failure Sequence (Lost Wakeup / Hang)
```text
CPU 0 (Writer 1) CPU 1 (Reader)
// Writer 1 commits an event
// and enters rb_wakeups()
// Compiler-optimized rb_wakeups():
// 1. Read waiters_pending into register
reg = buffer->irq_work.waiters_pending // reg = false
// Reader enters ring_buffer_wait()
// and sets waiters_pending to true
buffer->irq_work.waiters_pending = true
// 2. Unconditionally write false
buffer->irq_work.waiters_pending = false
// (This overwrites Reader's 'true'!)
// Reader checks for data
rb_watermark_hit() // returns false
// Reader goes to sleep
schedule()
// 3. Check register and skip queueing
if (reg) // false, so skip rb_irq_work_queue
```
Subsequently, when Writer 2 commits another event (which now hits the reader's
watermark), it enters `rb_wakeups()`, reads `waiters_pending` as `false` (since
it was overwritten by Writer 1), and does not queue the wakeup. The reader
sleeps indefinitely.
---
### Recommended Fix
To prevent the compiler from performing unsafe optimizations such as store
hoisting or merging, all accesses to `waiters_pending` and
`full_waiters_pending` must be annotated with `READ_ONCE()` and `WRITE_ONCE()`.
For example, in `rb_wakeups()`:
```c
static __always_inline void
rb_wakeups(struct trace_buffer *buffer, struct ring_buffer_per_cpu *cpu_buffer)
{
if (READ_ONCE(buffer->irq_work.waiters_pending)) {
WRITE_ONCE(buffer->irq_work.waiters_pending, false);
/* irq_work_queue() supplies its own memory barriers */
rb_irq_work_queue(&buffer->irq_work);
}
if (READ_ONCE(cpu_buffer->irq_work.waiters_pending)) {
WRITE_ONCE(cpu_buffer->irq_work.waiters_pending, false);
/* irq_work_queue() supplies its own memory barriers */
rb_irq_work_queue(&cpu_buffer->irq_work);
}
if (cpu_buffer->last_pages_touch ==
local_read(&cpu_buffer->pages_touched))
return;
if (cpu_buffer->reader_page == cpu_buffer->commit_page)
return;
if (!READ_ONCE(cpu_buffer->irq_work.full_waiters_pending))
return;
cpu_buffer->last_pages_touch = local_read(&cpu_buffer->pages_touched);
if (!full_hit(buffer, cpu_buffer->cpu, cpu_buffer->shortest_full))
return;
cpu_buffer->irq_work.wakeup_full = true;
WRITE_ONCE(cpu_buffer->irq_work.full_waiters_pending, false);
/* irq_work_queue() supplies its own memory barriers */
rb_irq_work_queue(&cpu_buffer->irq_work);
}
```
And similarly, use `WRITE_ONCE()` when setting these flags to `true` in
`rb_wait_cond()` and `ring_buffer_poll_wait()`, and `READ_ONCE()` /
`WRITE_ONCE()` in `rb_wake_up_waiters()`.]
|