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

["Jackie Liu" <liu.yun@linux.dev>]

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