[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <55AD0C5B.2010002@hitachi.com>
Date: Mon, 20 Jul 2015 23:57:31 +0900
From: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
To: Tom Zanussi <tom.zanussi@...ux.intel.com>, rostedt@...dmis.org
CC: daniel.wagner@...-carit.de, namhyung@...nel.org,
josh@...htriplett.org, andi@...stfloor.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v9 21/22] tracing: Add enable_hist/disable_hist triggers
On 2015/07/17 2:22, Tom Zanussi wrote:
> Similar to enable_event/disable_event triggers, these triggers enable
> and disable the aggregation of events into maps rather than enabling
> and disabling their writing into the trace buffer.
>
> They can be used to automatically start and stop hist triggers based
> on a matching filter condition.
>
> If there's a paused hist trigger on system:event, the following would
> start it when the filter condition was hit:
>
> # echo enable_hist:system:event [ if filter] > event/trigger
>
> And the following would disable a running system:event hist trigger:
>
> # echo disable_hist:system:event [ if filter] > event/trigger
>
> See Documentation/trace/events.txt for real examples.
Hmm, do we really need this? Since we've already had multiple instances,
if someone wants to make histogram separated from event logger, he/she can
make another instance for that, and disable/enable event itself.
I'm considering if we accept this methods, we'll need to accept another
enable/disable triggers for each action too in the future.
Thank you,
>
> Signed-off-by: Tom Zanussi <tom.zanussi@...ux.intel.com>
> ---
> include/linux/trace_events.h | 1 +
> kernel/trace/trace.c | 11 ++++
> kernel/trace/trace.h | 32 ++++++++++
> kernel/trace/trace_events_hist.c | 115 ++++++++++++++++++++++++++++++++++++
> kernel/trace/trace_events_trigger.c | 71 ++++++++++++----------
> 5 files changed, 199 insertions(+), 31 deletions(-)
>
> diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
> index 0faf48b..0f3ffdd 100644
> --- a/include/linux/trace_events.h
> +++ b/include/linux/trace_events.h
> @@ -411,6 +411,7 @@ enum event_trigger_type {
> ETT_STACKTRACE = (1 << 2),
> ETT_EVENT_ENABLE = (1 << 3),
> ETT_EVENT_HIST = (1 << 4),
> + ETT_HIST_ENABLE = (1 << 5),
> };
>
> extern int filter_match_preds(struct event_filter *filter, void *rec);
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 16c64a2..c581750 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -3761,6 +3761,10 @@ static const char readme_msg[] =
> "\t trigger: traceon, traceoff\n"
> "\t enable_event:<system>:<event>\n"
> "\t disable_event:<system>:<event>\n"
> +#ifdef CONFIG_HIST_TRIGGERS
> + "\t enable_hist:<system>:<event>\n"
> + "\t disable_hist:<system>:<event>\n"
> +#endif
> #ifdef CONFIG_STACKTRACE
> "\t\t stacktrace\n"
> #endif
> @@ -3836,6 +3840,13 @@ static const char readme_msg[] =
> "\t restart a paused hist trigger.\n\n"
> "\t The 'clear' param will clear the contents of a running hist\n"
> "\t trigger and leave its current paused/active state.\n\n"
> + "\t The enable_hist and disable_hist triggers can be used to\n"
> + "\t have one event conditionally start and stop another event's\n"
> + "\t already-attached hist trigger. Any number of enable_hist\n"
> + "\t and disable_hist triggers can be attached to a given event,\n"
> + "\t allowing that event to kick off and stop aggregations on\n"
> + "\t a host of other events. See Documentation/trace/events.txt\n"
> + "\t for examples.\n"
> #endif
> ;
>
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index e6cb781..5e2e3b0 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -1102,8 +1102,10 @@ extern const struct file_operations event_hist_fops;
>
> #ifdef CONFIG_HIST_TRIGGERS
> extern int register_trigger_hist_cmd(void);
> +extern int register_trigger_hist_enable_disable_cmds(void);
> #else
> static inline int register_trigger_hist_cmd(void) { return 0; }
> +static inline int register_trigger_hist_enable_disable_cmds(void) { return 0; }
> #endif
>
> extern int register_trigger_cmds(void);
> @@ -1121,6 +1123,34 @@ struct event_trigger_data {
> struct list_head list;
> };
>
> +/* Avoid typos */
> +#define ENABLE_EVENT_STR "enable_event"
> +#define DISABLE_EVENT_STR "disable_event"
> +#define ENABLE_HIST_STR "enable_hist"
> +#define DISABLE_HIST_STR "disable_hist"
> +
> +struct enable_trigger_data {
> + struct trace_event_file *file;
> + bool enable;
> + bool hist;
> +};
> +
> +extern int event_enable_trigger_print(struct seq_file *m,
> + struct event_trigger_ops *ops,
> + struct event_trigger_data *data);
> +extern void event_enable_trigger_free(struct event_trigger_ops *ops,
> + struct event_trigger_data *data);
> +extern int event_enable_trigger_func(struct event_command *cmd_ops,
> + struct trace_event_file *file,
> + char *glob, char *cmd, char *param);
> +extern int event_enable_register_trigger(char *glob,
> + struct event_trigger_ops *ops,
> + struct event_trigger_data *data,
> + struct trace_event_file *file);
> +extern void event_enable_unregister_trigger(char *glob,
> + struct event_trigger_ops *ops,
> + struct event_trigger_data *test,
> + struct trace_event_file *file);
> extern void trigger_data_free(struct event_trigger_data *data);
> extern int event_trigger_init(struct event_trigger_ops *ops,
> struct event_trigger_data *data);
> @@ -1134,6 +1164,8 @@ extern int set_trigger_filter(char *filter_str,
> struct event_trigger_data *trigger_data,
> struct trace_event_file *file);
> extern int register_event_command(struct event_command *cmd);
> +extern int unregister_event_command(struct event_command *cmd);
> +extern int register_trigger_hist_enable_disable_cmds(void);
>
> /**
> * struct event_trigger_ops - callbacks for trace event triggers
> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index 4ba7645..6a43611 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -1345,3 +1345,118 @@ __init int register_trigger_hist_cmd(void)
>
> return ret;
> }
> +
> +static void
> +hist_enable_trigger(struct event_trigger_data *data, void *rec)
> +{
> + struct enable_trigger_data *enable_data = data->private_data;
> + struct event_trigger_data *test;
> +
> + list_for_each_entry_rcu(test, &enable_data->file->triggers, list) {
> + if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
> + if (enable_data->enable)
> + test->paused = false;
> + else
> + test->paused = true;
> + break;
> + }
> + }
> +}
> +
> +static void
> +hist_enable_count_trigger(struct event_trigger_data *data, void *rec)
> +{
> + if (!data->count)
> + return;
> +
> + if (data->count != -1)
> + (data->count)--;
> +
> + hist_enable_trigger(data, rec);
> +}
> +
> +static struct event_trigger_ops hist_enable_trigger_ops = {
> + .func = hist_enable_trigger,
> + .print = event_enable_trigger_print,
> + .init = event_trigger_init,
> + .free = event_enable_trigger_free,
> +};
> +
> +static struct event_trigger_ops hist_enable_count_trigger_ops = {
> + .func = hist_enable_count_trigger,
> + .print = event_enable_trigger_print,
> + .init = event_trigger_init,
> + .free = event_enable_trigger_free,
> +};
> +
> +static struct event_trigger_ops hist_disable_trigger_ops = {
> + .func = hist_enable_trigger,
> + .print = event_enable_trigger_print,
> + .init = event_trigger_init,
> + .free = event_enable_trigger_free,
> +};
> +
> +static struct event_trigger_ops hist_disable_count_trigger_ops = {
> + .func = hist_enable_count_trigger,
> + .print = event_enable_trigger_print,
> + .init = event_trigger_init,
> + .free = event_enable_trigger_free,
> +};
> +
> +static struct event_trigger_ops *
> +hist_enable_get_trigger_ops(char *cmd, char *param)
> +{
> + struct event_trigger_ops *ops;
> + bool enable;
> +
> + enable = (strcmp(cmd, ENABLE_HIST_STR) == 0);
> +
> + if (enable)
> + ops = param ? &hist_enable_count_trigger_ops :
> + &hist_enable_trigger_ops;
> + else
> + ops = param ? &hist_disable_count_trigger_ops :
> + &hist_disable_trigger_ops;
> +
> + return ops;
> +}
> +
> +static struct event_command trigger_hist_enable_cmd = {
> + .name = ENABLE_HIST_STR,
> + .trigger_type = ETT_HIST_ENABLE,
> + .func = event_enable_trigger_func,
> + .reg = event_enable_register_trigger,
> + .unreg = event_enable_unregister_trigger,
> + .get_trigger_ops = hist_enable_get_trigger_ops,
> + .set_filter = set_trigger_filter,
> +};
> +
> +static struct event_command trigger_hist_disable_cmd = {
> + .name = DISABLE_HIST_STR,
> + .trigger_type = ETT_HIST_ENABLE,
> + .func = event_enable_trigger_func,
> + .reg = event_enable_register_trigger,
> + .unreg = event_enable_unregister_trigger,
> + .get_trigger_ops = hist_enable_get_trigger_ops,
> + .set_filter = set_trigger_filter,
> +};
> +
> +static __init void unregister_trigger_hist_enable_disable_cmds(void)
> +{
> + unregister_event_command(&trigger_hist_enable_cmd);
> + unregister_event_command(&trigger_hist_disable_cmd);
> +}
> +
> +__init int register_trigger_hist_enable_disable_cmds(void)
> +{
> + int ret;
> +
> + ret = register_event_command(&trigger_hist_enable_cmd);
> + if (WARN_ON(ret < 0))
> + return ret;
> + ret = register_event_command(&trigger_hist_disable_cmd);
> + if (WARN_ON(ret < 0))
> + unregister_trigger_hist_enable_disable_cmds();
> +
> + return ret;
> +}
> diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> index e80f30b..9490d8f 100644
> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -338,7 +338,7 @@ __init int register_event_command(struct event_command *cmd)
> * Currently we only unregister event commands from __init, so mark
> * this __init too.
> */
> -static __init int unregister_event_command(struct event_command *cmd)
> +__init int unregister_event_command(struct event_command *cmd)
> {
> struct event_command *p, *n;
> int ret = -ENODEV;
> @@ -1052,15 +1052,6 @@ static __init void unregister_trigger_traceon_traceoff_cmds(void)
> unregister_event_command(&trigger_traceoff_cmd);
> }
>
> -/* Avoid typos */
> -#define ENABLE_EVENT_STR "enable_event"
> -#define DISABLE_EVENT_STR "disable_event"
> -
> -struct enable_trigger_data {
> - struct trace_event_file *file;
> - bool enable;
> -};
> -
> static void
> event_enable_trigger(struct event_trigger_data *data, void *rec)
> {
> @@ -1090,14 +1081,16 @@ event_enable_count_trigger(struct event_trigger_data *data, void *rec)
> event_enable_trigger(data, rec);
> }
>
> -static int
> -event_enable_trigger_print(struct seq_file *m, struct event_trigger_ops *ops,
> - struct event_trigger_data *data)
> +int event_enable_trigger_print(struct seq_file *m,
> + struct event_trigger_ops *ops,
> + struct event_trigger_data *data)
> {
> struct enable_trigger_data *enable_data = data->private_data;
>
> seq_printf(m, "%s:%s:%s",
> - enable_data->enable ? ENABLE_EVENT_STR : DISABLE_EVENT_STR,
> + enable_data->hist ?
> + (enable_data->enable ? ENABLE_HIST_STR : DISABLE_HIST_STR) :
> + (enable_data->enable ? ENABLE_EVENT_STR : DISABLE_EVENT_STR),
> enable_data->file->event_call->class->system,
> trace_event_name(enable_data->file->event_call));
>
> @@ -1114,9 +1107,8 @@ event_enable_trigger_print(struct seq_file *m, struct event_trigger_ops *ops,
> return 0;
> }
>
> -static void
> -event_enable_trigger_free(struct event_trigger_ops *ops,
> - struct event_trigger_data *data)
> +void event_enable_trigger_free(struct event_trigger_ops *ops,
> + struct event_trigger_data *data)
> {
> struct enable_trigger_data *enable_data = data->private_data;
>
> @@ -1161,10 +1153,9 @@ static struct event_trigger_ops event_disable_count_trigger_ops = {
> .free = event_enable_trigger_free,
> };
>
> -static int
> -event_enable_trigger_func(struct event_command *cmd_ops,
> - struct trace_event_file *file,
> - char *glob, char *cmd, char *param)
> +int event_enable_trigger_func(struct event_command *cmd_ops,
> + struct trace_event_file *file,
> + char *glob, char *cmd, char *param)
> {
> struct trace_event_file *event_enable_file;
> struct enable_trigger_data *enable_data;
> @@ -1173,6 +1164,7 @@ event_enable_trigger_func(struct event_command *cmd_ops,
> struct trace_array *tr = file->tr;
> const char *system;
> const char *event;
> + bool hist = false;
> char *trigger;
> char *number;
> bool enable;
> @@ -1197,8 +1189,15 @@ event_enable_trigger_func(struct event_command *cmd_ops,
> if (!event_enable_file)
> goto out;
>
> - enable = strcmp(cmd, ENABLE_EVENT_STR) == 0;
> +#ifdef CONFIG_HIST_TRIGGERS
> + hist = ((strcmp(cmd, ENABLE_HIST_STR) == 0) ||
> + (strcmp(cmd, DISABLE_HIST_STR) == 0));
>
> + enable = ((strcmp(cmd, ENABLE_EVENT_STR) == 0) ||
> + (strcmp(cmd, ENABLE_HIST_STR) == 0));
> +#else
> + enable = strcmp(cmd, ENABLE_EVENT_STR) == 0;
> +#endif
> trigger_ops = cmd_ops->get_trigger_ops(cmd, trigger);
>
> ret = -ENOMEM;
> @@ -1218,6 +1217,7 @@ event_enable_trigger_func(struct event_command *cmd_ops,
> INIT_LIST_HEAD(&trigger_data->list);
> RCU_INIT_POINTER(trigger_data->filter, NULL);
>
> + enable_data->hist = hist;
> enable_data->enable = enable;
> enable_data->file = event_enable_file;
> trigger_data->private_data = enable_data;
> @@ -1295,10 +1295,10 @@ event_enable_trigger_func(struct event_command *cmd_ops,
> goto out;
> }
>
> -static int event_enable_register_trigger(char *glob,
> - struct event_trigger_ops *ops,
> - struct event_trigger_data *data,
> - struct trace_event_file *file)
> +int event_enable_register_trigger(char *glob,
> + struct event_trigger_ops *ops,
> + struct event_trigger_data *data,
> + struct trace_event_file *file)
> {
> struct enable_trigger_data *enable_data = data->private_data;
> struct enable_trigger_data *test_enable_data;
> @@ -1308,6 +1308,8 @@ static int event_enable_register_trigger(char *glob,
> list_for_each_entry_rcu(test, &file->triggers, list) {
> test_enable_data = test->private_data;
> if (test_enable_data &&
> + (test->cmd_ops->trigger_type ==
> + data->cmd_ops->trigger_type) &&
> (test_enable_data->file == enable_data->file)) {
> ret = -EEXIST;
> goto out;
> @@ -1333,10 +1335,10 @@ out:
> return ret;
> }
>
> -static void event_enable_unregister_trigger(char *glob,
> - struct event_trigger_ops *ops,
> - struct event_trigger_data *test,
> - struct trace_event_file *file)
> +void event_enable_unregister_trigger(char *glob,
> + struct event_trigger_ops *ops,
> + struct event_trigger_data *test,
> + struct trace_event_file *file)
> {
> struct enable_trigger_data *test_enable_data = test->private_data;
> struct enable_trigger_data *enable_data;
> @@ -1346,6 +1348,8 @@ static void event_enable_unregister_trigger(char *glob,
> list_for_each_entry_rcu(data, &file->triggers, list) {
> enable_data = data->private_data;
> if (enable_data &&
> + (data->cmd_ops->trigger_type ==
> + test->cmd_ops->trigger_type) &&
> (enable_data->file == test_enable_data->file)) {
> unregistered = true;
> list_del_rcu(&data->list);
> @@ -1365,8 +1369,12 @@ event_enable_get_trigger_ops(char *cmd, char *param)
> struct event_trigger_ops *ops;
> bool enable;
>
> +#ifdef CONFIG_HIST_TRIGGERS
> + enable = ((strcmp(cmd, ENABLE_EVENT_STR) == 0) ||
> + (strcmp(cmd, ENABLE_HIST_STR) == 0));
> +#else
> enable = strcmp(cmd, ENABLE_EVENT_STR) == 0;
> -
> +#endif
> if (enable)
> ops = param ? &event_enable_count_trigger_ops :
> &event_enable_trigger_ops;
> @@ -1437,6 +1445,7 @@ __init int register_trigger_cmds(void)
> register_trigger_snapshot_cmd();
> register_trigger_stacktrace_cmd();
> register_trigger_enable_disable_cmds();
> + register_trigger_hist_enable_disable_cmds();
> register_trigger_hist_cmd();
>
> return 0;
>
--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@...achi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists