Re: [PATCH v9 13/22] tracing: Add hist trigger support for clearing a trace
On Wed, 2015-07-22 at 22:50 +0900, Masami Hiramatsu wrote:
> On 2015/07/17 2:22, Tom Zanussi wrote:
> > Allow users to append 'clear' to an existing trigger in order to have
> > the hash table cleared.
> >
> > This expands the hist trigger syntax from this:
> > # echo hist:keys=xxx:vals=yyy:sort=zzz.descending:pause/cont \
> >[ if filter] > event/trigger
> >
> > to this:
> >
> > # echo hist:keys=xxx:vals=yyy:sort=zzz.descending:pause/cont/clear \
> > [ if filter] > event/trigger
>
> By the way, since pause/cont/clear is not the trigger but the commands
> (which executed immediately), I think it should be a write fops of
> "hist" special file.
>
> e.g. to clear the histogram, write 0 to hist
>
> # echo 0 > event/hist
>
> And pause/cont will be 1 and 2.
>
> # echo 1 > event/hist <- pause
> and
> # echo 2 > event/hist <- continue
>
> What would you think ?
>
I do think it makes sense, and like the idea. But if we allow multiple
histograms per event, does it still work? (I mean in that case, we'd
like to pause/continue/clear them individually, right?)
Tom
> Thanks,
>
>
> >
> > Signed-off-by: Tom Zanussi
> > ---
> > kernel/trace/trace.c | 4 +++-
> > kernel/trace/trace_events_hist.c | 25 -
> > 2 files changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 547bbc8..27daa28 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -3791,7 +3791,7 @@ static const char readme_msg[] =
> > "\t[:values= > "\t[:sort=field1,field2,...]\n"
> > "\t[:size=#entries]\n"
> > - "\t[:pause][:continue]\n"
> > + "\t[:pause][:continue][:clear]\n"
> > "\t[if ]\n\n"
> > "\tWhen a matching event is hit, an entry is added to a hash\n"
> > "\ttable using the key(s) and value(s) named. Keys and values\n"
> > @@ -3826,6 +3826,8 @@ static const char readme_msg[] =
> > "\ttrigger or to start a hist trigger but not log any events\n"
> > "\tuntil told to do so. 'continue' can be used to start or\n"
> > "\trestart a paused hist trigger.\n\n"
> > + "\tThe 'clear' param will clear the contents of a running hist\n"
> > + "\ttrigger and leave its current paused/active state.\n\n"
> > #endif
> > ;
> >
> > diff --git a/kernel/trace/trace_events_hist.c
> > b/kernel/trace/trace_events_hist.c
> > index 3ae58e7..d8259fe 100644
> > --- a/kernel/trace/trace_events_hist.c
> > +++ b/kernel/trace/trace_events_hist.c
> > @@ -80,6 +80,7 @@ struct hist_trigger_attrs {
> > char*sort_key_str;
> > boolpause;
> > boolcont;
> > + boolclear;
> > unsigned intmap_bits;
> > };
> >
> > @@ -188,6 +189,8 @@ static struct hist_trigger_attrs
> > *parse_hist_trigger_attrs(char *trigger_str)
> > attrs->sort_key_str = kstrdup(str, GFP_KERNEL);
> > else if (!strncmp(str, "pause", strlen("pause")))
> > attrs->pause = true;
> > + else if (!strncmp(str, "clear", strlen("clear")))
> > + attrs->clear = true;
> > else if (!strncmp(str, "continue", strlen("continue")) ||
> > !strncmp(str, "cont", strlen("cont")))
> > attrs->cont = true;
> > @@ -888,6 +891,24 @@ static struct event_trigger_ops
> > *event_hist_get_trigger_ops(char *cmd,
> > return &event_hist_trigger_ops;
> > }
> >
> > +static void hist_clear(struct event_trigger_data *data)
> > +{
> > + struct hist_trigger_data *hist_data = data->private_data;
> > + bool paused;
> > +
> > + paused = data->paused;
> > + data->paused = true;
> > +
> > + synchronize_sched();
> > +
> > + tracing_map_clear(hist_data->map);
> > +
> > + atomic64_set(&hist_data->total_hits, 0);
> > + atomic64_set(&hist_data->drops, 0);
> > +
> > + data->paused = paused;
> > +}
> > +
> > static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,
> > struct event_trigger_data *data,
> > struct trace_event_file *file)
> > @@ -902,6 +923,8 @@ static int hist_register_trigger(char *glob, struct
> > event_trigger_ops *ops,
> > test->paused = true;
> > else if (hist_data->attrs->cont)
> > test->paused = false;
> > + else if (hist_data->attrs->clear)
> > + hist_clear(test);
> > else
> > ret = -EEXIST;
> > goto out;
> > @@ -1003,7 +1026,7 @@ static int event_hist_trigger_func(struct
> > event_command *cmd_ops,
> > * triggers registered a failure too.
> > */
> > if (!ret) {
> > - if (!(attrs->pause || attrs->cont))
> > + if (!(attr
Re: [PATCH v9 13/22] tracing: Add hist trigger support for clearing a trace
On 2015/07/17 2:22, Tom Zanussi wrote:
> Allow users to append 'clear' to an existing trigger in order to have
> the hash table cleared.
>
> This expands the hist trigger syntax from this:
> # echo hist:keys=xxx:vals=yyy:sort=zzz.descending:pause/cont \
>[ if filter] > event/trigger
>
> to this:
>
> # echo hist:keys=xxx:vals=yyy:sort=zzz.descending:pause/cont/clear \
> [ if filter] > event/trigger
By the way, since pause/cont/clear is not the trigger but the commands
(which executed immediately), I think it should be a write fops of
"hist" special file.
e.g. to clear the histogram, write 0 to hist
# echo 0 > event/hist
And pause/cont will be 1 and 2.
# echo 1 > event/hist <- pause
and
# echo 2 > event/hist <- continue
What would you think ?
Thanks,
>
> Signed-off-by: Tom Zanussi
> ---
> kernel/trace/trace.c | 4 +++-
> kernel/trace/trace_events_hist.c | 25 -
> 2 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 547bbc8..27daa28 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -3791,7 +3791,7 @@ static const char readme_msg[] =
> "\t[:values= "\t[:sort=field1,field2,...]\n"
> "\t[:size=#entries]\n"
> - "\t[:pause][:continue]\n"
> + "\t[:pause][:continue][:clear]\n"
> "\t[if ]\n\n"
> "\tWhen a matching event is hit, an entry is added to a hash\n"
> "\ttable using the key(s) and value(s) named. Keys and values\n"
> @@ -3826,6 +3826,8 @@ static const char readme_msg[] =
> "\ttrigger or to start a hist trigger but not log any events\n"
> "\tuntil told to do so. 'continue' can be used to start or\n"
> "\trestart a paused hist trigger.\n\n"
> + "\tThe 'clear' param will clear the contents of a running hist\n"
> + "\ttrigger and leave its current paused/active state.\n\n"
> #endif
> ;
>
> diff --git a/kernel/trace/trace_events_hist.c
> b/kernel/trace/trace_events_hist.c
> index 3ae58e7..d8259fe 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -80,6 +80,7 @@ struct hist_trigger_attrs {
> char*sort_key_str;
> boolpause;
> boolcont;
> + boolclear;
> unsigned intmap_bits;
> };
>
> @@ -188,6 +189,8 @@ static struct hist_trigger_attrs
> *parse_hist_trigger_attrs(char *trigger_str)
> attrs->sort_key_str = kstrdup(str, GFP_KERNEL);
> else if (!strncmp(str, "pause", strlen("pause")))
> attrs->pause = true;
> + else if (!strncmp(str, "clear", strlen("clear")))
> + attrs->clear = true;
> else if (!strncmp(str, "continue", strlen("continue")) ||
>!strncmp(str, "cont", strlen("cont")))
> attrs->cont = true;
> @@ -888,6 +891,24 @@ static struct event_trigger_ops
> *event_hist_get_trigger_ops(char *cmd,
> return &event_hist_trigger_ops;
> }
>
> +static void hist_clear(struct event_trigger_data *data)
> +{
> + struct hist_trigger_data *hist_data = data->private_data;
> + bool paused;
> +
> + paused = data->paused;
> + data->paused = true;
> +
> + synchronize_sched();
> +
> + tracing_map_clear(hist_data->map);
> +
> + atomic64_set(&hist_data->total_hits, 0);
> + atomic64_set(&hist_data->drops, 0);
> +
> + data->paused = paused;
> +}
> +
> static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,
>struct event_trigger_data *data,
>struct trace_event_file *file)
> @@ -902,6 +923,8 @@ static int hist_register_trigger(char *glob, struct
> event_trigger_ops *ops,
> test->paused = true;
> else if (hist_data->attrs->cont)
> test->paused = false;
> + else if (hist_data->attrs->clear)
> + hist_clear(test);
> else
> ret = -EEXIST;
> goto out;
> @@ -1003,7 +1026,7 @@ static int event_hist_trigger_func(struct event_command
> *cmd_ops,
>* triggers registered a failure too.
>*/
> if (!ret) {
> - if (!(attrs->pause || attrs->cont))
> + if (!(attrs->pause || attrs->cont || attrs->clear))
> ret = -ENOENT;
> goto out_free;
> } else if (ret < 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...@hitachi.com
--
To unsubscribe from this list: send the
[PATCH v9 13/22] tracing: Add hist trigger support for clearing a trace
Allow users to append 'clear' to an existing trigger in order to have
the hash table cleared.
This expands the hist trigger syntax from this:
# echo hist:keys=xxx:vals=yyy:sort=zzz.descending:pause/cont \
[ if filter] > event/trigger
to this:
# echo hist:keys=xxx:vals=yyy:sort=zzz.descending:pause/cont/clear \
[ if filter] > event/trigger
Signed-off-by: Tom Zanussi
---
kernel/trace/trace.c | 4 +++-
kernel/trace/trace_events_hist.c | 25 -
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 547bbc8..27daa28 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3791,7 +3791,7 @@ static const char readme_msg[] =
"\t[:values=]\n\n"
"\tWhen a matching event is hit, an entry is added to a hash\n"
"\ttable using the key(s) and value(s) named. Keys and values\n"
@@ -3826,6 +3826,8 @@ static const char readme_msg[] =
"\ttrigger or to start a hist trigger but not log any events\n"
"\tuntil told to do so. 'continue' can be used to start or\n"
"\trestart a paused hist trigger.\n\n"
+ "\tThe 'clear' param will clear the contents of a running hist\n"
+ "\ttrigger and leave its current paused/active state.\n\n"
#endif
;
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 3ae58e7..d8259fe 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -80,6 +80,7 @@ struct hist_trigger_attrs {
char*sort_key_str;
boolpause;
boolcont;
+ boolclear;
unsigned intmap_bits;
};
@@ -188,6 +189,8 @@ static struct hist_trigger_attrs
*parse_hist_trigger_attrs(char *trigger_str)
attrs->sort_key_str = kstrdup(str, GFP_KERNEL);
else if (!strncmp(str, "pause", strlen("pause")))
attrs->pause = true;
+ else if (!strncmp(str, "clear", strlen("clear")))
+ attrs->clear = true;
else if (!strncmp(str, "continue", strlen("continue")) ||
!strncmp(str, "cont", strlen("cont")))
attrs->cont = true;
@@ -888,6 +891,24 @@ static struct event_trigger_ops
*event_hist_get_trigger_ops(char *cmd,
return &event_hist_trigger_ops;
}
+static void hist_clear(struct event_trigger_data *data)
+{
+ struct hist_trigger_data *hist_data = data->private_data;
+ bool paused;
+
+ paused = data->paused;
+ data->paused = true;
+
+ synchronize_sched();
+
+ tracing_map_clear(hist_data->map);
+
+ atomic64_set(&hist_data->total_hits, 0);
+ atomic64_set(&hist_data->drops, 0);
+
+ data->paused = paused;
+}
+
static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,
struct event_trigger_data *data,
struct trace_event_file *file)
@@ -902,6 +923,8 @@ static int hist_register_trigger(char *glob, struct
event_trigger_ops *ops,
test->paused = true;
else if (hist_data->attrs->cont)
test->paused = false;
+ else if (hist_data->attrs->clear)
+ hist_clear(test);
else
ret = -EEXIST;
goto out;
@@ -1003,7 +1026,7 @@ static int event_hist_trigger_func(struct event_command
*cmd_ops,
* triggers registered a failure too.
*/
if (!ret) {
- if (!(attrs->pause || attrs->cont))
+ if (!(attrs->pause || attrs->cont || attrs->clear))
ret = -ENOENT;
goto out_free;
} else if (ret < 0)
--
1.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
