lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ