On Fri, 2016-02-19 at 17:35 -0500, Steven Rostedt wrote:
> On Fri, 19 Feb 2016 14:30:13 -0500
> Steven Rostedt <[email protected]> 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,
