Re: [PATCH v8 03/10] tracing: Add 'traceon' and 'traceoff' event trigger commands
On Fri, 06 Sep 2013 21:37:58 +0900
Masami Hiramatsu wrote:
> (2013/09/06 5:46), Steven Rostedt wrote:
> > On Mon, 2 Sep 2013 22:52:19 -0500
> > Tom Zanussi wrote:
> >> extern void destroy_preds(struct ftrace_event_call *call);
> >> diff --git a/kernel/trace/trace_events_trigger.c
> >> b/kernel/trace/trace_events_trigger.c
> >> index 85319cf..5388d55 100644
> >> --- a/kernel/trace/trace_events_trigger.c
> >> +++ b/kernel/trace/trace_events_trigger.c
> >> @@ -28,6 +28,13 @@
> >> static LIST_HEAD(trigger_commands);
> >> static DEFINE_MUTEX(trigger_cmd_mutex);
> >>
> >> +static void
> >> +trigger_data_free(struct event_trigger_data *data)
> >> +{
> >> + synchronize_sched(); /* make sure current triggers exit before free */
> >
> > Again, I think this can and should be synchronize_rcu().
> >
>
> As in the previous patch, event triggers called under preempt disabled.
Yeah, my fault. I was thinking tracepoints used rcu_read_lock(). This
is fine.
>
> > +void event_triggers_call(struct ftrace_event_file *file)
> > +{
> > + struct event_trigger_data *data;
> > +
> > + if (list_empty(>triggers))
> > + return;
> > +
> > + preempt_disable_notrace();
> > + list_for_each_entry_rcu(data, >triggers, list)
> > + data->ops->func(data);
> > + preempt_enable_notrace();
> > +}
>
> In this case, I think synchronize_sched() is correct. Of course,
> we need to discuss why it needs to disable preempt here. :)
>
Yeah, that is confusing.
-- Steve
--
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/
Re: [PATCH v8 03/10] tracing: Add 'traceon' and 'traceoff' event trigger commands
(2013/09/06 5:46), Steven Rostedt wrote:
> On Mon, 2 Sep 2013 22:52:19 -0500
> Tom Zanussi wrote:
>> extern void destroy_preds(struct ftrace_event_call *call);
>> diff --git a/kernel/trace/trace_events_trigger.c
>> b/kernel/trace/trace_events_trigger.c
>> index 85319cf..5388d55 100644
>> --- a/kernel/trace/trace_events_trigger.c
>> +++ b/kernel/trace/trace_events_trigger.c
>> @@ -28,6 +28,13 @@
>> static LIST_HEAD(trigger_commands);
>> static DEFINE_MUTEX(trigger_cmd_mutex);
>>
>> +static void
>> +trigger_data_free(struct event_trigger_data *data)
>> +{
>> +synchronize_sched(); /* make sure current triggers exit before free */
>
> Again, I think this can and should be synchronize_rcu().
>
As in the previous patch, event triggers called under preempt disabled.
> +void event_triggers_call(struct ftrace_event_file *file)
> +{
> + struct event_trigger_data *data;
> +
> + if (list_empty(>triggers))
> + return;
> +
> + preempt_disable_notrace();
> + list_for_each_entry_rcu(data, >triggers, list)
> + data->ops->func(data);
> + preempt_enable_notrace();
> +}
In this case, I think synchronize_sched() is correct. Of course,
we need to discuss why it needs to disable preempt here. :)
Thank you,
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com
--
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/
Re: [PATCH v8 03/10] tracing: Add 'traceon' and 'traceoff' event trigger commands
(2013/09/06 5:46), Steven Rostedt wrote:
On Mon, 2 Sep 2013 22:52:19 -0500
Tom Zanussi tom.zanu...@linux.intel.com wrote:
extern void destroy_preds(struct ftrace_event_call *call);
diff --git a/kernel/trace/trace_events_trigger.c
b/kernel/trace/trace_events_trigger.c
index 85319cf..5388d55 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -28,6 +28,13 @@
static LIST_HEAD(trigger_commands);
static DEFINE_MUTEX(trigger_cmd_mutex);
+static void
+trigger_data_free(struct event_trigger_data *data)
+{
+synchronize_sched(); /* make sure current triggers exit before free */
Again, I think this can and should be synchronize_rcu().
As in the previous patch, event triggers called under preempt disabled.
+void event_triggers_call(struct ftrace_event_file *file)
+{
+ struct event_trigger_data *data;
+
+ if (list_empty(file-triggers))
+ return;
+
+ preempt_disable_notrace();
+ list_for_each_entry_rcu(data, file-triggers, list)
+ data-ops-func(data);
+ preempt_enable_notrace();
+}
In this case, I think synchronize_sched() is correct. Of course,
we need to discuss why it needs to disable preempt here. :)
Thank you,
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com
--
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/
Re: [PATCH v8 03/10] tracing: Add 'traceon' and 'traceoff' event trigger commands
On Fri, 06 Sep 2013 21:37:58 +0900
Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote:
(2013/09/06 5:46), Steven Rostedt wrote:
On Mon, 2 Sep 2013 22:52:19 -0500
Tom Zanussi tom.zanu...@linux.intel.com wrote:
extern void destroy_preds(struct ftrace_event_call *call);
diff --git a/kernel/trace/trace_events_trigger.c
b/kernel/trace/trace_events_trigger.c
index 85319cf..5388d55 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -28,6 +28,13 @@
static LIST_HEAD(trigger_commands);
static DEFINE_MUTEX(trigger_cmd_mutex);
+static void
+trigger_data_free(struct event_trigger_data *data)
+{
+ synchronize_sched(); /* make sure current triggers exit before free */
Again, I think this can and should be synchronize_rcu().
As in the previous patch, event triggers called under preempt disabled.
Yeah, my fault. I was thinking tracepoints used rcu_read_lock(). This
is fine.
+void event_triggers_call(struct ftrace_event_file *file)
+{
+ struct event_trigger_data *data;
+
+ if (list_empty(file-triggers))
+ return;
+
+ preempt_disable_notrace();
+ list_for_each_entry_rcu(data, file-triggers, list)
+ data-ops-func(data);
+ preempt_enable_notrace();
+}
In this case, I think synchronize_sched() is correct. Of course,
we need to discuss why it needs to disable preempt here. :)
Yeah, that is confusing.
-- Steve
--
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/
[PATCH v8 03/10] tracing: Add 'traceon' and 'traceoff' event trigger commands
Add 'traceon' and 'traceoff' event_command commands. traceon and
traceoff event triggers are added by the user via these commands in a
similar way and using practically the same syntax as the analagous
'traceon' and 'traceoff' ftrace function commands, but instead of
writing to the set_ftrace_filter file, the traceon and traceoff
triggers are written to the per-event 'trigger' files:
echo 'traceon' > .../tracing/events/somesys/someevent/trigger
echo 'traceoff' > .../tracing/events/somesys/someevent/trigger
The above command will turn tracing on or off whenever someevent is
hit.
This also adds a 'count' version that limits the number of times the
command will be invoked:
echo 'traceon:N' > .../tracing/events/somesys/someevent/trigger
echo 'traceoff:N' > .../tracing/events/somesys/someevent/trigger
Where N is the number of times the command will be invoked.
The above commands will will turn tracing on or off whenever someevent
is hit, but only N times.
Some common register/unregister_trigger() implementations of the
event_command reg()/unreg() callbacks are also provided, which add and
remove trigger instances to the per-event list of triggers, and
arm/disarm them as appropriate. event_trigger_callback() is a
general-purpose event_command func() implementation that orchestrates
command parsing and registration for most normal commands.
Most event commands will use these, but some will override and
possibly reuse them.
The event_trigger_init(), event_trigger_free(), and
event_trigger_print() functions are meant to be common implementations
of the event_trigger_ops init(), free(), and print() ops,
respectively.
Most trigger_ops implementations will use these, but some will
override and possibly reuse them.
Signed-off-by: Tom Zanussi
---
include/linux/ftrace_event.h| 1 +
kernel/trace/trace_events_trigger.c | 436
2 files changed, 437 insertions(+)
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 34ae1d4..a14650b 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -318,6 +318,7 @@ struct ftrace_event_file {
enum event_trigger_type {
ETT_NONE= (0),
+ ETT_TRACE_ONOFF = (1 << 0),
};
extern void destroy_preds(struct ftrace_event_call *call);
diff --git a/kernel/trace/trace_events_trigger.c
b/kernel/trace/trace_events_trigger.c
index 85319cf..5388d55 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -28,6 +28,13 @@
static LIST_HEAD(trigger_commands);
static DEFINE_MUTEX(trigger_cmd_mutex);
+static void
+trigger_data_free(struct event_trigger_data *data)
+{
+ synchronize_sched(); /* make sure current triggers exit before free */
+ kfree(data);
+}
+
void event_triggers_call(struct ftrace_event_file *file)
{
struct event_trigger_data *data;
@@ -215,6 +222,121 @@ const struct file_operations event_trigger_fops = {
.release = event_trigger_release,
};
+/*
+ * Currently we only register event commands from __init, so mark this
+ * __init too.
+ */
+static __init int register_event_command(struct event_command *cmd,
+struct list_head *cmd_list,
+struct mutex *cmd_list_mutex)
+{
+ struct event_command *p;
+ int ret = 0;
+
+ mutex_lock(cmd_list_mutex);
+ list_for_each_entry(p, cmd_list, list) {
+ if (strcmp(cmd->name, p->name) == 0) {
+ ret = -EBUSY;
+ goto out_unlock;
+ }
+ }
+ list_add(>list, cmd_list);
+ out_unlock:
+ mutex_unlock(cmd_list_mutex);
+
+ return ret;
+}
+
+/*
+ * Currently we only unregister event commands from __init, so mark
+ * this __init too.
+ */
+static __init int unregister_event_command(struct event_command *cmd,
+ struct list_head *cmd_list,
+ struct mutex *cmd_list_mutex)
+{
+ struct event_command *p, *n;
+ int ret = -ENODEV;
+
+ mutex_lock(cmd_list_mutex);
+ list_for_each_entry_safe(p, n, cmd_list, list) {
+ if (strcmp(cmd->name, p->name) == 0) {
+ ret = 0;
+ list_del_init(>list);
+ goto out_unlock;
+ }
+ }
+ out_unlock:
+ mutex_unlock(cmd_list_mutex);
+
+ return ret;
+}
+
+/**
+ * event_trigger_print - generic event_trigger_ops @print implementation
+ *
+ * Common implementation for event triggers to print themselves.
+ *
+ * Usually wrapped by a function that simply sets the @name of the
+ * trigger command and then invokes this.
+ */
+static int
+event_trigger_print(const char *name, struct seq_file *m,
+ void *data, char *filter_str)
+{
+ long count = (long)data;
+
+ seq_printf(m, "%s", name);
[PATCH v8 03/10] tracing: Add 'traceon' and 'traceoff' event trigger commands
Add 'traceon' and 'traceoff' event_command commands. traceon and
traceoff event triggers are added by the user via these commands in a
similar way and using practically the same syntax as the analagous
'traceon' and 'traceoff' ftrace function commands, but instead of
writing to the set_ftrace_filter file, the traceon and traceoff
triggers are written to the per-event 'trigger' files:
echo 'traceon' .../tracing/events/somesys/someevent/trigger
echo 'traceoff' .../tracing/events/somesys/someevent/trigger
The above command will turn tracing on or off whenever someevent is
hit.
This also adds a 'count' version that limits the number of times the
command will be invoked:
echo 'traceon:N' .../tracing/events/somesys/someevent/trigger
echo 'traceoff:N' .../tracing/events/somesys/someevent/trigger
Where N is the number of times the command will be invoked.
The above commands will will turn tracing on or off whenever someevent
is hit, but only N times.
Some common register/unregister_trigger() implementations of the
event_command reg()/unreg() callbacks are also provided, which add and
remove trigger instances to the per-event list of triggers, and
arm/disarm them as appropriate. event_trigger_callback() is a
general-purpose event_command func() implementation that orchestrates
command parsing and registration for most normal commands.
Most event commands will use these, but some will override and
possibly reuse them.
The event_trigger_init(), event_trigger_free(), and
event_trigger_print() functions are meant to be common implementations
of the event_trigger_ops init(), free(), and print() ops,
respectively.
Most trigger_ops implementations will use these, but some will
override and possibly reuse them.
Signed-off-by: Tom Zanussi tom.zanu...@linux.intel.com
---
include/linux/ftrace_event.h| 1 +
kernel/trace/trace_events_trigger.c | 436
2 files changed, 437 insertions(+)
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 34ae1d4..a14650b 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -318,6 +318,7 @@ struct ftrace_event_file {
enum event_trigger_type {
ETT_NONE= (0),
+ ETT_TRACE_ONOFF = (1 0),
};
extern void destroy_preds(struct ftrace_event_call *call);
diff --git a/kernel/trace/trace_events_trigger.c
b/kernel/trace/trace_events_trigger.c
index 85319cf..5388d55 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -28,6 +28,13 @@
static LIST_HEAD(trigger_commands);
static DEFINE_MUTEX(trigger_cmd_mutex);
+static void
+trigger_data_free(struct event_trigger_data *data)
+{
+ synchronize_sched(); /* make sure current triggers exit before free */
+ kfree(data);
+}
+
void event_triggers_call(struct ftrace_event_file *file)
{
struct event_trigger_data *data;
@@ -215,6 +222,121 @@ const struct file_operations event_trigger_fops = {
.release = event_trigger_release,
};
+/*
+ * Currently we only register event commands from __init, so mark this
+ * __init too.
+ */
+static __init int register_event_command(struct event_command *cmd,
+struct list_head *cmd_list,
+struct mutex *cmd_list_mutex)
+{
+ struct event_command *p;
+ int ret = 0;
+
+ mutex_lock(cmd_list_mutex);
+ list_for_each_entry(p, cmd_list, list) {
+ if (strcmp(cmd-name, p-name) == 0) {
+ ret = -EBUSY;
+ goto out_unlock;
+ }
+ }
+ list_add(cmd-list, cmd_list);
+ out_unlock:
+ mutex_unlock(cmd_list_mutex);
+
+ return ret;
+}
+
+/*
+ * Currently we only unregister event commands from __init, so mark
+ * this __init too.
+ */
+static __init int unregister_event_command(struct event_command *cmd,
+ struct list_head *cmd_list,
+ struct mutex *cmd_list_mutex)
+{
+ struct event_command *p, *n;
+ int ret = -ENODEV;
+
+ mutex_lock(cmd_list_mutex);
+ list_for_each_entry_safe(p, n, cmd_list, list) {
+ if (strcmp(cmd-name, p-name) == 0) {
+ ret = 0;
+ list_del_init(p-list);
+ goto out_unlock;
+ }
+ }
+ out_unlock:
+ mutex_unlock(cmd_list_mutex);
+
+ return ret;
+}
+
+/**
+ * event_trigger_print - generic event_trigger_ops @print implementation
+ *
+ * Common implementation for event triggers to print themselves.
+ *
+ * Usually wrapped by a function that simply sets the @name of the
+ * trigger command and then invokes this.
+ */
+static int
+event_trigger_print(const char *name, struct seq_file *m,
+ void *data, char *filter_str)
+{
+ long count = (long)data;
+
+
