Messages in this thread | | | Subject | Re: [PATCH v13 06/29] tracing: Add needs_rec flag to event triggers | From | Tom Zanussi <> | Date | Fri, 19 Feb 2016 17:31:11 -0600 |
| |
On Fri, 2016-02-19 at 17:35 -0500, Steven Rostedt wrote: > On Fri, 19 Feb 2016 14:30:13 -0500 > Steven Rostedt <rostedt@goodmis.org> wrote: > > > > @@ -1333,6 +1339,7 @@ struct event_command { > > > char *name; > > > enum event_trigger_type trigger_type; > > > bool post_trigger; > > > + bool needs_rec; > > > > Note, from what I understand, gcc sucks with bool in structures. Best > > thing to do here is to create a "int flags" field, and check the result > > with masks. > > > > You don't need to update this patch (I'm still working on the series), > > but a patch on top of these may be necessary. I could add the patch too. > > > > I've added this patch after patch this patch. What do you think? >
Looks great, thanks!
Do you want me to respin adding this and the fixes for the previous comments before continuing?
Tom
> -- Steve > > diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h > index 789e88ab5acd..f60c0444ef1e 100644 > --- a/kernel/trace/trace.h > +++ b/kernel/trace/trace.h > @@ -1272,30 +1272,7 @@ struct event_trigger_ops { > * values are defined by adding new values to the trigger_type > * enum in include/linux/trace_events.h. > * > - * @post_trigger: A flag that says whether or not this command needs > - * to have its action delayed until after the current event has > - * been closed. Some triggers need to avoid being invoked while > - * an event is currently in the process of being logged, since > - * the trigger may itself log data into the trace buffer. Thus > - * we make sure the current event is committed before invoking > - * those triggers. To do that, the trigger invocation is split > - * in two - the first part checks the filter using the current > - * trace record; if a command has the @post_trigger flag set, it > - * sets a bit for itself in the return value, otherwise it > - * directly invokes the trigger. Once all commands have been > - * either invoked or set their return flag, the current record is > - * either committed or discarded. At that point, if any commands > - * have deferred their triggers, those commands are finally > - * invoked following the close of the current event. In other > - * words, if the event_trigger_ops @func() probe implementation > - * itself logs to the trace buffer, this flag should be set, > - * otherwise it can be left unspecified. > - * > - * @needs_rec: A flag that says whether or not this command needs > - * access to the trace record in order to perform its function, > - * regardless of whether or not it has a filter associated with > - * it (filters make a trigger require access to the trace record > - * but are not always present). > + * @flags: See the enum event_command_flags below. > * > * All the methods below, except for @set_filter() and @unreg_all(), > * must be implemented. > @@ -1340,8 +1317,7 @@ struct event_command { > struct list_head list; > char *name; > enum event_trigger_type trigger_type; > - bool post_trigger; > - bool needs_rec; > + int flags; > int (*func)(struct event_command *cmd_ops, > struct trace_event_file *file, > char *glob, char *cmd, char *params); > @@ -1360,6 +1336,49 @@ struct event_command { > struct event_trigger_ops *(*get_trigger_ops)(char *cmd, char *param); > }; > > +/** > + * enum event_command_flags - flags for struct event_command > + * > + * @POST_TRIGGER: A flag that says whether or not this command needs > + * to have its action delayed until after the current event has > + * been closed. Some triggers need to avoid being invoked while > + * an event is currently in the process of being logged, since > + * the trigger may itself log data into the trace buffer. Thus > + * we make sure the current event is committed before invoking > + * those triggers. To do that, the trigger invocation is split > + * in two - the first part checks the filter using the current > + * trace record; if a command has the @post_trigger flag set, it > + * sets a bit for itself in the return value, otherwise it > + * directly invokes the trigger. Once all commands have been > + * either invoked or set their return flag, the current record is > + * either committed or discarded. At that point, if any commands > + * have deferred their triggers, those commands are finally > + * invoked following the close of the current event. In other > + * words, if the event_trigger_ops @func() probe implementation > + * itself logs to the trace buffer, this flag should be set, > + * otherwise it can be left unspecified. > + * > + * @NEEDS_REC: A flag that says whether or not this command needs > + * access to the trace record in order to perform its function, > + * regardless of whether or not it has a filter associated with > + * it (filters make a trigger require access to the trace record > + * but are not always present). > + */ > +enum event_command_flags { > + EVENT_CMD_FL_POST_TRIGGER = 1, > + EVENT_CMD_FL_NEEDS_REC = 2, > +}; > + > +static inline bool event_command_post_trigger(struct event_command *cmd_ops) > +{ > + return cmd_ops->flags & EVENT_CMD_FL_POST_TRIGGER; > +} > + > +static inline bool event_command_needs_rec(struct event_command *cmd_ops) > +{ > + return cmd_ops->flags & EVENT_CMD_FL_NEEDS_REC; > +} > + > extern int trace_event_enable_disable(struct trace_event_file *file, > int enable, int soft_disable); > extern int tracing_alloc_snapshot(void); > diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c > index cbb7ee531983..d67992f3bb0e 100644 > --- a/kernel/trace/trace_events_trigger.c > +++ b/kernel/trace/trace_events_trigger.c > @@ -81,7 +81,7 @@ event_triggers_call(struct trace_event_file *file, void *rec) > filter = rcu_dereference_sched(data->filter); > if (filter && !filter_match_preds(filter, rec)) > continue; > - if (data->cmd_ops->post_trigger) { > + if (event_command_post_trigger(data->cmd_ops)) { > tt |= data->cmd_ops->trigger_type; > continue; > } > @@ -506,8 +506,8 @@ void update_cond_flag(struct trace_event_file *file) > bool set_cond = false; > > list_for_each_entry_rcu(data, &file->triggers, list) { > - if (data->filter || data->cmd_ops->post_trigger || > - data->cmd_ops->needs_rec) { > + if (data->filter || event_command_post_trigger(data->cmd_ops) || > + event_command_needs_rec(data->cmd_ops)) { > set_cond = true; > break; > } > @@ -1035,7 +1035,7 @@ stacktrace_get_trigger_ops(char *cmd, char *param) > static struct event_command trigger_stacktrace_cmd = { > .name = "stacktrace", > .trigger_type = ETT_STACKTRACE, > - .post_trigger = true, > + .flags = EVENT_CMD_FL_POST_TRIGGER, > .func = event_trigger_callback, > .reg = register_trigger, > .unreg = unregister_trigger,
| |