[PATCH] tracing: Propagate errors from remote event bulk updates

[PATCH] tracing: Propagate errors from remote event bulk updates

[Jackie Liu]

From: Jackie Liu <liuyun01@kylinos.cn>

remote_events_dir_enable_write() ignores the return value from
trace_remote_enable_event(). If a remote rejects an event state change,
the write therefore reports success even though the affected event remains
in its previous state.

Keep trying all events, but retain and return the first error. This matches
__ftrace_set_clr_event_nolock(), which permits partial updates while
notifying userspace when an operation fails.

Fixes: 775cb093bc50 ("tracing: Add events/ root files to trace remotes")
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
---
 kernel/trace/trace_remote.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c
index 0f6ef5c36d84..5212e685f0a3 100644
--- a/kernel/trace/trace_remote.c
+++ b/kernel/trace/trace_remote.c
@@ -1149,11 +1149,22 @@ static ssize_t remote_events_dir_enable_write(struct file *filp, const char __us
 	guard(mutex)(&remote->lock);
 
 	for (i = 0; i < remote->nr_events; i++) {
+		int eret;
 		struct remote_event *evt = &remote->events[i];
 
-		trace_remote_enable_event(remote, evt, enable);
+		eret = trace_remote_enable_event(remote, evt, enable);
+		/*
+		 * Save the first error and return that. Some events
+		 * may still have been enabled, but let the user
+		 * know that something went wrong.
+		 */
+		if (!ret && eret)
+			ret = eret;
 	}
 
+	if (ret)
+		return ret;
+
 	return count;
 }
 
-- 
2.54.0

Re: [PATCH] tracing: Propagate errors from remote event bulk updates

[Vincent Donnefort]

On Wed, Jul 15, 2026 at 02:04:48PM +0800, Jackie Liu wrote:
> From: Jackie Liu <liuyun01@kylinos.cn>
> 
> remote_events_dir_enable_write() ignores the return value from
> trace_remote_enable_event(). If a remote rejects an event state change,
> the write therefore reports success even though the affected event remains
> in its previous state.
> 
> Keep trying all events, but retain and return the first error. This matches
> __ftrace_set_clr_event_nolock(), which permits partial updates while
> notifying userspace when an operation fails.
> 
> Fixes: 775cb093bc50 ("tracing: Add events/ root files to trace remotes")
> Assisted-by: Codex:gpt-5.6-sol
> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>

I have sent a related improvement for that here [1]

[1] https://lore.kernel.org/all/20260605163825.1762953-3-vdonnefort@google.com/


> ---
>  kernel/trace/trace_remote.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c
> index 0f6ef5c36d84..5212e685f0a3 100644
> --- a/kernel/trace/trace_remote.c
> +++ b/kernel/trace/trace_remote.c
> @@ -1149,11 +1149,22 @@ static ssize_t remote_events_dir_enable_write(struct file *filp, const char __us
>  	guard(mutex)(&remote->lock);
>  
>  	for (i = 0; i < remote->nr_events; i++) {
> +		int eret;
>  		struct remote_event *evt = &remote->events[i];
>  
> -		trace_remote_enable_event(remote, evt, enable);
> +		eret = trace_remote_enable_event(remote, evt, enable);
> +		/*
> +		 * Save the first error and return that. Some events
> +		 * may still have been enabled, but let the user
> +		 * know that something went wrong.
> +		 */
> +		if (!ret && eret)
> +			ret = eret;
>  	}
>  
> +	if (ret)
> +		return ret;
> +
>  	return count;
>  }
>  
> -- 
> 2.54.0
> 
> 

Re: [PATCH] tracing: Propagate errors from remote event bulk updates

[Jackie Liu]

2026年7月15日 14:44, "Vincent Donnefort" <vdonnefort@google.com mailto:vdonnefort@google.com?to=%22Vincent%20Donnefort%22%20%3Cvdonnefort%40google.com%3E > 写到:


> 
> On Wed, Jul 15, 2026 at 02:04:48PM +0800, Jackie Liu wrote:
> 
> > 
> > From: Jackie Liu <liuyun01@kylinos.cn>
> >  
> >  remote_events_dir_enable_write() ignores the return value from
> >  trace_remote_enable_event(). If a remote rejects an event state change,
> >  the write therefore reports success even though the affected event remains
> >  in its previous state.
> >  
> >  Keep trying all events, but retain and return the first error. This matches
> >  __ftrace_set_clr_event_nolock(), which permits partial updates while
> >  notifying userspace when an operation fails.
> >  
> >  Fixes: 775cb093bc50 ("tracing: Add events/ root files to trace remotes")
> >  Assisted-by: Codex:gpt-5.6-sol
> >  Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> > 
> I have sent a related improvement for that here [1]
> 
> [1] https://lore.kernel.org/all/20260605163825.1762953-3-vdonnefort@google.com/

Thanks for the pointer.

I checked the v2 series. Patch 02/18 handles registration failure cleanup,
and patch 03/18 changes the boolean parsing in
remote_events_dir_enable_write(), but the loop still ignores errors from
trace_remote_enable_event().

Would you prefer this fix to be folded into your series, or should I resend
a v2 rebased on top of it?

-- 
Jackie Liu

> 
> > 
> > ---
> >  kernel/trace/trace_remote.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >  
> >  diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c
> >  index 0f6ef5c36d84..5212e685f0a3 100644
> >  --- a/kernel/trace/trace_remote.c
> >  +++ b/kernel/trace/trace_remote.c
> >  @@ -1149,11 +1149,22 @@ static ssize_t remote_events_dir_enable_write(struct file *filp, const char __us
> >  guard(mutex)(&remote->lock);
> >  
> >  for (i = 0; i < remote->nr_events; i++) {
> >  + int eret;
> >  struct remote_event *evt = &remote->events[i];
> >  
> >  - trace_remote_enable_event(remote, evt, enable);
> >  + eret = trace_remote_enable_event(remote, evt, enable);
> >  + /*
> >  + * Save the first error and return that. Some events
> >  + * may still have been enabled, but let the user
> >  + * know that something went wrong.
> >  + */
> >  + if (!ret && eret)
> >  + ret = eret;
> >  }
> >  
> >  + if (ret)
> >  + return ret;
> >  +
> >  return count;
> >  }
> >  
> >  -- 
> >  2.54.0
> >
>

Re: [PATCH] tracing: Propagate errors from remote event bulk updates

[Vincent Donnefort]

On Wed, Jul 15, 2026 at 07:02:01AM +0000, Jackie Liu wrote:
> 2026年7月15日 14:44, "Vincent Donnefort" <vdonnefort@google.com mailto:vdonnefort@google.com?to=%22Vincent%20Donnefort%22%20%3Cvdonnefort%40google.com%3E > 写到:
> 
> 
> > 
> > On Wed, Jul 15, 2026 at 02:04:48PM +0800, Jackie Liu wrote:
> > 
> > > 
> > > From: Jackie Liu <liuyun01@kylinos.cn>
> > >  
> > >  remote_events_dir_enable_write() ignores the return value from
> > >  trace_remote_enable_event(). If a remote rejects an event state change,
> > >  the write therefore reports success even though the affected event remains
> > >  in its previous state.
> > >  
> > >  Keep trying all events, but retain and return the first error. This matches
> > >  __ftrace_set_clr_event_nolock(), which permits partial updates while
> > >  notifying userspace when an operation fails.
> > >  
> > >  Fixes: 775cb093bc50 ("tracing: Add events/ root files to trace remotes")
> > >  Assisted-by: Codex:gpt-5.6-sol
> > >  Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> > > 
> > I have sent a related improvement for that here [1]
> > 
> > [1] https://lore.kernel.org/all/20260605163825.1762953-3-vdonnefort@google.com/
> 
> Thanks for the pointer.
> 
> I checked the v2 series. Patch 02/18 handles registration failure cleanup,
> and patch 03/18 changes the boolean parsing in
> remote_events_dir_enable_write(), but the loop still ignores errors from
> trace_remote_enable_event().
> 
> Would you prefer this fix to be folded into your series, or should I resend
> a v2 rebased on top of it?

Ha yes my bad. Then no, no need to send it separately. 

> 
> -- 
> Jackie Liu
> 
> > 
> > > 
> > > ---
> > >  kernel/trace/trace_remote.c | 13 ++++++++++++-
> > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > >  
> > >  diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c
> > >  index 0f6ef5c36d84..5212e685f0a3 100644
> > >  --- a/kernel/trace/trace_remote.c
> > >  +++ b/kernel/trace/trace_remote.c
> > >  @@ -1149,11 +1149,22 @@ static ssize_t remote_events_dir_enable_write(struct file *filp, const char __us
> > >  guard(mutex)(&remote->lock);
> > >  
> > >  for (i = 0; i < remote->nr_events; i++) {
> > >  + int eret;
> > >  struct remote_event *evt = &remote->events[i];

Could you declare ert after? I have tried to use the reverse christmas tree
everywhere in the trace remote code.

> > >  
> > >  - trace_remote_enable_event(remote, evt, enable);
> > >  + eret = trace_remote_enable_event(remote, evt, enable);
> > >  + /*
> > >  + * Save the first error and return that. Some events
> > >  + * may still have been enabled, but let the user
> > >  + * know that something went wrong.
> > >  + */
> > >  + if (!ret && eret)
> > >  + ret = eret;
> > >  }
> > >  
> > >  + if (ret)
> > >  + return ret;
> > >  +
> > >  return count;
> > >  }
> > >  
> > >  -- 
> > >  2.54.0
> > >
> >