[PATCH v2] tracing: Keep copying markers when one instance is stopped

[PATCH v2] tracing: Keep copying markers when one instance is stopped

[Jaewon Jung]

When several instances enable copy_trace_marker, stopping one instance
can prevent the remaining buffers from receiving markers.

Continue copying text and raw markers even if one copy fails. Return the
last write's result, keeping -ENODEV when no destinations are selected.

Fixes: 7b382efd5e8a ("tracing: Allow the top level trace_marker to write into another instances")
Signed-off-by: Jaewon Jung <jaewon8868@gmail.com>
---
All writes are now attempted regardless of errors.
The last write's result is returned. Would this approach work?

Changes in v2:
- Drop the stable Cc.
- Do not save earlier copy errors. Return the last write's result.
- Rebase onto trace/for-next.

Tested on x86_64 QEMU:
- Before: Copying stops at the first error, which is returned.
  Later live buffers receive no markers.
- After: All buffers are tried, and the last write's result is returned.
- Existing text/raw marker and instance selftests: 6 PASS.

 kernel/trace/trace.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 08614af8fad1..0b66c55d378b 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -6330,8 +6330,6 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
 		guard(rcu)();
 		list_for_each_entry_rcu(tr, &marker_copies, marker_list) {
 			written = write_marker_to_buffer(tr, buf, cnt, ip);
-			if (written < 0)
-				break;
 		}
 	} else {
 		written = write_marker_to_buffer(tr, buf, cnt, ip);
@@ -6409,8 +6407,6 @@ tracing_mark_raw_write(struct file *filp, const char __user *ubuf,
 		guard(rcu)();
 		list_for_each_entry_rcu(tr, &marker_copies, marker_list) {
 			written = write_raw_marker_to_buffer(tr, buf, cnt);
-			if (written < 0)
-				break;
 		}
 	} else {
 		written = write_raw_marker_to_buffer(tr, buf, cnt);

base-commit: d7ff48d92a0f28bc0612da47c054efbc28c19562
-- 
2.43.0

Re: [PATCH v2] tracing: Keep copying markers when one instance is stopped

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] tracing: successful marker write overwritten by tail error in marker_copies
--

commit 2345280f3b3625fa6e36116fc50947e792b25719
Author: Jaewon Jung <jaewon8868@gmail.com>

tracing: Keep copying markers when one instance is stopped

This commit removes the early break on write errors in tracing_mark_write()
and tracing_mark_raw_write() when iterating over marker_copies so that
stopped instances do not prevent remaining instances from receiving markers.

> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 08614af8fad1c..0b66c55d378b1 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -6330,8 +6330,6 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
>  		guard(rcu)();
>  		list_for_each_entry_rcu(tr, &marker_copies, marker_list) {
>  			written = write_marker_to_buffer(tr, buf, cnt, ip);
> -			if (written < 0)
> -				break;
>  		}
>  	} else {
>  		written = write_marker_to_buffer(tr, buf, cnt, ip);
> @@ -6409,8 +6407,6 @@ tracing_mark_raw_write(struct file *filp, const char __user *ubuf,
>  		guard(rcu)();
>  		list_for_each_entry_rcu(tr, &marker_copies, marker_list) {
>  			written = write_raw_marker_to_buffer(tr, buf, cnt);
> -			if (written < 0)
> -				break;

[Severity: Medium]
As reported by Steven Rostedt on v1 [1], should errors from stopped
instances in marker_copies be prevented from overwriting a successful write
return value?

When instances enable copy_trace_marker, update_marker_trace() adds each
instance to the head of marker_copies:

kernel/trace/trace.c:update_marker_trace() {
	if (enabled) {
		if (tr->trace_flags & TRACE_ITER(COPY_MARKER))
			return false;

		list_add_rcu(&tr->marker_list, &marker_copies);
		tr->trace_flags |= TRACE_ITER(COPY_MARKER);
		return true;
	}

This leaves earlier-enabled instances and global_trace at the tail of
marker_copies.

Because tracing_mark_write() and tracing_mark_raw_write() unconditionally
assign written on every iteration of marker_copies, if an earlier instance
in the list is active and receives the marker (setting written = cnt), but
the last instance in marker_copies has tracing disabled,
write_marker_to_buffer() or write_raw_marker_to_buffer() will return -EBADF
on that tail instance. Won't that overwrite written and cause the syscall
to return -EBADF to userspace even though the marker was successfully
written to the active instance(s)?

[1] https://lore.kernel.org/r/20261102091751.2ee7c4d3@gandalf.local.home

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103005219.3236380-1-jaewon8868@gmail.com?part=1

Re: [PATCH v2] tracing: Keep copying markers when one instance is stopped

[Steven Rostedt]

On Tue, 03 Nov 2026 01:05:27 +0000
sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] tracing: successful marker write overwritten by tail error in marker_copies
> --
> 
> commit 2345280f3b3625fa6e36116fc50947e792b25719
> Author: Jaewon Jung <jaewon8868@gmail.com>
> 
> tracing: Keep copying markers when one instance is stopped
> 
> This commit removes the early break on write errors in tracing_mark_write()
> and tracing_mark_raw_write() when iterating over marker_copies so that
> stopped instances do not prevent remaining instances from receiving markers.
> 
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 08614af8fad1c..0b66c55d378b1 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -6330,8 +6330,6 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
> >  		guard(rcu)();
> >  		list_for_each_entry_rcu(tr, &marker_copies, marker_list) {
> >  			written = write_marker_to_buffer(tr, buf, cnt, ip);
> > -			if (written < 0)
> > -				break;
> >  		}
> >  	} else {
> >  		written = write_marker_to_buffer(tr, buf, cnt, ip);
> > @@ -6409,8 +6407,6 @@ tracing_mark_raw_write(struct file *filp, const char __user *ubuf,
> >  		guard(rcu)();
> >  		list_for_each_entry_rcu(tr, &marker_copies, marker_list) {
> >  			written = write_raw_marker_to_buffer(tr, buf, cnt);
> > -			if (written < 0)
> > -				break;  
> 
> [Severity: Medium]
> As reported by Steven Rostedt on v1 [1], should errors from stopped
> instances in marker_copies be prevented from overwriting a successful write
> return value?
> 
> When instances enable copy_trace_marker, update_marker_trace() adds each
> instance to the head of marker_copies:
> 
> kernel/trace/trace.c:update_marker_trace() {
> 	if (enabled) {
> 		if (tr->trace_flags & TRACE_ITER(COPY_MARKER))
> 			return false;
> 
> 		list_add_rcu(&tr->marker_list, &marker_copies);
> 		tr->trace_flags |= TRACE_ITER(COPY_MARKER);
> 		return true;
> 	}
> 
> This leaves earlier-enabled instances and global_trace at the tail of
> marker_copies.
> 
> Because tracing_mark_write() and tracing_mark_raw_write() unconditionally
> assign written on every iteration of marker_copies, if an earlier instance
> in the list is active and receives the marker (setting written = cnt), but
> the last instance in marker_copies has tracing disabled,
> write_marker_to_buffer() or write_raw_marker_to_buffer() will return -EBADF
> on that tail instance. Won't that overwrite written and cause the syscall
> to return -EBADF to userspace even though the marker was successfully
> written to the active instance(s)?
> 

We only want to record the the output of the global array, so we should have it be:

	if (tr == &global_trace) {
		guard(rcu)();
		list_for_each_entry_rcu(tr, &marker_copies, marker_list) {
			ssize_t w = write_marker_to_buffer(tr, buf, cnt, ip);
			if (tr == @global_trace)
				written = w;
		}

-- Steve

Re: [PATCH v2] tracing: Keep copying markers when one instance is stopped

[Jaewon Jung]

The copy_trace_marker documentation says:

> If no instance has this option set, then a write will error with
> the errno of ENODEV.

With global copying disabled and another instance enabled, both versions
record the marker in that instance, but the proposed change returns
-ENODEV.

This seems inconsistent with the documentation. Should we return the
last write's result, as in v2?

Re: [PATCH v2] tracing: Keep copying markers when one instance is stopped

[Steven Rostedt]

On Wed,  4 Nov 2026 15:35:38 +0900
Jaewon Jung <jaewon8868@gmail.com> wrote:

> The copy_trace_marker documentation says:
> 
> > If no instance has this option set, then a write will error with
> > the errno of ENODEV.  
> 
> With global copying disabled and another instance enabled, both versions
> record the marker in that instance, but the proposed change returns
> -ENODEV.

Hmm, that's a side effect I guess I didn't expect :-p

> 
> This seems inconsistent with the documentation. Should we return the
> last write's result, as in v2?

Perhaps just return the most successful write:

	if (tr == &global_trace) {
		guard(rcu)();
		list_for_each_entry_rcu(tr, &marker_copies, marker_list) {
			ssize_t w = write_marker_to_buffer(tr, buf, cnt, ip);
			if (w >= 0 && w > written)
				written = w;
		}

So it produces the output that wrote the most, and an error if non worked.

In reality, if the write_marker_to_buffer() succeeds then it should always
have the same return value.

Thanks,

-- Steve