| 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
| ||
|
Date: Wed, 22 Jul 2015 15:22:37 -0500
From: Tom Zanussi <tom.zanussi@...ux.intel.com>
To: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
Cc: rostedt@...dmis.org, daniel.wagner@...-carit.de,
namhyung@...nel.org, josh@...htriplett.org, andi@...stfloor.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v9 12/22] tracing: Add hist trigger support for pausing
and continuing a trace
Hi Masami,
On Wed, 2015-07-22 at 17:20 +0900, Masami Hiramatsu wrote:
> Hi Tom,
>
> On 2015/07/17 2:22, Tom Zanussi wrote:
> > Allow users to append 'pause' or 'continue' to an existing trigger in
> > order to have it paused or to have a paused trace continue.
> >
> > This expands the hist trigger syntax from this:
> > # echo hist:keys=xxx:vals=yyy:sort=zzz.descending \
> > [ if filter] > event/trigger
> >
> > to this:
> >
> > # echo hist:keys=xxx:vals=yyy:sort=zzz.descending:pause or cont \
> > [ if filter] > event/trigger
>
> Since the only one hist trigger can be set on one event, it seems
> that we don't need keys for pause/cont/clear (e.g. hist:pause is enough).
> Anyway, I've found an odd behavior.
Right, because currently there is only one hist trigger per event, the
key is ignored, which also accounts for the 'odd' behavior.
But rather than saying it's expected and document it, I think the
conclusion from the other comments are that we'll be allowing multiple
hist triggers per event, and in that case, they need to be uniquely
identifiable by both key and filter.
>
> [root@...alhost tracing]# echo 'hist:keys=parent_pid' > events/sched/sched_process_fork/trigger
> [root@...alhost tracing]# echo 'hist:keys=common_pid:pause' > events/sched/sched_process_fork/trigger
> [root@...alhost tracing]# cat events/sched/sched_process_fork/trigger
> hist:keys=parent_pid:vals=hitcount:sort=hitcount:size=2048 [paused]
>
> So, the second "pause" command can work with different keys.
> Moreover, I can remove it with different keys.
>
Right, this goes away once we have code that deals with multiple
histograms per event, which I'll go ahead and implement rather than
document the confusion...
Tom
> [root@...alhost tracing]# echo '!hist:keys=child_pid' > events/sched/sched_process_fork/trigger
> [root@...alhost tracing]# cat events/sched/sched_process_fork/trigger
> # Available triggers:
> # traceon traceoff snapshot stacktrace enable_event disable_event enable_hist disable_hist hist
>
> Thank you,
>
> >
> > Signed-off-by: Tom Zanussi <tom.zanussi@...ux.intel.com>
> > ---
> > kernel/trace/trace.c | 5 +++++
> > kernel/trace/trace_events_hist.c | 26 +++++++++++++++++++++++---
> > 2 files changed, 28 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 5dd1fc4..547bbc8 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -3791,6 +3791,7 @@ static const char readme_msg[] =
> > "\t [:values=<field1[,field2,...]]\n"
> > "\t [:sort=field1,field2,...]\n"
> > "\t [:size=#entries]\n"
> > + "\t [:pause][:continue]\n"
> > "\t [if <filter>]\n\n"
> > "\t When a matching event is hit, an entry is added to a hash\n"
> > "\t table using the key(s) and value(s) named. Keys and values\n"
> > @@ -3821,6 +3822,10 @@ static const char readme_msg[] =
> > "\t on. The default if unspecified is 'hitcount' and the.\n"
> > "\t default sort order is 'ascending'. To sort in the opposite\n"
> > "\t direction, append .descending' to the sort key.\n\n"
> > + "\t The 'pause' param can be used to pause an existing hist\n"
> > + "\t trigger or to start a hist trigger but not log any events\n"
> > + "\t until told to do so. 'continue' can be used to start or\n"
> > + "\t restart a paused hist trigger.\n\n"
> > #endif
> > ;
> >
> > diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> > index 6bf224f..3ae58e7 100644
> > --- a/kernel/trace/trace_events_hist.c
> > +++ b/kernel/trace/trace_events_hist.c
> > @@ -78,6 +78,8 @@ struct hist_trigger_attrs {
> > char *keys_str;
> > char *vals_str;
> > char *sort_key_str;
> > + bool pause;
> > + bool cont;
> > unsigned int map_bits;
> > };
> >
> > @@ -184,6 +186,11 @@ static struct hist_trigger_attrs *parse_hist_trigger_attrs(char *trigger_str)
> > attrs->vals_str = kstrdup(str, GFP_KERNEL);
> > else if (!strncmp(str, "sort", strlen("sort")))
> > attrs->sort_key_str = kstrdup(str, GFP_KERNEL);
> > + else if (!strncmp(str, "pause", strlen("pause")))
> > + attrs->pause = true;
> > + else if (!strncmp(str, "continue", strlen("continue")) ||
> > + !strncmp(str, "cont", strlen("cont")))
> > + attrs->cont = true;
> > else if (!strncmp(str, "size", strlen("size"))) {
> > int map_bits = parse_map_size(str);
> >
> > @@ -843,7 +850,10 @@ static int event_hist_trigger_print(struct seq_file *m,
> > if (data->filter_str)
> > seq_printf(m, " if %s", data->filter_str);
> >
> > - seq_puts(m, " [active]");
> > + if (data->paused)
> > + seq_puts(m, " [paused]");
> > + else
> > + seq_puts(m, " [active]");
> >
> > seq_putc(m, '\n');
> >
> > @@ -882,16 +892,25 @@ static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,
> > struct event_trigger_data *data,
> > struct trace_event_file *file)
> > {
> > + struct hist_trigger_data *hist_data = data->private_data;
> > struct event_trigger_data *test;
> > int ret = 0;
> >
> > list_for_each_entry_rcu(test, &file->triggers, list) {
> > if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
> > - ret = -EEXIST;
> > + if (hist_data->attrs->pause)
> > + test->paused = true;
> > + else if (hist_data->attrs->cont)
> > + test->paused = false;
> > + else
> > + ret = -EEXIST;
> > goto out;
> > }
> > }
> >
> > + if (hist_data->attrs->pause)
> > + data->paused = true;
> > +
> > if (data->ops->init) {
> > ret = data->ops->init(data->ops, data);
> > if (ret < 0)
> > @@ -984,7 +1003,8 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
> > * triggers registered a failure too.
> > */
> > if (!ret) {
> > - ret = -ENOENT;
> > + if (!(attrs->pause || attrs->cont))
> > + ret = -ENOENT;
> > goto out_free;
> > } else if (ret < 0)
> > goto out_free;
> >
>
>
--
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