On Fri, Jan 02, 2026 at 01:36:19PM -0500, Steven Rostedt wrote: > Nice. Perhaps we should do something similar for event triggers.
Hi Steve,
Thanks for the review; I’m glad you find the addition useful.
That is an excellent suggestion. Consolidating the active triggers
alongside the filters would certainly provide a more comprehensive
overview.
I shall incorporate a similar mechanism for event triggers and submit the
update as a formal patch series in the next revision.
> Enabled? Or just events with filters. I think this should just say:
>
> A list of events that have filters. This shows the system/event
> pair along with the filter that is attached to the event.
Acknowledged.
> This doesn't traverse the trace_array. The seq_file does the traversing.
> Just state that this is part of the seq_file output and shows events with
> filters.
Acknowledged.
> > + * Uses RCU to safely dereference the volatile filter pointer.
>
> This is internal to the function and should not be part of the kerneldoc.
Acknowledged.
> > + */
> > +static int t_show_filters(struct seq_file *m, void *v)
> > +{
> > + struct trace_event_file *file = v;
> > + struct trace_event_call *call = file->event_call;
> > + struct event_filter *filter;
> > +
> > + rcu_read_lock();
>
> Use:
>
> guard(rcu)();
>
> instead.
>
> > + filter = rcu_dereference(file->filter);
> > + if (filter && filter->filter_string) {
> > + seq_printf(m, "%s:%s\t%s\n",
> > + call->class->system,
> > + trace_event_name(call),
> > + filter->filter_string);
> > + }
> > + rcu_read_unlock();
>
> And remove the rcu_read_unlock().
>
> Actually, the function may be better by just doing:
>
> guard(rcu)();
> filter = rcu_dereference(file->filter);
> if (!filter || !filter->filter_string)
> return 0;
>
> seq_printf(m, "%s:%s\t%s\n", call->class->system,
> trace_event_name(call), filter->filter_string);
>
> return 0;
Agreed. Using guard(rcu)() is indeed much cleaner and aligns well with the
current preference for scoped-based resource management in the kernel.
Your proposed refactoring also helpfully reduces the indentation level,
which improves readability. I shall adopt this approach and incorporate it
into the next version of the patch.
> Why not just:
>
> return ftrace_event_open(inode, file, &show_show_event_filters_seq_ops);
>
> ?
Acknowledged.
Kind regards,
--
Aaron Tomlin
signature.asc
Description: PGP signature
