* [PATCH v4 0/6] trace-cmd reset fixes
@ 2019-03-11 8:33 Tzvetomir Stoyanov
2019-03-11 8:33 ` [PATCH v4 1/6] trace-cmd: Fix "trace-cmd reset" command to restore "tracng_on" Tzvetomir Stoyanov
` (6 more replies)
0 siblings, 7 replies; 9+ messages in thread
From: Tzvetomir Stoyanov @ 2019-03-11 8:33 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
[
V4 changes:
- An extra patch was inluded in v3 (not realeted to trace-cmd reset fixes), by mistake.
V3 changes:
- Refactored reset_max_latency() and reset_max_latency_instance(),
as Steven Rostedt suggested.
V2 changes:
- Fixed subjects of the patches.
- Refactored add_event_pid() and reset_event_pid(),
as Steven Rostedt suggested.
]
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 (6):
trace-cmd: Fix "trace-cmd reset" command to restore "tracng_on"
trace-cmd: Fix "trace-cmd reset -a -d" segfault
trace-cmd: Fix "trace-cmd reset" command to restore default clock
trace-cmd: Reafctored add_event_pid()to utilize write_instance_file()
trace-cmd: Fix "trace-cmd reset" command to restore the default value
of set_event_pid
trace-cmd: Reafctored reset_max_latency() to utilize
write_instance_file()
tracecmd/trace-record.c | 137 ++++++++++++++++++++++------------------
1 file changed, 75 insertions(+), 62 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 1/6] trace-cmd: Fix "trace-cmd reset" command to restore "tracng_on"
2019-03-11 8:33 [PATCH v4 0/6] trace-cmd reset fixes Tzvetomir Stoyanov
@ 2019-03-11 8:33 ` Tzvetomir Stoyanov
2019-03-11 8:33 ` [PATCH v4 2/6] trace-cmd: Fix "trace-cmd reset -a -d" segfault Tzvetomir Stoyanov
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Tzvetomir Stoyanov @ 2019-03-11 8:33 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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 8beefab..fc658b2 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -4496,6 +4496,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.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 2/6] trace-cmd: Fix "trace-cmd reset -a -d" segfault
2019-03-11 8:33 [PATCH v4 0/6] trace-cmd reset fixes Tzvetomir Stoyanov
2019-03-11 8:33 ` [PATCH v4 1/6] trace-cmd: Fix "trace-cmd reset" command to restore "tracng_on" Tzvetomir Stoyanov
@ 2019-03-11 8:33 ` Tzvetomir Stoyanov
2019-03-11 8:33 ` [PATCH v4 3/6] trace-cmd: Fix "trace-cmd reset" command to restore default clock Tzvetomir Stoyanov
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Tzvetomir Stoyanov @ 2019-03-11 8:33 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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 fc658b2..d7267fe 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -4472,14 +4472,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.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 3/6] trace-cmd: Fix "trace-cmd reset" command to restore default clock
2019-03-11 8:33 [PATCH v4 0/6] trace-cmd reset fixes Tzvetomir Stoyanov
2019-03-11 8:33 ` [PATCH v4 1/6] trace-cmd: Fix "trace-cmd reset" command to restore "tracng_on" Tzvetomir Stoyanov
2019-03-11 8:33 ` [PATCH v4 2/6] trace-cmd: Fix "trace-cmd reset -a -d" segfault Tzvetomir Stoyanov
@ 2019-03-11 8:33 ` Tzvetomir Stoyanov
2019-03-11 8:33 ` [PATCH v4 4/6] trace-cmd: Reafctored add_event_pid()to utilize write_instance_file() Tzvetomir Stoyanov
` (3 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Tzvetomir Stoyanov @ 2019-03-11 8:33 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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 d7267fe..6972b9f 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -3793,6 +3793,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;
@@ -4494,6 +4502,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.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 4/6] trace-cmd: Reafctored add_event_pid()to utilize write_instance_file()
2019-03-11 8:33 [PATCH v4 0/6] trace-cmd reset fixes Tzvetomir Stoyanov
` (2 preceding siblings ...)
2019-03-11 8:33 ` [PATCH v4 3/6] trace-cmd: Fix "trace-cmd reset" command to restore default clock Tzvetomir Stoyanov
@ 2019-03-11 8:33 ` Tzvetomir Stoyanov
2019-03-11 8:33 ` [PATCH v4 5/6] trace-cmd: Fix "trace-cmd reset" command to restore the default value of set_event_pid Tzvetomir Stoyanov
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Tzvetomir Stoyanov @ 2019-03-11 8:33 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
This patch changes add_event_pid() to utilize write_instance_file() for
writing set_event_pid instance file, instead of directly opening it.
Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
tracecmd/trace-record.c | 89 +++++++++++++++++++----------------------
1 file changed, 42 insertions(+), 47 deletions(-)
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 6972b9f..a398723 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -1094,30 +1094,64 @@ static void update_sched_events(struct buffer_instance *instance, int pid)
static int open_instance_fd(struct buffer_instance *instance,
const char *file, int flags);
-static void add_event_pid(const char *buf, int len)
+static int write_file(const char *file, const char *str, const char *type)
{
- struct buffer_instance *instance;
+ char buf[BUFSIZ];
int fd;
+ int ret;
- for_all_instances(instance) {
- fd = open_instance_fd(instance, "set_event_pid", O_WRONLY);
- write(fd, buf, len);
+ fd = open(file, O_WRONLY | O_TRUNC);
+ if (fd < 0)
+ die("opening to '%s'", file);
+ ret = write(fd, str, strlen(str));
+ close(fd);
+ if (ret < 0 && type) {
+ /* write failed */
+ fd = open(file, O_RDONLY);
+ if (fd < 0)
+ die("writing to '%s'", file);
+ /* the filter has the error */
+ while ((ret = read(fd, buf, BUFSIZ)) > 0)
+ fprintf(stderr, "%.*s", ret, buf);
+ die("Failed %s of %s\n", type, file);
close(fd);
}
+ return ret;
+}
+
+static int
+write_instance_file(struct buffer_instance *instance,
+ const char *file, const char *str, const char *type)
+{
+ char *path;
+ int ret;
+
+ path = get_instance_file(instance, file);
+ ret = write_file(path, str, type);
+ tracecmd_put_tracing_file(path);
+
+ return ret;
+}
+
+static void add_event_pid(const char *buf)
+{
+ struct buffer_instance *instance;
+
+ for_all_instances(instance)
+ write_instance_file(instance, "set_event_pid", buf, "event_pid");
}
static void add_new_filter_pid(int pid)
{
struct buffer_instance *instance;
char buf[100];
- int len;
add_filter_pid(pid, 0);
- len = sprintf(buf, "%d", pid);
+ sprintf(buf, "%d", pid);
update_ftrace_pid(buf, 0);
if (have_set_event_pid)
- return add_event_pid(buf, len);
+ return add_event_pid(buf);
common_pid_filter = append_pid_filter(common_pid_filter, "common_pid", pid);
@@ -1596,45 +1630,6 @@ static void reset_events(void)
reset_events_instance(instance);
}
-static int write_file(const char *file, const char *str, const char *type)
-{
- char buf[BUFSIZ];
- int fd;
- int ret;
-
- fd = open(file, O_WRONLY | O_TRUNC);
- if (fd < 0)
- die("opening to '%s'", file);
- ret = write(fd, str, strlen(str));
- close(fd);
- if (ret < 0 && type) {
- /* write failed */
- fd = open(file, O_RDONLY);
- if (fd < 0)
- die("writing to '%s'", file);
- /* the filter has the error */
- while ((ret = read(fd, buf, BUFSIZ)) > 0)
- fprintf(stderr, "%.*s", ret, buf);
- die("Failed %s of %s\n", type, file);
- close(fd);
- }
- return ret;
-}
-
-static int
-write_instance_file(struct buffer_instance *instance,
- const char *file, const char *str, const char *type)
-{
- char *path;
- int ret;
-
- path = get_instance_file(instance, file);
- ret = write_file(path, str, type);
- tracecmd_put_tracing_file(path);
-
- return ret;
-}
-
enum {
STATE_NEWLINE,
STATE_SKIP,
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 5/6] trace-cmd: Fix "trace-cmd reset" command to restore the default value of set_event_pid
2019-03-11 8:33 [PATCH v4 0/6] trace-cmd reset fixes Tzvetomir Stoyanov
` (3 preceding siblings ...)
2019-03-11 8:33 ` [PATCH v4 4/6] trace-cmd: Reafctored add_event_pid()to utilize write_instance_file() Tzvetomir Stoyanov
@ 2019-03-11 8:33 ` Tzvetomir Stoyanov
2019-03-11 8:33 ` [PATCH v4 6/6] trace-cmd: Reafctored reset_max_latency() to utilize write_instance_file() Tzvetomir Stoyanov
2019-03-11 13:02 ` [PATCH v4 0/6] trace-cmd reset fixes Slavomir Kaslev
6 siblings, 0 replies; 9+ messages in thread
From: Tzvetomir Stoyanov @ 2019-03-11 8:33 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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 | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index a398723..bdf0c02 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -3796,6 +3796,12 @@ static void reset_clock(void)
write_instance_file(instance, "trace_clock", "local", "clock");
}
+static void reset_event_pid(void)
+{
+ add_event_pid("");
+}
+
+
static void clear_triggers(void)
{
struct buffer_instance *instance;
@@ -4499,6 +4505,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.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 6/6] trace-cmd: Reafctored reset_max_latency() to utilize write_instance_file()
2019-03-11 8:33 [PATCH v4 0/6] trace-cmd reset fixes Tzvetomir Stoyanov
` (4 preceding siblings ...)
2019-03-11 8:33 ` [PATCH v4 5/6] trace-cmd: Fix "trace-cmd reset" command to restore the default value of set_event_pid Tzvetomir Stoyanov
@ 2019-03-11 8:33 ` Tzvetomir Stoyanov
2019-03-13 1:28 ` Steven Rostedt
2019-03-11 13:02 ` [PATCH v4 0/6] trace-cmd reset fixes Slavomir Kaslev
6 siblings, 1 reply; 9+ messages in thread
From: Tzvetomir Stoyanov @ 2019-03-11 8:33 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
This patch changes reset_max_latency() to utilize write_instance_file() for
writing set_event_pid instance file, instead of directly opening it. It also
changes the function to work per instance.
Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
tracecmd/trace-record.c | 99 ++++++++++++++++++++---------------------
1 file changed, 49 insertions(+), 50 deletions(-)
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index bdf0c02..dfbb0cd 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -814,19 +814,49 @@ static void clear_trace(void)
fclose(fp);
}
-static void reset_max_latency(void)
+static int write_file(const char *file, const char *str, const char *type)
+{
+ char buf[BUFSIZ];
+ int fd;
+ int ret;
+
+ fd = open(file, O_WRONLY | O_TRUNC);
+ if (fd < 0)
+ die("opening to '%s'", file);
+ ret = write(fd, str, strlen(str));
+ close(fd);
+ if (ret < 0 && type) {
+ /* write failed */
+ fd = open(file, O_RDONLY);
+ if (fd < 0)
+ die("writing to '%s'", file);
+ /* the filter has the error */
+ while ((ret = read(fd, buf, BUFSIZ)) > 0)
+ fprintf(stderr, "%.*s", ret, buf);
+ die("Failed %s of %s\n", type, file);
+ close(fd);
+ }
+ return ret;
+}
+
+static int
+write_instance_file(struct buffer_instance *instance,
+ const char *file, const char *str, const char *type)
{
- FILE *fp;
char *path;
+ int ret;
- /* reset the trace */
- path = tracecmd_get_tracing_file("tracing_max_latency");
- fp = fopen(path, "w");
- if (!fp)
- die("writing to '%s'", path);
+ path = get_instance_file(instance, file);
+ ret = write_file(path, str, type);
tracecmd_put_tracing_file(path);
- fwrite("0", 1, 1, fp);
- fclose(fp);
+
+ return ret;
+}
+
+static void reset_max_latency(struct buffer_instance *instance)
+{
+ write_instance_file(instance,
+ "tracing_max_latency", "0", "max_latency");
}
static void add_filter_pid(int pid, int exclude)
@@ -1094,45 +1124,6 @@ static void update_sched_events(struct buffer_instance *instance, int pid)
static int open_instance_fd(struct buffer_instance *instance,
const char *file, int flags);
-static int write_file(const char *file, const char *str, const char *type)
-{
- char buf[BUFSIZ];
- int fd;
- int ret;
-
- fd = open(file, O_WRONLY | O_TRUNC);
- if (fd < 0)
- die("opening to '%s'", file);
- ret = write(fd, str, strlen(str));
- close(fd);
- if (ret < 0 && type) {
- /* write failed */
- fd = open(file, O_RDONLY);
- if (fd < 0)
- die("writing to '%s'", file);
- /* the filter has the error */
- while ((ret = read(fd, buf, BUFSIZ)) > 0)
- fprintf(stderr, "%.*s", ret, buf);
- die("Failed %s of %s\n", type, file);
- close(fd);
- }
- return ret;
-}
-
-static int
-write_instance_file(struct buffer_instance *instance,
- const char *file, const char *str, const char *type)
-{
- char *path;
- int ret;
-
- path = get_instance_file(instance, file);
- ret = write_file(path, str, type);
- tracecmd_put_tracing_file(path);
-
- return ret;
-}
-
static void add_event_pid(const char *buf)
{
struct buffer_instance *instance;
@@ -1931,6 +1922,14 @@ static int read_tracing_on(struct buffer_instance *instance)
return ret;
}
+static void reset_max_latency_instance(void)
+{
+ struct buffer_instance *instance;
+
+ for_all_instances(instance)
+ reset_max_latency(instance);
+}
+
void tracecmd_enable_tracing(void)
{
struct buffer_instance *instance;
@@ -1941,7 +1940,7 @@ void tracecmd_enable_tracing(void)
write_tracing_on(instance, 1);
if (latency)
- reset_max_latency();
+ reset_max_latency_instance();
}
void tracecmd_disable_tracing(void)
@@ -3801,7 +3800,6 @@ static void reset_event_pid(void)
add_event_pid("");
}
-
static void clear_triggers(void)
{
struct buffer_instance *instance;
@@ -4506,6 +4504,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.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v4 0/6] trace-cmd reset fixes
2019-03-11 8:33 [PATCH v4 0/6] trace-cmd reset fixes Tzvetomir Stoyanov
` (5 preceding siblings ...)
2019-03-11 8:33 ` [PATCH v4 6/6] trace-cmd: Reafctored reset_max_latency() to utilize write_instance_file() Tzvetomir Stoyanov
@ 2019-03-11 13:02 ` Slavomir Kaslev
6 siblings, 0 replies; 9+ messages in thread
From: Slavomir Kaslev @ 2019-03-11 13:02 UTC (permalink / raw)
To: Tzvetomir Stoyanov; +Cc: Steven Rostedt, linux-trace-devel
On Mon, Mar 11, 2019 at 10:33 AM Tzvetomir Stoyanov
<tstoyanov@vmware.com> wrote:
>
> [
> V4 changes:
> - An extra patch was inluded in v3 (not realeted to trace-cmd reset fixes), by mistake.
> V3 changes:
> - Refactored reset_max_latency() and reset_max_latency_instance(),
> as Steven Rostedt suggested.
> V2 changes:
> - Fixed subjects of the patches.
> - Refactored add_event_pid() and reset_event_pid(),
> as Steven Rostedt suggested.
> ]
>
> 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 (6):
> trace-cmd: Fix "trace-cmd reset" command to restore "tracng_on"
> trace-cmd: Fix "trace-cmd reset -a -d" segfault
> trace-cmd: Fix "trace-cmd reset" command to restore default clock
> trace-cmd: Reafctored add_event_pid()to utilize write_instance_file()
> trace-cmd: Fix "trace-cmd reset" command to restore the default value
> of set_event_pid
> trace-cmd: Reafctored reset_max_latency() to utilize
> write_instance_file()
>
> tracecmd/trace-record.c | 137 ++++++++++++++++++++++------------------
> 1 file changed, 75 insertions(+), 62 deletions(-)
>
> --
> 2.20.1
>
Nit: write_file() and write_instance_file() are moved twice in patches
4 and 6. Moving them once in its new place in trace-record.c would
have made the patchset smaller and easier to review.
With that said, all patches looks good to me.
Reviewed-by: Slavomir Kaslev <kaslevs@vmware.com>
Thanks!
--
Slavomir Kaslev
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 6/6] trace-cmd: Reafctored reset_max_latency() to utilize write_instance_file()
2019-03-11 8:33 ` [PATCH v4 6/6] trace-cmd: Reafctored reset_max_latency() to utilize write_instance_file() Tzvetomir Stoyanov
@ 2019-03-13 1:28 ` Steven Rostedt
0 siblings, 0 replies; 9+ messages in thread
From: Steven Rostedt @ 2019-03-13 1:28 UTC (permalink / raw)
To: Tzvetomir Stoyanov; +Cc: linux-trace-devel
[-- Attachment #1: Type: text/plain, Size: 1943 bytes --]
On Mon, 11 Mar 2019 10:33:39 +0200
Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:
> diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
> index bdf0c02..dfbb0cd 100644
> --- a/tracecmd/trace-record.c
> +++ b/tracecmd/trace-record.c
> @@ -814,19 +814,49 @@ static void clear_trace(void)
> fclose(fp);
> }
>
> -static void reset_max_latency(void)
> +static int write_file(const char *file, const char *str, const char *type)
> +{
> + char buf[BUFSIZ];
> + int fd;
> + int ret;
> +
> + fd = open(file, O_WRONLY | O_TRUNC);
> + if (fd < 0)
> + die("opening to '%s'", file);
> + ret = write(fd, str, strlen(str));
> + close(fd);
> + if (ret < 0 && type) {
> + /* write failed */
> + fd = open(file, O_RDONLY);
> + if (fd < 0)
> + die("writing to '%s'", file);
> + /* the filter has the error */
> + while ((ret = read(fd, buf, BUFSIZ)) > 0)
> + fprintf(stderr, "%.*s", ret, buf);
> + die("Failed %s of %s\n", type, file);
> + close(fd);
> + }
> + return ret;
> +}
> +
> +static int
> +write_instance_file(struct buffer_instance *instance,
> + const char *file, const char *str, const char *type)
> {
> - FILE *fp;
> char *path;
> + int ret;
>
> - /* reset the trace */
> - path = tracecmd_get_tracing_file("tracing_max_latency");
> - fp = fopen(path, "w");
> - if (!fp)
> - die("writing to '%s'", path);
> + path = get_instance_file(instance, file);
> + ret = write_file(path, str, type);
> tracecmd_put_tracing_file(path);
> - fwrite("0", 1, 1, fp);
> - fclose(fp);
> +
> + return ret;
To make this easier to review and understand, I added a change before
patch 4 that moved write_instance_file() up further. This way, there's
a single patch that moves the code. Especially, since you moved it
twice. Sometimes its cleaner to do the move as a separate change.
Especially when you are modifying the change near the move.
I rebased the changes on top of the move, and the result is attached.
-- Steve
[-- Attachment #2: 0001-trace-cmd-Move-write_instance_file-up-in-file.patch --]
[-- Type: text/x-patch, Size: 2804 bytes --]
From cbd567febd2559b055576ec48312fa53f46f76f5 Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Date: Tue, 12 Mar 2019 21:13:19 -0400
Subject: [PATCH 1/4] trace-cmd: Move write_instance_file() up in file
To allow for reset_max_latency() and add_filter_pid() to utilize
write_instance_file(), move it up before those functions. It is in a better
location now as well.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
tracecmd/trace-record.c | 78 ++++++++++++++++++++---------------------
1 file changed, 39 insertions(+), 39 deletions(-)
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 00dc5ad7..f1f5d29d 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -776,6 +776,45 @@ get_instance_dir(struct buffer_instance *instance)
return path;
}
+static int write_file(const char *file, const char *str, const char *type)
+{
+ char buf[BUFSIZ];
+ int fd;
+ int ret;
+
+ fd = open(file, O_WRONLY | O_TRUNC);
+ if (fd < 0)
+ die("opening to '%s'", file);
+ ret = write(fd, str, strlen(str));
+ close(fd);
+ if (ret < 0 && type) {
+ /* write failed */
+ fd = open(file, O_RDONLY);
+ if (fd < 0)
+ die("writing to '%s'", file);
+ /* the filter has the error */
+ while ((ret = read(fd, buf, BUFSIZ)) > 0)
+ fprintf(stderr, "%.*s", ret, buf);
+ die("Failed %s of %s\n", type, file);
+ close(fd);
+ }
+ return ret;
+}
+
+static int
+write_instance_file(struct buffer_instance *instance,
+ const char *file, const char *str, const char *type)
+{
+ char *path;
+ int ret;
+
+ path = get_instance_file(instance, file);
+ ret = write_file(path, str, type);
+ tracecmd_put_tracing_file(path);
+
+ return ret;
+}
+
static void __clear_trace(struct buffer_instance *instance)
{
FILE *fp;
@@ -1596,45 +1635,6 @@ static void reset_events(void)
reset_events_instance(instance);
}
-static int write_file(const char *file, const char *str, const char *type)
-{
- char buf[BUFSIZ];
- int fd;
- int ret;
-
- fd = open(file, O_WRONLY | O_TRUNC);
- if (fd < 0)
- die("opening to '%s'", file);
- ret = write(fd, str, strlen(str));
- close(fd);
- if (ret < 0 && type) {
- /* write failed */
- fd = open(file, O_RDONLY);
- if (fd < 0)
- die("writing to '%s'", file);
- /* the filter has the error */
- while ((ret = read(fd, buf, BUFSIZ)) > 0)
- fprintf(stderr, "%.*s", ret, buf);
- die("Failed %s of %s\n", type, file);
- close(fd);
- }
- return ret;
-}
-
-static int
-write_instance_file(struct buffer_instance *instance,
- const char *file, const char *str, const char *type)
-{
- char *path;
- int ret;
-
- path = get_instance_file(instance, file);
- ret = write_file(path, str, type);
- tracecmd_put_tracing_file(path);
-
- return ret;
-}
-
enum {
STATE_NEWLINE,
STATE_SKIP,
--
2.20.1
[-- Attachment #3: 0002-trace-cmd-Refactore-add_event_pid-to-utilize-write_i.patch --]
[-- Type: text/x-patch, Size: 1911 bytes --]
From cd5a23168280917774094eeb77b5a359e4f4a292 Mon Sep 17 00:00:00 2001
From: Tzvetomir Stoyanov <tstoyanov@vmware.com>
Date: Mon, 11 Mar 2019 10:33:37 +0200
Subject: [PATCH 2/4] trace-cmd: Refactore add_event_pid()to utilize
write_instance_file()
This patch changes add_event_pid() to utilize write_instance_file() for
writing set_event_pid instance file, instead of directly opening it.
Link: http://lore.kernel.org/linux-trace-devel/20190311083339.21581-5-tstoyanov@vmware.com
Reviewed-by: Slavomir Kaslev <kaslevs@vmware.com>
Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
tracecmd/trace-record.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index f1f5d29d..801dad24 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -1133,30 +1133,25 @@ static void update_sched_events(struct buffer_instance *instance, int pid)
static int open_instance_fd(struct buffer_instance *instance,
const char *file, int flags);
-static void add_event_pid(const char *buf, int len)
+static void add_event_pid(const char *buf)
{
struct buffer_instance *instance;
- int fd;
- for_all_instances(instance) {
- fd = open_instance_fd(instance, "set_event_pid", O_WRONLY);
- write(fd, buf, len);
- close(fd);
- }
+ for_all_instances(instance)
+ write_instance_file(instance, "set_event_pid", buf, "event_pid");
}
static void add_new_filter_pid(int pid)
{
struct buffer_instance *instance;
char buf[100];
- int len;
add_filter_pid(pid, 0);
- len = sprintf(buf, "%d", pid);
+ sprintf(buf, "%d", pid);
update_ftrace_pid(buf, 0);
if (have_set_event_pid)
- return add_event_pid(buf, len);
+ return add_event_pid(buf);
common_pid_filter = append_pid_filter(common_pid_filter, "common_pid", pid);
--
2.20.1
[-- Attachment #4: 0004-trace-cmd-Refactore-reset_max_latency-to-utilize-wri.patch --]
[-- Type: text/x-patch, Size: 2507 bytes --]
From 7b4254303277ee6c5833f427fea7f00fa161bd74 Mon Sep 17 00:00:00 2001
From: Tzvetomir Stoyanov <tstoyanov@vmware.com>
Date: Mon, 11 Mar 2019 10:33:39 +0200
Subject: [PATCH 4/4] trace-cmd: Refactore reset_max_latency() to utilize
write_instance_file()
This patch changes reset_max_latency() to utilize write_instance_file() for
writing set_event_pid instance file, instead of directly opening it. It also
changes the function to work per instance.
Link: http://lore.kernel.org/linux-trace-devel/20190311083339.21581-7-tstoyanov@vmware.com
Reviewed-by: Slavomir Kaslev <kaslevs@vmware.com>
Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
tracecmd/trace-record.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index fe22ad08..c16abe2f 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -853,19 +853,10 @@ static void clear_trace(void)
fclose(fp);
}
-static void reset_max_latency(void)
+static void reset_max_latency(struct buffer_instance *instance)
{
- FILE *fp;
- char *path;
-
- /* reset the trace */
- path = tracecmd_get_tracing_file("tracing_max_latency");
- fp = fopen(path, "w");
- if (!fp)
- die("writing to '%s'", path);
- tracecmd_put_tracing_file(path);
- fwrite("0", 1, 1, fp);
- fclose(fp);
+ write_instance_file(instance,
+ "tracing_max_latency", "0", "max_latency");
}
static void add_filter_pid(int pid, int exclude)
@@ -1931,6 +1922,14 @@ static int read_tracing_on(struct buffer_instance *instance)
return ret;
}
+static void reset_max_latency_instance(void)
+{
+ struct buffer_instance *instance;
+
+ for_all_instances(instance)
+ reset_max_latency(instance);
+}
+
void tracecmd_enable_tracing(void)
{
struct buffer_instance *instance;
@@ -1941,7 +1940,7 @@ void tracecmd_enable_tracing(void)
write_tracing_on(instance, 1);
if (latency)
- reset_max_latency();
+ reset_max_latency_instance();
}
void tracecmd_disable_tracing(void)
@@ -3802,7 +3801,6 @@ static void reset_event_pid(void)
add_event_pid("");
}
-
static void clear_triggers(void)
{
struct buffer_instance *instance;
@@ -4507,6 +4505,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.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-03-13 1:28 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-11 8:33 [PATCH v4 0/6] trace-cmd reset fixes Tzvetomir Stoyanov
2019-03-11 8:33 ` [PATCH v4 1/6] trace-cmd: Fix "trace-cmd reset" command to restore "tracng_on" Tzvetomir Stoyanov
2019-03-11 8:33 ` [PATCH v4 2/6] trace-cmd: Fix "trace-cmd reset -a -d" segfault Tzvetomir Stoyanov
2019-03-11 8:33 ` [PATCH v4 3/6] trace-cmd: Fix "trace-cmd reset" command to restore default clock Tzvetomir Stoyanov
2019-03-11 8:33 ` [PATCH v4 4/6] trace-cmd: Reafctored add_event_pid()to utilize write_instance_file() Tzvetomir Stoyanov
2019-03-11 8:33 ` [PATCH v4 5/6] trace-cmd: Fix "trace-cmd reset" command to restore the default value of set_event_pid Tzvetomir Stoyanov
2019-03-11 8:33 ` [PATCH v4 6/6] trace-cmd: Reafctored reset_max_latency() to utilize write_instance_file() Tzvetomir Stoyanov
2019-03-13 1:28 ` Steven Rostedt
2019-03-11 13:02 ` [PATCH v4 0/6] trace-cmd reset fixes Slavomir Kaslev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).