[PATCH v2 0/2] ring-buffer: Fix snapshot swap race and improve user experience

[PATCH v2 0/2] ring-buffer: Fix snapshot swap race and improve user experience

[Tengda Wu]

Patch 1 fixes a race condition in ring_buffer_swap_cpu() where the
committing counter check can be bypassed during a write operation,
leading to inconsistent buffer state and a RB_WARN_ON. The fix uses
current_context instead, which remains valid throughout the entire
write.

Patch 2 improves the snapshot user experience by using work_on_cpu()
instead of smp_call_function_single(), ensuring the swap only occurs
after any ongoing write completes and avoiding confusing -EBUSY errors.

Changelog:
v2: Add fix patch for ring_buffer_swap_cpu(). (Steven)
    Reorganize commit messages and Fixes tags.
v1: https://lore.kernel.org/all/20260730011912.2325121-1-wutengda@huaweicloud.com/

Tengda Wu (2):
  ring-buffer: Use current_context for safe per-CPU buffer swap
  tracing/snapshot: Use work_on_cpu to avoid -EBUSY during snapshot swap

 kernel/trace/ring_buffer.c    | 8 ++++----
 kernel/trace/trace_snapshot.c | 8 +++++---
 2 files changed, 9 insertions(+), 7 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] ring-buffer: Use current_context for safe per-CPU buffer swap

[Tengda Wu]

The ring_buffer_swap_cpu() function currently checks the per-CPU
committing counter to determine if a buffer is actively being written to
before performing the swap. However, there exists a race window where
this check can be bypassed:

    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
                    /* 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

The committing counter can temporarily drop to 0 during a single write
operation (within rb_move_tail), creating a window where swap can
succeed even though the write is still in progress. This leads to
inconsistent buffer state and triggers the RB_WARN_ON in rb_commit().

Replace the committing counter check with current_context checks, which
are set at the entry of ring_buffer_lock_reserve() and remain valid
throughout the entire write operation, providing a reliable indicator of
buffer busy state during swap.

Fixes: 4239c38fe0b3 ("ring-buffer: Process commits whenever moving to a new page.")
Cc: stable@vger.kernel.org
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
 kernel/trace/ring_buffer.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index e863eb0d10f7..28d6a3812da0 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -6850,7 +6850,7 @@ int ring_buffer_swap_cpu(struct trace_buffer *buffer_a,
 {
 	struct ring_buffer_per_cpu *cpu_buffer_a;
 	struct ring_buffer_per_cpu *cpu_buffer_b;
-	int ret = -EINVAL;
+	int ret = -EBUSY;
 
 	if (!cpumask_test_cpu(cpu, buffer_a->cpumask) ||
 	    !cpumask_test_cpu(cpu, buffer_b->cpumask))
@@ -6891,10 +6891,10 @@ int ring_buffer_swap_cpu(struct trace_buffer *buffer_a,
 	atomic_inc(&cpu_buffer_a->record_disabled);
 	atomic_inc(&cpu_buffer_b->record_disabled);
 
-	ret = -EBUSY;
-	if (local_read(&cpu_buffer_a->committing))
+	/* Do not swap if either buffer is in the process of writing */
+	if (cpu_buffer_a->current_context)
 		goto out_dec;
-	if (local_read(&cpu_buffer_b->committing))
+	if (cpu_buffer_b->current_context)
 		goto out_dec;
 
 	/*
-- 
2.34.1

[PATCH v2 2/2] tracing/snapshot: Use work_on_cpu to avoid -EBUSY during snapshot swap

[Tengda Wu]

The snapshot operation currently uses smp_call_function_single() to
invoke ring_buffer_swap_cpu() on the target CPU. Since interrupt can hit
at any point during the write path, this may return -EBUSY if the target
CPU is actively writing to the buffer, which could be confusing to users
who may not understand the internal state of the ring buffer.

Replace smp_call_function_single() with work_on_cpu() for the snapshot
operation. This ensures that the swap only occurs after the ongoing
write completes.

Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
 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

Re: [PATCH v2 2/2] tracing/snapshot: Use work_on_cpu to avoid -EBUSY during snapshot swap

[Steven Rostedt]

Sashiko brings up a good point. This will break the recording of what
task is being interrupted.

  https://sashiko.dev/#/patchset/20260803005640.2445666-2-wutengda%40huaweicloud.com


> --- Patch [2]: [PATCH v2 2/2] tracing/snapshot: Use work_on_cpu to avoid -EBUSY during snapshot swap ---
> commit b61ca8359fdea427c80a410f6d4052c68af5eae3
> Author: Tengda Wu <wutengda@huaweicloud.com>
> 
> tracing/snapshot: Use work_on_cpu to avoid -EBUSY during snapshot swap
> 
> This commit replaces smp_call_function_single() with work_on_cpu() in
> the snapshot write path to avoid -EBUSY errors when a target CPU is
> actively writing to the ring buffer.
> 
> > diff --git a/kernel/trace/trace_snapshot.c b/kernel/trace/trace_snapshot.c
> > --- a/kernel/trace/trace_snapshot.c
> > +++ b/kernel/trace/trace_snapshot.c
> 
> [ ... ]
> 
> > -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());
> 
> When calling update_max_tr_single() from a workqueue thread, does passing
> current result in the trace snapshot recording the kworker task instead of
> the interrupted task?
> 
> In the previous smp_call_function_single() implementation, current would
> represent the task executing on the target CPU, but now it will capture
> the kworker thread.

And thinking of one usecase, what if you had a high priroity task you
wanted to snashot? This wouldn't work to do that.

Now we can still keep the ipi and send data that holds the tr and
state. It waits for the ipi to finish anyway. Then it can check if the
swap happened or not, and could report a fail, or it could try again?

> 
> > +	local_irq_enable();
> > +	return 0;
> >  }
> 
> [ ... ]
> 
> > @@ -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);
> 
> Can this result in swapping the wrong CPU's trace ring buffer if
> iter->cpu_file is an offline CPU, or if the CPU goes offline concurrently?
> 
> Because cpus_read_lock() is not held, if the target CPU is offline, the
> workqueue subsystem may execute the work on a fallback online CPU.
> 
> When tracing_swap_cpu_buffer() then calls smp_processor_id(), it will
> evaluate to the fallback CPU's ID rather than the intended target CPU.
> 
> This could potentially modify an unrelated online CPU's trace buffer while
> leaving the requested offline CPU's buffer unchanged.

This too looks to be a valid concern.

> 
> >  		}
> >  		tracing_disarm_snapshot(tr);
> >  		break;

-- Steve

Re: [PATCH v2 2/2] tracing/snapshot: Use work_on_cpu to avoid -EBUSY during snapshot swap

[Steven Rostedt]

On Sat, 8 Aug 2026 10:58:55 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> Sashiko brings up a good point. This will break the recording of what
> task is being interrupted.
> 
>   https://sashiko.dev/#/patchset/20260803005640.2445666-2-wutengda%40huaweicloud.com

I'll apply your fist patch, so you can send new revisions of this patch
as a stand alone. But still start at v3.

-- Steve