On Wed, Jul 15, 2026 at 07:02:01AM +0000, Jackie Liu wrote:
> 2026年7月15日 14:44, "Vincent Donnefort" <[email protected]
> mailto:[email protected]?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 <[email protected]>
> > >
> > > 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 <[email protected]>
> > >
> > I have sent a related improvement for that here [1]
> >
> > [1]
> > https://lore.kernel.org/all/[email protected]/
>
> 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
> > >
> >
