| 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
| ||
|
Message-ID: <1437667104.20186.8.camel@tzanussi-mobl.amr.corp.intel.com>
Date: Thu, 23 Jul 2015 10:58:24 -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: Re: [PATCH v9 12/22] tracing: Add hist trigger support for
pausing and continuing a trace
On Thu, 2015-07-23 at 23:06 +0900, Masami Hiramatsu wrote:
> On 2015/07/23 5:22, Tom Zanussi wrote:
> > 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.
>
> Agreed.
>
> BTW, as I've wrote in another reply, pause/cont/clear commands should be
> for the "hist" file, not for trigger. So, below command will be better.
>
> # echo pause:name=foo > event/hist
> # echo clear:keys=xxx > event/hist
>
Right, that all makes sense, I think we're on the same page with
everything now..
Thanks,
Tom
> Thank you,
>
>
>
> >> [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/
> >
>
>
--
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