Commit 3163f635b20e ("tracing: Fix race issue between cpu buffer write
and swap") fixed most of the race conditions between snapshot's
ring_buffer_swap_cpu and ring_buffer_lock_{reserve, commit}. It achieved
this by replacing the asynchronous swap with smp_call_function_single to
trigger an interrupt on the target CPU to handle the swap.
However, this interrupt can still break in at any point during
ring_buffer_lock_{reserve, commit}. Currently, ring_buffer_swap_cpu()
relies on a cooperative check where cpu_buffer->committing is combined
with a condition in rb_reserve_next_event() that verifies if
READ_ONCE(cpu_buffer->buffer) != buffer to ensure state consistency. In
principle, once the execution passes this conditional check, no issues
should arise as long as cpu_buffer->committing remains non-zero.
Unfortunately, there is a window where cpu_buffer->committing can drop
to zero, which subsequently triggers a warning during rb_commit:
ring_buffer_lock_reserve
cpu_buffer = buffer->buffers[cpu]; // cpu_buffer_a
rb_reserve_next_event
rb_start_commit // inc committing
if (unlikely(READ_ONCE(cpu_buffer->buffer) != buffer)) {...}
__rb_reserve_next
rb_move_tail
rb_end_commit(cpu_buffer); // dec committing => 0
/* smp_call interrupt hits here, successfully swaps! */
local_inc(&cpu_buffer->committing);
ring_buffer_unlock_commit
cpu_buffer = buffer->buffers[cpu]; // cpu_buffer_b
rb_commit
rb_end_commit
RB_WARN_ON(cpu_buffer, !local_read(&cpu_buffer->committing))
// triggers warning
Instead of maintaining fragile complexity to allow the swap to intervene at
arbitrary points, simply replace smp_call_function_single with work_on_cpu.
This ensures the swap operation runs in a process context on the target
CPU, guaranteeing it only executes after ring_buffer_lock_{reserve, commit}
has fully completed.
Fixes: 3163f635b20e ("tracing: Fix race issue between cpu buffer write and
swap")
Cc: [email protected]
Signed-off-by: Tengda Wu <[email protected]>
---
kernel/trace/trace_snapshot.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/trace_snapshot.c b/kernel/trace/trace_snapshot.c
index 07b43c9863a2..22323c97289b 100644
--- a/kernel/trace/trace_snapshot.c
+++ b/kernel/trace/trace_snapshot.c
@@ -625,9 +625,12 @@ static int tracing_snapshot_open(struct inode *inode,
struct file *file)
return ret;
}
-static void tracing_swap_cpu_buffer(void *tr)
+static long tracing_swap_cpu_buffer(void *tr)
{
+ local_irq_disable();
update_max_tr_single((struct trace_array *)tr, current,
smp_processor_id());
+ local_irq_enable();
+ return 0;
}
static ssize_t
@@ -689,8 +692,7 @@ tracing_snapshot_write(struct file *filp, const char __user
*ubuf, size_t cnt,
update_max_tr(tr, current, smp_processor_id(), NULL);
local_irq_enable();
} else {
- smp_call_function_single(iter->cpu_file,
tracing_swap_cpu_buffer,
- (void *)tr, 1);
+ work_on_cpu(iter->cpu_file, tracing_swap_cpu_buffer,
(void *)tr);
}
tracing_disarm_snapshot(tr);
break;
--
2.34.1
