[PATCH 0/5] trace-cmd reset fixes

[PATCH 0/5] trace-cmd reset fixes

[Tzvetomir Stoyanov]

This patch series fixes "trace-cmd reset" command to 
restore default values of various ftrace configurations:
tracng_on, trace_clock, set_event_pid and tracing_max_latency.
It also fixes a segfault when the command is executed with 
"-a -d" options.

Tzvetomir Stoyanov (5):
  fix "trace-cmd reset" command to restore "tracng_on"
  fix "trace-cmd reset -a -d" segfault
  fix "trace-cmd reset" command to restore default clock
  fix "trace-cmd reset" command to restore the default value of
    set_event_pid
  fix "trace-cmd reset" command to restore the default value of
    tracing_max_latency

 tracecmd/trace-record.c | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

-- 
2.19.2

[PATCH 1/5] fix "trace-cmd reset" command to restore "tracng_on"

[Tzvetomir Stoyanov]

The "trace-cmd reset" command should restore ftrace to its default state.
By default, "tracing/current_tracer" is "nop" and "tracing/tracing_on" is "1".
This patch sets "tracing/tracing_on" to 1, when the command "trace-cmd reset"
is executed.

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
 tracecmd/trace-record.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 7666a24..a044cf5 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -4456,6 +4456,8 @@ void trace_reset(int argc, char **argv)
 	clear_triggers();
 	tracecmd_remove_instances();
 	clear_func_filters();
+	/* restore tracing_on to 1 */
+	tracecmd_enable_tracing();
 	exit(0);
 }
 
-- 
2.19.2

[PATCH 2/5] fix "trace-cmd reset -a -d" segfault

[Tzvetomir Stoyanov]

This patch fixes a segfault when "trace-cmd reset -a -d"
is executed and there is at least one ftrace instance created.

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
 tracecmd/trace-record.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index a044cf5..e02660a 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -4432,14 +4432,14 @@ void trace_reset(int argc, char **argv)
 		case 'a':
 			last_specified_all = 1;
 			add_all_instances();
-			for_each_instance(instance) {
-				instance->flags |= BUFFER_FL_KEEP;
+			for_each_instance(inst) {
+				inst->flags |= BUFFER_FL_KEEP;
 			}
 			break;
 		case 'd':
 			if (last_specified_all) {
 				for_each_instance(inst) {
-					instance->flags &= ~BUFFER_FL_KEEP;
+					inst->flags &= ~BUFFER_FL_KEEP;
 				}
 			} else {
 				if (is_top_instance(instance))
-- 
2.19.2

[PATCH 3/5] fix "trace-cmd reset" command to restore default clock

[Tzvetomir Stoyanov]

The "trace-cmd reset" command should restore ftrace to its default state.
This patch sets "tracing/trace_clock" to its default value "local",
when the "trace-cmd reset" command is executed.

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
 tracecmd/trace-record.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index e02660a..38d9871 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -3753,6 +3753,14 @@ static void clear_filters(void)
 		clear_instance_filters(instance);
 }
 
+static void reset_clock(void)
+{
+	struct buffer_instance *instance;
+
+	for_all_instances(instance)
+		write_instance_file(instance, "trace_clock", "local", "clock");
+}
+
 static void clear_triggers(void)
 {
 	struct buffer_instance *instance;
@@ -4454,6 +4462,8 @@ void trace_reset(int argc, char **argv)
 	set_buffer_size();
 	clear_filters();
 	clear_triggers();
+	/* set clock to "local" */
+	reset_clock();
 	tracecmd_remove_instances();
 	clear_func_filters();
 	/* restore tracing_on to 1 */
-- 
2.19.2

[PATCH 4/5] fix "trace-cmd reset" command to restore the default value of set_event_pid

[Tzvetomir Stoyanov]

The "trace-cmd reset" command should restore ftrace to its default state.
This patch sets "tracing/set_event_pid" to an empty string,
when the "trace-cmd reset" command is executed.

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
 tracecmd/trace-record.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 38d9871..f77e49a 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -3761,6 +3761,15 @@ static void reset_clock(void)
 		write_instance_file(instance, "trace_clock", "local", "clock");
 }
 
+static void reset_event_pid(void)
+{
+	struct buffer_instance *instance;
+
+	for_all_instances(instance)
+		write_instance_file(instance, "set_event_pid", "", "event_pid");
+}
+
+
 static void clear_triggers(void)
 {
 	struct buffer_instance *instance;
@@ -4464,6 +4473,7 @@ void trace_reset(int argc, char **argv)
 	clear_triggers();
 	/* set clock to "local" */
 	reset_clock();
+	reset_event_pid();
 	tracecmd_remove_instances();
 	clear_func_filters();
 	/* restore tracing_on to 1 */
-- 
2.19.2

[PATCH 5/5] fix "trace-cmd reset" command to restore the default value of tracing_max_latency

[Tzvetomir Stoyanov]

The "trace-cmd reset" command should restore ftrace to its default state.
This patch sets "tracing/tracing_max_latency" to 0,
when the "trace-cmd reset" command is executed.

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
 tracecmd/trace-record.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index f77e49a..a9bcd8d 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -3769,6 +3769,14 @@ static void reset_event_pid(void)
 		write_instance_file(instance, "set_event_pid", "", "event_pid");
 }
 
+static void reset_max_latency_instance(void)
+{
+	struct buffer_instance *instance;
+
+	for_all_instances(instance)
+		write_instance_file(instance,
+				    "tracing_max_latency", "0", "max_latency");
+}
 
 static void clear_triggers(void)
 {
@@ -4474,6 +4482,7 @@ void trace_reset(int argc, char **argv)
 	/* set clock to "local" */
 	reset_clock();
 	reset_event_pid();
+	reset_max_latency_instance();
 	tracecmd_remove_instances();
 	clear_func_filters();
 	/* restore tracing_on to 1 */
-- 
2.19.2

Re: [PATCH 0/5] trace-cmd reset fixes

[Slavomir Kaslev]

On Thu, Dec 13, 2018 at 4:22 PM Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:
>
> This patch series fixes "trace-cmd reset" command to
> restore default values of various ftrace configurations:
> tracng_on, trace_clock, set_event_pid and tracing_max_latency.
> It also fixes a segfault when the command is executed with
> "-a -d" options.
>
> Tzvetomir Stoyanov (5):
>   fix "trace-cmd reset" command to restore "tracng_on"
>   fix "trace-cmd reset -a -d" segfault
>   fix "trace-cmd reset" command to restore default clock
>   fix "trace-cmd reset" command to restore the default value of
>     set_event_pid
>   fix "trace-cmd reset" command to restore the default value of
>     tracing_max_latency
>
>  tracecmd/trace-record.c | 37 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 34 insertions(+), 3 deletions(-)

All 5 patches seems good to me

Reviewed-by: Slavomir Kaslev <kaslevs@vmware.com>

Thank you,

-- Slavi

Re: [PATCH 4/5] fix "trace-cmd reset" command to restore the default value of set_event_pid

[Steven Rostedt]

I took the first 3 patches, but fixed the subject to say:

	trace-cmd: Fix ....

As all commits should have a "system:" format and the description part
start with a capital letter (note, this is the requirement for the
Linux kernel as well).

On Thu, 13 Dec 2018 14:21:43 +0000
Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:

> The "trace-cmd reset" command should restore ftrace to its default state.
> This patch sets "tracing/set_event_pid" to an empty string,
> when the "trace-cmd reset" command is executed.
> 
> Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
> ---
>  tracecmd/trace-record.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
> index 38d9871..f77e49a 100644
> --- a/tracecmd/trace-record.c
> +++ b/tracecmd/trace-record.c
> @@ -3761,6 +3761,15 @@ static void reset_clock(void)
>  		write_instance_file(instance, "trace_clock", "local", "clock");
>  }
>  
> +static void reset_event_pid(void)
> +{
> +	struct buffer_instance *instance;
> +
> +	for_all_instances(instance)
> +		write_instance_file(instance, "set_event_pid", "", "event_pid");
> +}

Hmm, we should utilize "add_event_pid()" here instead, and simply pass
in an empty line.

Before doing that, make a clean up patch that makes add_event_pid()
utilize write_instance_file() instead of open coding it.

Thanks Tzvetomir!

-- Steve

> +
> +
>  static void clear_triggers(void)
>  {
>  	struct buffer_instance *instance;
> @@ -4464,6 +4473,7 @@ void trace_reset(int argc, char **argv)
>  	clear_triggers();
>  	/* set clock to "local" */
>  	reset_clock();
> +	reset_event_pid();
>  	tracecmd_remove_instances();
>  	clear_func_filters();
>  	/* restore tracing_on to 1 */

Re: [PATCH 5/5] fix "trace-cmd reset" command to restore the default value of tracing_max_latency

[Steven Rostedt]

On Thu, 13 Dec 2018 14:21:44 +0000
Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:

> The "trace-cmd reset" command should restore ftrace to its default state.
> This patch sets "tracing/tracing_max_latency" to 0,
> when the "trace-cmd reset" command is executed.
> 
> Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
> ---
>  tracecmd/trace-record.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
> index f77e49a..a9bcd8d 100644
> --- a/tracecmd/trace-record.c
> +++ b/tracecmd/trace-record.c
> @@ -3769,6 +3769,14 @@ static void reset_event_pid(void)
>  		write_instance_file(instance, "set_event_pid", "", "event_pid");
>  }
>  
> +static void reset_max_latency_instance(void)
> +{
> +	struct buffer_instance *instance;
> +
> +	for_all_instances(instance)
> +		write_instance_file(instance,
> +				    "tracing_max_latency", "0", "max_latency");
> +}

We should fix reset_max_latency() use write_instance_file, and then
make a reset_max_latency_instance() to call it on all instance.

-- Steve

>  
>  static void clear_triggers(void)
>  {
> @@ -4474,6 +4482,7 @@ void trace_reset(int argc, char **argv)
>  	/* set clock to "local" */
>  	reset_clock();
>  	reset_event_pid();
> +	reset_max_latency_instance();
>  	tracecmd_remove_instances();
>  	clear_func_filters();
>  	/* restore tracing_on to 1 */