| Message ID | 20210930223821.11025-1-beaub@linux.microsoft.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Commit | 4f67cca70c0f615e9cfe6ac42244f3416ec60877 |
| Headers | show |
| Series | synth_events: Do not block other dyn_event systems during create | expand |
Hi Beau, On Thu, 30 Sep 2021 15:38:21 -0700 Beau Belgrave <beaub@linux.microsoft.com> wrote: > synth_events is returning -EINVAL if the dyn_event create command does > not contain ' \t'. This prevents other systems from getting called back. > synth_events needs to return -ECANCELED in these cases when the command > is not targeting the synth_event system. Thanks for clean up the synthetic event! This looks good to me. Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org> So now are you trying to reuse synth event for user event? Then I think you need to register a new dyn_event ops so that histogram will not submit the event. BTW, how do you filter an event written by a user process? Will you add an array of event id for the file data structure? Thank you, > > Signed-off-by: Beau Belgrave <beaub@linux.microsoft.com> > --- > kernel/trace/trace_events_synth.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c > index 9315fc03e303..08b7ea639cea 100644 > --- a/kernel/trace/trace_events_synth.c > +++ b/kernel/trace/trace_events_synth.c > @@ -2051,6 +2051,13 @@ static int create_synth_event(const char *raw_command) > > last_cmd_set(raw_command); > > + name = raw_command; > + > + /* Don't try to process if not our system */ > + if (name[0] != 's' || name[1] != ':') > + return -ECANCELED; > + name += 2; > + > p = strpbrk(raw_command, " \t"); > if (!p) { > synth_err(SYNTH_ERR_INVALID_CMD, 0); > @@ -2059,12 +2066,6 @@ static int create_synth_event(const char *raw_command) > > fields = skip_spaces(p); > > - name = raw_command; > - > - if (name[0] != 's' || name[1] != ':') > - return -ECANCELED; > - name += 2; > - > /* This interface accepts group name prefix */ > if (strchr(name, '/')) { > len = str_has_prefix(name, SYNTH_SYSTEM "/"); > -- > 2.17.1 >
> Thanks for clean up the synthetic event! > This looks good to me. > > Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org> No problem, thanks for the review! > So now are you trying to reuse synth event for user event? > Then I think you need to register a new dyn_event ops so > that histogram will not submit the event. A user events patch will get sent out soon, I'm finalizing some testing. User events will register it's own dyn_event ops and allow users the option to create, delete and view status of user events via the dynamic_events tracefs file (in addition to the user mode IOCTL/ABI). As probes attach to the user events the status of this is reflected in dynamic_events, which makes it easy for admins to see why one is busy, etc. It also makes it easy to verifying the system is working as expected with just a terminal. > BTW, how do you filter an event written by a user process? > Will you add an array of event id for the file data structure? The filtering would happen at the trace_event level, the only filtering at the user event level is if a probe has been enabled on the underlying trace_event or not. That is done via the shared page bits being cleared or set. Bits are updated as probe un/registrations occur. Users can advertise field values, offsets, etc as well as the print_fmt in the newer ABI/patch based on the feedback from LPC2021. These land in the trace_event fields and are viewable via tracefs like any other evnet. I hope that is enough to light up the history feature of trace_event. I am not familiar with it as much as you all. Thanks, -Beau
diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c index 9315fc03e303..08b7ea639cea 100644 --- a/kernel/trace/trace_events_synth.c +++ b/kernel/trace/trace_events_synth.c @@ -2051,6 +2051,13 @@ static int create_synth_event(const char *raw_command) last_cmd_set(raw_command); + name = raw_command; + + /* Don't try to process if not our system */ + if (name[0] != 's' || name[1] != ':') + return -ECANCELED; + name += 2; + p = strpbrk(raw_command, " \t"); if (!p) { synth_err(SYNTH_ERR_INVALID_CMD, 0); @@ -2059,12 +2066,6 @@ static int create_synth_event(const char *raw_command) fields = skip_spaces(p); - name = raw_command; - - if (name[0] != 's' || name[1] != ':') - return -ECANCELED; - name += 2; - /* This interface accepts group name prefix */ if (strchr(name, '/')) { len = str_has_prefix(name, SYNTH_SYSTEM "/");
synth_events is returning -EINVAL if the dyn_event create command does not contain ' \t'. This prevents other systems from getting called back. synth_events needs to return -ECANCELED in these cases when the command is not targeting the synth_event system. Signed-off-by: Beau Belgrave <beaub@linux.microsoft.com> --- kernel/trace/trace_events_synth.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)