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

On 2026/8/8 22:58, Steven Rostedt 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

quoted

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

commit b61ca8359fdea427c80a410f6d4052c68af5eae3
Author: Tengda Wu [off-list ref]

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.

quoted

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

[ ... ]

quoted

-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?

Indeed, work_on_cpu() cannot handle these scenarios properly.

I just tried handling the return value and noticed that there is already
a log message inside update_max_tr_single() that reports the
ring_buffer_swap_cpu() error.
(I had previously overlooked this error handling and didn't notice it.)

So, it seems that no additional handling is necessary. I will keep the
current implementation unchanged.

Thank you for your accurate catch and thoughtful suggestions.

Best regards,
Tengda

quoted

quoted

+	local_irq_enable();
+	return 0;
 }

[ ... ]

quoted

@@ -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.

quoted

quoted

 		}
 		tracing_disarm_snapshot(tr);
 		break;

-- Steve