Hi Steve, On Wed, 2025-11-26 at 15:24 -0500, Steven Rostedt wrote: > From: Steven Rostedt <[email protected]> > > Currently a trigger can only be added to individual events. Some triggers > (like stacktrace) can be useful to add as a bulk trigger for a set of > system events (like interrupt or scheduling). > > Add a trigger file to the system directories: > > /sys/kernel/tracing/events/*/trigger > > And allow stacktrace trigger to be enabled for all those events. > > Writing into the system/trigger file acts the same as writing into each of > the system event's trigger files individually. > > This also allows to remove a trigger from all events in a subsystem (even > if it's not a subsystem trigger!). >
This looks very useful! Just a couple comments below.. > Signed-off-by: Steven Rostedt (Google) <[email protected]> > --- > Changes since v2: > https://patch.msgid.link/[email protected] > > - Removed unneeded NULL initialization of tr (Masami Hiramatsu) > > > [ ... ] > +static ssize_t > +event_system_trigger_write(struct file *filp, const char __user *ubuf, > + size_t cnt, loff_t *ppos) > +{ > + struct trace_subsystem_dir *dir = filp->private_data; > + struct event_command *p; > + char *command, *next; > + char *buf __free(kfree) = get_user_buf(ubuf, cnt); nit: I think you could move this up a couple lines and keep the reverse Christmas tree. > + bool remove = false; > + bool found = false; > + ssize_t ret; > + > + if (!buf) > + return 0; > + > + if (IS_ERR(buf)) > + return PTR_ERR(buf); > + > + /* system triggers are not allowed to have counters */ > + if (strchr(buf, ':')) > + return -EINVAL; > + As mentioned by Masami, I think this would preclude the hist triggers from having this enabled. I'm guessing you didn't see the hist commands as one of the triggers that would be useful as system triggers, but there might be a couple cases where they could be if using the common fields e.g. echo hist:keys=common_pid.execname:vals=hitcount > events/syscalls/trigger or anywhere just echo hist:keys=common_stacktrace > trigger Just about everything else would error out, but these might be worth having. Maybe you could just change this check to look for :<number> at the end of the command? Thanks, Tom
