2013-04-05 01:53:51

by Yoshihiro YUNOMAE

[permalink] [raw]
Subject: [PATCH 0/2] trace-cmd: Support a raw format for outputting timestamp

Hi Steven,

This patch series extracts the trace_clock file for switching outputting format
of timestamp because in tsc or counter modes, trace-cmd should output timestamp
not in the sec.usec format but in the raw format.

For example, we will show results in trace-cmd report mode as follows by
applying this patch series:

<for local or global mode>
bash-9022 [000] 34984.109846: sched_wakeup: comm=migration/3 pid=23 prio=0 success=1 target_cpu=003
<idle>-0 [002] 34984.109847: cpu_idle: state=4294967295 cpu_id=2
bash-9022 [000] 34984.109848: sched_switch: prev_comm=bash prev_pid=9022 prev_prio=120 prev_state=R ==> next_comm=migration/0 next_pid=8 next_prio=0
<idle>-0 [002] 34984.109849: sched_switch: prev_comm=swapper/2 prev_pid=0 prev_prio=120 prev_state=R ==> next_comm=migration/2 next_pid=18 next_prio=0

<for tsc or counter mode>
bash-9022 [000] 34984109846: sched_wakeup: comm=migration/3 pid=23 prio=0 success=1 target_cpu=003
<idle>-0 [002] 34984109847: cpu_idle: state=4294967295 cpu_id=2
bash-9022 [000] 34984109848: sched_switch: prev_comm=bash prev_pid=9022 prev_prio=120 prev_state=R ==> next_comm=migration/0 next_pid=8 next_prio=0
<idle>-0 [002] 34984109849: sched_switch: prev_comm=swapper/2 prev_pid=0 prev_prio=120 prev_state=R ==> next_comm=migration/2 next_pid=18 next_prio=0

Would you apply this patch series for trace-cmd?

Thank you,

---

Yoshihiro YUNOMAE (2):
trace-cmd: Define general functions for outputting/inputting saved_cmdlines
trace-cmd: Support trace_clock extraction


event-parse.c | 44 ++++++++++++++++++++++++++++--------
event-parse.h | 3 ++
trace-cmd.h | 1 +
trace-input.c | 61 ++++++++++++++++++++++++++++++++++++++++----------
trace-output.c | 68 ++++++++++++++++++++++++++++++++++----------------------
trace-util.c | 26 +++++++++++++++++++++
6 files changed, 153 insertions(+), 50 deletions(-)

--
Yoshihiro YUNOMAE
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]


2013-04-05 01:53:42

by Yoshihiro YUNOMAE

[permalink] [raw]
Subject: [PATCH 1/2] trace-cmd: Define general functions for outputting/inputting saved_cmdlines

Currently, trace-cmd outputs data of saved_cmdlines to a trace.dat file
in create_file_fd() and inputs the data from the file in tracecmd_init_data().
On the other hand, trace-cmd will also output and input data of trace_clock in
those functions in the patch "trace-cmd: Support trace_clock extraction".
The source code of the output/input of saved_cmdlines data can be reused when
extract trace_clock, so we define general functions for outputting/inputting a
file on debugfs.

Signed-off-by: Yoshihiro YUNOMAE <[email protected]>
---
trace-input.c | 45 +++++++++++++++++++++++++++++------------
trace-output.c | 62 ++++++++++++++++++++++++++++++++------------------------
2 files changed, 67 insertions(+), 40 deletions(-)

diff --git a/trace-input.c b/trace-input.c
index 6f60409..ba3a21e 100644
--- a/trace-input.c
+++ b/trace-input.c
@@ -1758,6 +1758,37 @@ static int handle_options(struct tracecmd_input *handle)
return 0;
}

+static int read_data_and_size(struct tracecmd_input *handle,
+ char **data, unsigned long long *size)
+{
+ *size = read8(handle);
+ if (*size < 0)
+ return -1;
+ *data = malloc(*size + 1);
+ if (!*data)
+ return -1;
+ if (do_read_check(handle, *data, *size)) {
+ free(*data);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int read_and_parse_cmdlines(struct tracecmd_input *handle,
+ struct pevent *pevent)
+{
+ unsigned long long size;
+ char *cmdlines;
+
+ if (read_data_and_size(handle, &cmdlines, &size) < 0)
+ return -1;
+ cmdlines[size] = 0;
+ parse_cmdlines(pevent, cmdlines, size);
+ free(cmdlines);
+ return 0;
+}
+
/**
* tracecmd_init_data - prepare reading the data from trace.dat
* @handle: input handle for the trace.dat file
@@ -1771,23 +1802,11 @@ int tracecmd_init_data(struct tracecmd_input *handle)
enum kbuffer_long_size long_size;
enum kbuffer_endian endian;
unsigned long long size;
- char *cmdlines;
char buf[10];
int cpu;

- size = read8(handle);
- if (size < 0)
- return -1;
- cmdlines = malloc(size + 1);
- if (!cmdlines)
+ if (read_and_parse_cmdlines(handle, pevent) < 0)
return -1;
- if (do_read_check(handle, cmdlines, size)) {
- free(cmdlines);
- return -1;
- }
- cmdlines[size] = 0;
- parse_cmdlines(pevent, cmdlines, size);
- free(cmdlines);

handle->cpus = read4(handle);
if (handle->cpus < 0)
diff --git a/trace-output.c b/trace-output.c
index c4d668d..e0d4ff4 100644
--- a/trace-output.c
+++ b/trace-output.c
@@ -686,6 +686,39 @@ static int read_ftrace_printk(struct tracecmd_output *handle)
return -1;
}

+static int save_tracing_file_data(struct tracecmd_output *handle,
+ const char *filename)
+{
+ unsigned long long endian8;
+ char *file = NULL;
+ struct stat st;
+ off64_t check_size;
+ off64_t size;
+ int ret;
+
+ file = get_tracing_file(handle, filename);
+ ret = stat(file, &st);
+ if (ret >= 0) {
+ size = get_size(file);
+ endian8 = convert_endian_8(handle, size);
+ if (do_write_check(handle, &endian8, 8))
+ return -1;
+ check_size = copy_file(handle, file);
+ if (size != check_size) {
+ errno = EINVAL;
+ warning("error in size of file '%s'", file);
+ return -1;
+ }
+ } else {
+ size = 0;
+ endian8 = convert_endian_8(handle, size);
+ if (do_write_check(handle, &endian8, 8))
+ return -1;
+ }
+ put_tracing_file(file);
+ return 0;
+}
+
static struct tracecmd_output *
create_file_fd(int fd, struct tracecmd_input *ihandle,
const char *tracing_dir,
@@ -693,15 +726,9 @@ create_file_fd(int fd, struct tracecmd_input *ihandle,
struct tracecmd_event_list *list)
{
struct tracecmd_output *handle;
- unsigned long long endian8;
struct pevent *pevent;
char buf[BUFSIZ];
- char *file = NULL;
- struct stat st;
- off64_t check_size;
- off64_t size;
int endian4;
- int ret;

handle = malloc(sizeof(*handle));
if (!handle)
@@ -774,27 +801,8 @@ create_file_fd(int fd, struct tracecmd_input *ihandle,
/*
* Save the command lines;
*/
- file = get_tracing_file(handle, "saved_cmdlines");
- ret = stat(file, &st);
- if (ret >= 0) {
- size = get_size(file);
- endian8 = convert_endian_8(handle, size);
- if (do_write_check(handle, &endian8, 8))
- goto out_free;
- check_size = copy_file(handle, file);
- if (size != check_size) {
- errno = EINVAL;
- warning("error in size of file '%s'", file);
- goto out_free;
- }
- } else {
- size = 0;
- endian8 = convert_endian_8(handle, size);
- if (do_write_check(handle, &endian8, 8))
- goto out_free;
- }
- put_tracing_file(file);
- file = NULL;
+ if (save_tracing_file_data(handle, "saved_cmdlines") < 0)
+ goto out_free;

return handle;

2013-04-05 01:53:47

by Yoshihiro YUNOMAE

[permalink] [raw]
Subject: [PATCH 2/2] trace-cmd: Support trace_clock extraction

In this patch, trace-cmd reads trace_clock on debugfs in the report/extract
modes and outputs the data to trace.dat file. Then, in the report mode,
trace-cmd reads trace_clock data from the file and switches outputting format
of timestamp for each trace_clock.

Note that by applying this patch, the binary format of trace.data is changed
as follows:

<Current binary format>
...
[size of saved_cmdlines]
[saved_cmdlines contents]
[total cpu number]
...

<Changed binary format>
...
[size of saved_cmdlines]
[saved_cmdlines contents]
[size of trace_clock] <== add
[trace_clock contents] <== add
[total cpu number]
...

Signed-off-by: Yoshihiro YUNOMAE <[email protected]>
---
event-parse.c | 44 ++++++++++++++++++++++++++++++++++----------
event-parse.h | 3 +++
trace-cmd.h | 1 +
trace-input.c | 16 ++++++++++++++++
trace-output.c | 6 ++++++
trace-util.c | 26 ++++++++++++++++++++++++++
6 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/event-parse.c b/event-parse.c
index 05c1412..ff39d63 100644
--- a/event-parse.c
+++ b/event-parse.c
@@ -23,6 +23,7 @@
* Frederic Weisbecker gave his permission to relicense the code to
* the Lesser General Public License.
*/
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -305,6 +306,11 @@ int pevent_register_comm(struct pevent *pevent, const char *comm, int pid)
return 0;
}

+void pevent_register_trace_clock(struct pevent *pevent, char *trace_clock)
+{
+ pevent->trace_clock = trace_clock;
+}
+
struct func_map {
unsigned long long addr;
char *func;
@@ -4404,6 +4410,15 @@ void pevent_event_info(struct trace_seq *s, struct event_format *event,
trace_seq_terminate(s);
}

+static bool is_timestamp_in_ns(char *trace_clock)
+{
+ if (!strcmp(trace_clock, "local") || !strcmp(trace_clock, "global"))
+ return true;
+
+ /* trace_clock is setting in tsc or counter mode */
+ return false;
+}
+
void pevent_print_event(struct pevent *pevent, struct trace_seq *s,
struct pevent_record *record)
{
@@ -4418,9 +4433,13 @@ void pevent_print_event(struct pevent *pevent, struct trace_seq *s,
int pid;
int len;
int p;
+ bool use_sec_format;

- secs = record->ts / NSECS_PER_SEC;
- nsecs = record->ts - secs * NSECS_PER_SEC;
+ use_sec_format = is_timestamp_in_ns(pevent->trace_clock);
+ if (use_sec_format) {
+ secs = record->ts / NSECS_PER_SEC;
+ nsecs = record->ts - secs * NSECS_PER_SEC;
+ }

if (record->size < 0) {
do_warning("ug! negative record size %d", record->size);
@@ -4445,15 +4464,20 @@ void pevent_print_event(struct pevent *pevent, struct trace_seq *s,
} else
trace_seq_printf(s, "%16s-%-5d [%03d]", comm, pid, record->cpu);

- if (pevent->flags & PEVENT_NSEC_OUTPUT) {
- usecs = nsecs;
- p = 9;
- } else {
- usecs = (nsecs + 500) / NSECS_PER_USEC;
- p = 6;
- }
+ if (use_sec_format) {
+ if (pevent->flags & PEVENT_NSEC_OUTPUT) {
+ usecs = nsecs;
+ p = 9;
+ } else {
+ usecs = (nsecs + 500) / NSECS_PER_USEC;
+ p = 6;
+ }

- trace_seq_printf(s, " %5lu.%0*lu: %s: ", secs, p, usecs, event->name);
+ trace_seq_printf(s, " %5lu.%0*lu: %s: ",
+ secs, p, usecs, event->name);
+ } else
+ trace_seq_printf(s, " %12llu: %s: ",
+ record->ts, event->name);

/* Space out the event names evenly. */
len = strlen(event->name);
diff --git a/event-parse.h b/event-parse.h
index de31909..540095f 100644
--- a/event-parse.h
+++ b/event-parse.h
@@ -456,6 +456,8 @@ struct pevent {

/* cache */
struct event_format *last_event;
+
+ char *trace_clock;
};

static inline void pevent_set_flag(struct pevent *pevent, int flag)
@@ -533,6 +535,7 @@ enum trace_flag_type {
};

int pevent_register_comm(struct pevent *pevent, const char *comm, int pid);
+void pevent_register_trace_clock(struct pevent *pevent, char *trace_clock);
int pevent_register_function(struct pevent *pevent, char *name,
unsigned long long addr, char *mod);
int pevent_register_print_string(struct pevent *pevent, char *fmt,
diff --git a/trace-cmd.h b/trace-cmd.h
index 37c037e..29cce65 100644
--- a/trace-cmd.h
+++ b/trace-cmd.h
@@ -30,6 +30,7 @@
#define TRACECMD_PTR2ERR(ptr) ((unisgned long)(ptr) & ~TRACECMD_ERR_MSK)

void parse_cmdlines(struct pevent *pevent, char *file, int size);
+void parse_trace_clock(struct pevent *pevent, char *file, int size);
void parse_proc_kallsyms(struct pevent *pevent, char *file, unsigned int size);
void parse_ftrace_printk(struct pevent *pevent, char *file, unsigned int size);

diff --git a/trace-input.c b/trace-input.c
index ba3a21e..92791d8 100644
--- a/trace-input.c
+++ b/trace-input.c
@@ -1789,6 +1789,20 @@ static int read_and_parse_cmdlines(struct tracecmd_input *handle,
return 0;
}

+static int read_and_parse_trace_clock(struct tracecmd_input *handle,
+ struct pevent *pevent)
+{
+ unsigned long long size;
+ char *trace_clock;
+
+ if (read_data_and_size(handle, &trace_clock, &size) < 0)
+ return -1;
+ trace_clock[size] = 0;
+ parse_trace_clock(pevent, trace_clock, size);
+ free(trace_clock);
+ return 0;
+}
+
/**
* tracecmd_init_data - prepare reading the data from trace.dat
* @handle: input handle for the trace.dat file
@@ -1807,6 +1821,8 @@ int tracecmd_init_data(struct tracecmd_input *handle)

if (read_and_parse_cmdlines(handle, pevent) < 0)
return -1;
+ if (read_and_parse_trace_clock(handle, pevent) < 0)
+ return -1;

handle->cpus = read4(handle);
if (handle->cpus < 0)
diff --git a/trace-output.c b/trace-output.c
index e0d4ff4..0d858a9 100644
--- a/trace-output.c
+++ b/trace-output.c
@@ -804,6 +804,12 @@ create_file_fd(int fd, struct tracecmd_input *ihandle,
if (save_tracing_file_data(handle, "saved_cmdlines") < 0)
goto out_free;

+ /*
+ * Save the current timestamp mode;
+ */
+ if (save_tracing_file_data(handle, "trace_clock") < 0)
+ goto out_free;
+
return handle;

out_free:
diff --git a/trace-util.c b/trace-util.c
index 9b26d1f..d6c1e29 100644
--- a/trace-util.c
+++ b/trace-util.c
@@ -375,6 +375,32 @@ void parse_cmdlines(struct pevent *pevent,
}
}

+static void extract_trace_clock(struct pevent *pevent, char *line)
+{
+ char *data;
+ char *clock;
+ char *next = NULL;
+
+ data = strtok_r(line, "[]", &next);
+ sscanf(data, "%ms", &clock);
+ pevent_register_trace_clock(pevent, clock);
+}
+
+void parse_trace_clock(struct pevent *pevent,
+ char *file, int size __maybe_unused)
+{
+ char *line;
+ char *next = NULL;
+
+ line = strtok_r(file, " ", &next);
+ while (line) {
+ /* current trace_clock is shown as "[local]". */
+ if (*line == '[')
+ return extract_trace_clock(pevent, line);
+ line = strtok_r(NULL, " ", &next);
+ }
+}
+
void parse_proc_kallsyms(struct pevent *pevent,
char *file, unsigned int size __maybe_unused)
{

2013-04-09 16:00:41

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 2/2] trace-cmd: Support trace_clock extraction

On Fri, 2013-04-05 at 10:54 +0900, Yoshihiro YUNOMAE wrote:
> In this patch, trace-cmd reads trace_clock on debugfs in the report/extract
> modes and outputs the data to trace.dat file. Then, in the report mode,
> trace-cmd reads trace_clock data from the file and switches outputting format
> of timestamp for each trace_clock.
>
> Note that by applying this patch, the binary format of trace.data is changed
> as follows:
>
> <Current binary format>
> ...
> [size of saved_cmdlines]
> [saved_cmdlines contents]
> [total cpu number]
> ...
>
> <Changed binary format>
> ...
> [size of saved_cmdlines]
> [saved_cmdlines contents]
> [size of trace_clock] <== add
> [trace_clock contents] <== add
> [total cpu number]
> ...

Hi Yoshihiro,

I don't mind the feature addition, but I don't like the implementation.
This will break backward compatibility with the trace.dat file, which
I've been working hard not to have happen.

That doesn't mean that you can't implement this feature. This is what
the options section is for. You can add an option that includes the
trace_clock contents, or just simply say what type of clock is being
used. Then a new trace-cmd can show the output like you want it to, and
the old version will still work but show the old way, as trace-cmd is
made to simply ignore options it does not know about.

I think it's time for me to push my latest updates to trace-cmd. As this
will probably end up being the 3.0 version. I have examples there that
use the options feature for more extensions that you can look at.

Thanks,

-- Steve

>
> Signed-off-by: Yoshihiro YUNOMAE <[email protected]>
> ---
> event-parse.c | 44 ++++++++++++++++++++++++++++++++++----------
> event-parse.h | 3 +++
> trace-cmd.h | 1 +
> trace-input.c | 16 ++++++++++++++++
> trace-output.c | 6 ++++++
> trace-util.c | 26 ++++++++++++++++++++++++++
> 6 files changed, 86 insertions(+), 10 deletions(-)
>
> diff --git a/event-parse.c b/event-parse.c
> index 05c1412..ff39d63 100644
> --- a/event-parse.c
> +++ b/event-parse.c
> @@ -23,6 +23,7 @@
> * Frederic Weisbecker gave his permission to relicense the code to
> * the Lesser General Public License.
> */
> +#include <stdbool.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> @@ -305,6 +306,11 @@ int pevent_register_comm(struct pevent *pevent, const char *comm, int pid)
> return 0;
> }
>
> +void pevent_register_trace_clock(struct pevent *pevent, char *trace_clock)
> +{
> + pevent->trace_clock = trace_clock;
> +}
> +
> struct func_map {
> unsigned long long addr;
> char *func;
> @@ -4404,6 +4410,15 @@ void pevent_event_info(struct trace_seq *s, struct event_format *event,
> trace_seq_terminate(s);
> }
>
> +static bool is_timestamp_in_ns(char *trace_clock)
> +{
> + if (!strcmp(trace_clock, "local") || !strcmp(trace_clock, "global"))
> + return true;
> +
> + /* trace_clock is setting in tsc or counter mode */
> + return false;
> +}
> +
> void pevent_print_event(struct pevent *pevent, struct trace_seq *s,
> struct pevent_record *record)
> {
> @@ -4418,9 +4433,13 @@ void pevent_print_event(struct pevent *pevent, struct trace_seq *s,
> int pid;
> int len;
> int p;
> + bool use_sec_format;
>
> - secs = record->ts / NSECS_PER_SEC;
> - nsecs = record->ts - secs * NSECS_PER_SEC;
> + use_sec_format = is_timestamp_in_ns(pevent->trace_clock);
> + if (use_sec_format) {
> + secs = record->ts / NSECS_PER_SEC;
> + nsecs = record->ts - secs * NSECS_PER_SEC;
> + }
>
> if (record->size < 0) {
> do_warning("ug! negative record size %d", record->size);
> @@ -4445,15 +4464,20 @@ void pevent_print_event(struct pevent *pevent, struct trace_seq *s,
> } else
> trace_seq_printf(s, "%16s-%-5d [%03d]", comm, pid, record->cpu);
>
> - if (pevent->flags & PEVENT_NSEC_OUTPUT) {
> - usecs = nsecs;
> - p = 9;
> - } else {
> - usecs = (nsecs + 500) / NSECS_PER_USEC;
> - p = 6;
> - }
> + if (use_sec_format) {
> + if (pevent->flags & PEVENT_NSEC_OUTPUT) {
> + usecs = nsecs;
> + p = 9;
> + } else {
> + usecs = (nsecs + 500) / NSECS_PER_USEC;
> + p = 6;
> + }
>
> - trace_seq_printf(s, " %5lu.%0*lu: %s: ", secs, p, usecs, event->name);
> + trace_seq_printf(s, " %5lu.%0*lu: %s: ",
> + secs, p, usecs, event->name);
> + } else
> + trace_seq_printf(s, " %12llu: %s: ",
> + record->ts, event->name);
>
> /* Space out the event names evenly. */
> len = strlen(event->name);
> diff --git a/event-parse.h b/event-parse.h
> index de31909..540095f 100644
> --- a/event-parse.h
> +++ b/event-parse.h
> @@ -456,6 +456,8 @@ struct pevent {
>
> /* cache */
> struct event_format *last_event;
> +
> + char *trace_clock;
> };
>
> static inline void pevent_set_flag(struct pevent *pevent, int flag)
> @@ -533,6 +535,7 @@ enum trace_flag_type {
> };
>
> int pevent_register_comm(struct pevent *pevent, const char *comm, int pid);
> +void pevent_register_trace_clock(struct pevent *pevent, char *trace_clock);
> int pevent_register_function(struct pevent *pevent, char *name,
> unsigned long long addr, char *mod);
> int pevent_register_print_string(struct pevent *pevent, char *fmt,
> diff --git a/trace-cmd.h b/trace-cmd.h
> index 37c037e..29cce65 100644
> --- a/trace-cmd.h
> +++ b/trace-cmd.h
> @@ -30,6 +30,7 @@
> #define TRACECMD_PTR2ERR(ptr) ((unisgned long)(ptr) & ~TRACECMD_ERR_MSK)
>
> void parse_cmdlines(struct pevent *pevent, char *file, int size);
> +void parse_trace_clock(struct pevent *pevent, char *file, int size);
> void parse_proc_kallsyms(struct pevent *pevent, char *file, unsigned int size);
> void parse_ftrace_printk(struct pevent *pevent, char *file, unsigned int size);
>
> diff --git a/trace-input.c b/trace-input.c
> index ba3a21e..92791d8 100644
> --- a/trace-input.c
> +++ b/trace-input.c
> @@ -1789,6 +1789,20 @@ static int read_and_parse_cmdlines(struct tracecmd_input *handle,
> return 0;
> }
>
> +static int read_and_parse_trace_clock(struct tracecmd_input *handle,
> + struct pevent *pevent)
> +{
> + unsigned long long size;
> + char *trace_clock;
> +
> + if (read_data_and_size(handle, &trace_clock, &size) < 0)
> + return -1;
> + trace_clock[size] = 0;
> + parse_trace_clock(pevent, trace_clock, size);
> + free(trace_clock);
> + return 0;
> +}
> +
> /**
> * tracecmd_init_data - prepare reading the data from trace.dat
> * @handle: input handle for the trace.dat file
> @@ -1807,6 +1821,8 @@ int tracecmd_init_data(struct tracecmd_input *handle)
>
> if (read_and_parse_cmdlines(handle, pevent) < 0)
> return -1;
> + if (read_and_parse_trace_clock(handle, pevent) < 0)
> + return -1;
>
> handle->cpus = read4(handle);
> if (handle->cpus < 0)
> diff --git a/trace-output.c b/trace-output.c
> index e0d4ff4..0d858a9 100644
> --- a/trace-output.c
> +++ b/trace-output.c
> @@ -804,6 +804,12 @@ create_file_fd(int fd, struct tracecmd_input *ihandle,
> if (save_tracing_file_data(handle, "saved_cmdlines") < 0)
> goto out_free;
>
> + /*
> + * Save the current timestamp mode;
> + */
> + if (save_tracing_file_data(handle, "trace_clock") < 0)
> + goto out_free;
> +
> return handle;
>
> out_free:
> diff --git a/trace-util.c b/trace-util.c
> index 9b26d1f..d6c1e29 100644
> --- a/trace-util.c
> +++ b/trace-util.c
> @@ -375,6 +375,32 @@ void parse_cmdlines(struct pevent *pevent,
> }
> }
>
> +static void extract_trace_clock(struct pevent *pevent, char *line)
> +{
> + char *data;
> + char *clock;
> + char *next = NULL;
> +
> + data = strtok_r(line, "[]", &next);
> + sscanf(data, "%ms", &clock);
> + pevent_register_trace_clock(pevent, clock);
> +}
> +
> +void parse_trace_clock(struct pevent *pevent,
> + char *file, int size __maybe_unused)
> +{
> + char *line;
> + char *next = NULL;
> +
> + line = strtok_r(file, " ", &next);
> + while (line) {
> + /* current trace_clock is shown as "[local]". */
> + if (*line == '[')
> + return extract_trace_clock(pevent, line);
> + line = strtok_r(NULL, " ", &next);
> + }
> +}
> +
> void parse_proc_kallsyms(struct pevent *pevent,
> char *file, unsigned int size __maybe_unused)
> {

2013-04-09 16:07:17

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 2/2] trace-cmd: Support trace_clock extraction

On Tue, 2013-04-09 at 12:00 -0400, Steven Rostedt wrote:

> I think it's time for me to push my latest updates to trace-cmd. As this
> will probably end up being the 3.0 version. I have examples there that
> use the options feature for more extensions that you can look at.
>

I just pushed my latest changes. Specifically look at these:

commit d56f30679f9811a91ed471c8e081cc7ffbed1e62
Author: Steven Rostedt (Red Hat) <[email protected]>
Date: Wed Mar 6 11:31:39 2013 -0500

trace-cmd: Add recording to buffer instances


commit 417c903f3b91e012ee0dfedaf0bb8e5cc82780e1
Author: Steven Rostedt (Red Hat) <[email protected]>
Date: Wed Mar 6 16:12:16 2013 -0500

trace-cmd: Add support for multi-buffers in report


It allows trace-cmd to record multiple buffers into the trace.dat file
using the options. Old trace-cmd can still read this trace.dat file, but
it will just not report the multiple buffers. But it wont break, and
will still report the main buffers.

-- Steve


2013-04-11 10:09:06

by Yoshihiro YUNOMAE

[permalink] [raw]
Subject: Re: Re: [PATCH 2/2] trace-cmd: Support trace_clock extraction

Hi Steven,

Thank you for reviewing my patch set.

(2013/04/10 1:00), Steven Rostedt wrote:
> On Fri, 2013-04-05 at 10:54 +0900, Yoshihiro YUNOMAE wrote:
>> In this patch, trace-cmd reads trace_clock on debugfs in the report/extract
>> modes and outputs the data to trace.dat file. Then, in the report mode,
>> trace-cmd reads trace_clock data from the file and switches outputting format
>> of timestamp for each trace_clock.
>>
>> Note that by applying this patch, the binary format of trace.data is changed
>> as follows:
>>
>> <Current binary format>
>> ...
>> [size of saved_cmdlines]
>> [saved_cmdlines contents]
>> [total cpu number]
>> ...
>>
>> <Changed binary format>
>> ...
>> [size of saved_cmdlines]
>> [saved_cmdlines contents]
>> [size of trace_clock] <== add
>> [trace_clock contents] <== add
>> [total cpu number]
>> ...
>
> Hi Yoshihiro,
>
> I don't mind the feature addition, but I don't like the implementation.
> This will break backward compatibility with the trace.dat file, which
> I've been working hard not to have happen.

Yeah, I agree with you.
It is important that trace-cmd keeps backward compatibility.

> That doesn't mean that you can't implement this feature. This is what
> the options section is for. You can add an option that includes the
> trace_clock contents, or just simply say what type of clock is being
> used. Then a new trace-cmd can show the output like you want it to, and
> the old version will still work but show the old way, as trace-cmd is
> made to simply ignore options it does not know about.

Sounds good.
Maintenance of the implementation will be easier than that of my
implementation.

> I think it's time for me to push my latest updates to trace-cmd. As this
> will probably end up being the 3.0 version. I have examples there that
> use the options feature for more extensions that you can look at.

Thank you for pushing your latest patches!
I'll use your patches as a reference and submit the revised patches
using the options feature.

Thanks,
Yoshihiro YUNOMAE

--
Yoshihiro YUNOMAE
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]