Linux-Trace-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] Split reading the trace.dat options from trace data
@ 2020-04-09 13:28 Tzvetomir Stoyanov (VMware)
  2020-04-09 13:28 ` [PATCH 1/2] trace-cmd: Move reading of trace.dat options to tracecmd_read_headers() Tzvetomir Stoyanov (VMware)
  2020-04-09 13:28 ` [PATCH 2/2] trace-cmd: Add new API tracecmd_open_head() Tzvetomir Stoyanov (VMware)
  0 siblings, 2 replies; 5+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-04-09 13:28 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Add functionality which allows to implement opening a trace file on
stages - reading the trace headers and reading the trace data.

Tzvetomir Stoyanov (VMware) (2):
  trace-cmd: Move reading of trace.dat options to
    tracecmd_read_headers()
  trace-cmd: Add new API tracecmd_open_head()

 include/trace-cmd/trace-cmd.h |   2 +
 lib/trace-cmd/trace-input.c   | 117 +++++++++++++++++++++++++---------
 2 files changed, 90 insertions(+), 29 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] trace-cmd: Move reading of trace.dat options to tracecmd_read_headers()
  2020-04-09 13:28 [PATCH 0/2] Split reading the trace.dat options from trace data Tzvetomir Stoyanov (VMware)
@ 2020-04-09 13:28 ` Tzvetomir Stoyanov (VMware)
  2020-04-09 13:28 ` [PATCH 2/2] trace-cmd: Add new API tracecmd_open_head() Tzvetomir Stoyanov (VMware)
  1 sibling, 0 replies; 5+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-04-09 13:28 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

The options section from trace.dat file is logically part of trace headers.
In some use cases it is useful first to read all headers from the file,
analyze the information and do some processing, before reading the tracing
data. In the current implementation, reading of options is just before
reading the tracing data.
Moved the reading of options and a CPU count from read_cpu_data() to
tracecmd_read_headers(). This allows to implement APIs for trace.dat
file reading on stages - headers stage and tracing data stage.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h |  1 +
 lib/trace-cmd/trace-input.c   | 89 +++++++++++++++++++++++------------
 2 files changed, 61 insertions(+), 29 deletions(-)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 595919d5..3f96bbde 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -119,6 +119,7 @@ enum {
 	TRACECMD_FL_BUFFER_INSTANCE	= (1 << 1),
 	TRACECMD_FL_LATENCY		= (1 << 2),
 	TRACECMD_FL_IN_USECS		= (1 << 3),
+	TRACECMD_FL_FLYRECORD		= (1 << 4),
 };
 
 struct tracecmd_ftrace {
diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index f04528ae..ee9bfb52 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -141,6 +141,8 @@ struct tracecmd_input {
 
 __thread struct tracecmd_input *tracecmd_curr_thread_handle;
 
+static int read_options_type(struct tracecmd_input *handle);
+
 void tracecmd_set_flag(struct tracecmd_input *handle, int flag)
 {
 	handle->flags |= flag;
@@ -747,6 +749,19 @@ int tracecmd_get_parsing_failures(struct tracecmd_input *handle)
 	return 0;
 }
 
+static int read_cpus(struct tracecmd_input *handle)
+{
+	unsigned int cpus;
+
+	if (read4(handle, &cpus) < 0)
+		return -1;
+
+	handle->cpus = cpus;
+	tep_set_cpus(handle->pevent, handle->cpus);
+
+	return 0;
+}
+
 /**
  * tracecmd_read_headers - read the header information from trace.dat
  * @handle: input handle for the trace.dat file
@@ -784,6 +799,12 @@ int tracecmd_read_headers(struct tracecmd_input *handle)
 	if (read_and_parse_cmdlines(handle) < 0)
 		return -1;
 
+	if (read_cpus(handle) < 0)
+		return -1;
+
+	if (read_options_type(handle) < 0)
+		return -1;
+
 	tep_set_long_size(handle->pevent, handle->long_size);
 
 	return 0;
@@ -2576,23 +2597,13 @@ static int handle_options(struct tracecmd_input *handle)
 	return 0;
 }
 
-static int read_cpu_data(struct tracecmd_input *handle)
+static int read_options_type(struct tracecmd_input *handle)
 {
-	struct tep_handle *pevent = handle->pevent;
-	enum kbuffer_long_size long_size;
-	enum kbuffer_endian endian;
-	unsigned long long size;
-	unsigned long long max_size = 0;
-	unsigned long long pages;
 	char buf[10];
-	int cpus;
-	int cpu;
 
 	if (do_read_check(handle, buf, 10))
 		return -1;
 
-	cpus = handle->cpus;
-
 	/* check if this handles options */
 	if (strncmp(buf, "options", 7) == 0) {
 		if (handle_options(handle) < 0)
@@ -2602,17 +2613,41 @@ static int read_cpu_data(struct tracecmd_input *handle)
 	}
 
 	/*
-	 * Check if this is a latency report or not.
+	 * Check if this is a latency report or flyrecord.
 	 */
-	if (strncmp(buf, "latency", 7) == 0) {
+	if (strncmp(buf, "latency", 7) == 0)
 		handle->flags |= TRACECMD_FL_LATENCY;
+	else if (strncmp(buf, "flyrecord", 9) == 0)
+		handle->flags |= TRACECMD_FL_FLYRECORD;
+	else
+		return -1;
+
+	return 0;
+}
+
+static int read_cpu_data(struct tracecmd_input *handle)
+{
+	struct tep_handle *pevent = handle->pevent;
+	enum kbuffer_long_size long_size;
+	enum kbuffer_endian endian;
+	unsigned long long size;
+	unsigned long long max_size = 0;
+	unsigned long long pages;
+	int cpus;
+	int cpu;
+
+	/*
+	 * Check if this is a latency report or not.
+	 */
+	if (handle->flags & TRACECMD_FL_LATENCY)
 		return 1;
-	}
 
 	/* We expect this to be flyrecord */
-	if (strncmp(buf, "flyrecord", 9) != 0)
+	if (!(handle->flags & TRACECMD_FL_FLYRECORD))
 		return -1;
 
+	cpus = handle->cpus;
+
 	handle->cpu_data = malloc(sizeof(*handle->cpu_data) * handle->cpus);
 	if (!handle->cpu_data)
 		return -1;
@@ -2795,15 +2830,8 @@ static int read_and_parse_trace_clock(struct tracecmd_input *handle,
 int tracecmd_init_data(struct tracecmd_input *handle)
 {
 	struct tep_handle *pevent = handle->pevent;
-	unsigned int cpus;
 	int ret;
 
-	if (read4(handle, &cpus) < 0)
-		return -1;
-	handle->cpus = cpus;
-
-	tep_set_cpus(pevent, handle->cpus);
-
 	ret = read_cpu_data(handle);
 	if (ret < 0)
 		return ret;
@@ -3579,25 +3607,28 @@ tracecmd_buffer_instance_handle(struct tracecmd_input *handle, int indx)
 	if (ret < 0) {
 		warning("could not seek to buffer %s offset %ld\n",
 			buffer->name, buffer->offset);
-		tracecmd_close(new_handle);
-		return NULL;
+		goto error;
 	}
 
-	ret = read_cpu_data(new_handle);
+	ret = read_options_type(new_handle);
+	if (!ret)
+		ret = read_cpu_data(new_handle);
 	if (ret < 0) {
 		warning("failed to read sub buffer %s\n", buffer->name);
-		tracecmd_close(new_handle);
-		return NULL;
+		goto error;
 	}
 
 	ret = lseek64(handle->fd, offset, SEEK_SET);
 	if (ret < 0) {
 		warning("could not seek to back to offset %ld\n", offset);
-		tracecmd_close(new_handle);
-		return NULL;
+		goto error;
 	}
 
 	return new_handle;
+
+error:
+	tracecmd_close(new_handle);
+	return NULL;
 }
 
 int tracecmd_is_buffer_instance(struct tracecmd_input *handle)
-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] trace-cmd: Add new API tracecmd_open_head()
  2020-04-09 13:28 [PATCH 0/2] Split reading the trace.dat options from trace data Tzvetomir Stoyanov (VMware)
  2020-04-09 13:28 ` [PATCH 1/2] trace-cmd: Move reading of trace.dat options to tracecmd_read_headers() Tzvetomir Stoyanov (VMware)
@ 2020-04-09 13:28 ` Tzvetomir Stoyanov (VMware)
  2021-01-13 22:01   ` Steven Rostedt
  1 sibling, 1 reply; 5+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-04-09 13:28 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Add an API to create a tracecmd_handle from a file,
read and parse only the trace headers from the file.
This allows to implement opening a trace file on
stages - reading the trace headers and reading the trace data.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h |  1 +
 lib/trace-cmd/trace-input.c   | 28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 3f96bbde..e22aa251 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -143,6 +143,7 @@ typedef void (*tracecmd_handle_init_func)(struct tracecmd_input *handle,
 struct tracecmd_input *tracecmd_alloc(const char *file);
 struct tracecmd_input *tracecmd_alloc_fd(int fd);
 struct tracecmd_input *tracecmd_open(const char *file);
+struct tracecmd_input *tracecmd_open_head(const char *file);
 struct tracecmd_input *tracecmd_open_fd(int fd);
 void tracecmd_ref(struct tracecmd_input *handle);
 void tracecmd_close(struct tracecmd_input *handle);
diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index ee9bfb52..efc8d4bd 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -3175,6 +3175,34 @@ struct tracecmd_input *tracecmd_open(const char *file)
 	return tracecmd_open_fd(fd);
 }
 
+/**
+ * tracecmd_open_head - create a tracecmd_handle from a given file, read
+ *			read and parse only the trace headers from the file
+ * @file: the file name of the file that is of tracecmd data type.
+ */
+struct tracecmd_input *tracecmd_open_head(const char *file)
+{
+	struct tracecmd_input *handle;
+	int fd;
+
+	fd = open(file, O_RDONLY);
+	if (fd < 0)
+		return NULL;
+
+	handle = tracecmd_alloc_fd(fd);
+	if (!handle)
+		return NULL;
+
+	if (tracecmd_read_headers(handle) < 0)
+		goto fail;
+
+	return handle;
+
+fail:
+	tracecmd_close(handle);
+	return NULL;
+}
+
 /**
  * tracecmd_ref - add a reference to the handle
  * @handle: input handle for the trace.dat file
-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] trace-cmd: Add new API tracecmd_open_head()
  2020-04-09 13:28 ` [PATCH 2/2] trace-cmd: Add new API tracecmd_open_head() Tzvetomir Stoyanov (VMware)
@ 2021-01-13 22:01   ` Steven Rostedt
  2021-01-15  4:54     ` Tzvetomir Stoyanov
  0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2021-01-13 22:01 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Thu,  9 Apr 2020 16:28:25 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Add an API to create a tracecmd_handle from a file,
> read and parse only the trace headers from the file.
> This allows to implement opening a trace file on
> stages - reading the trace headers and reading the trace data.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  include/trace-cmd/trace-cmd.h |  1 +
>  lib/trace-cmd/trace-input.c   | 28 ++++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
> index 3f96bbde..e22aa251 100644
> --- a/include/trace-cmd/trace-cmd.h
> +++ b/include/trace-cmd/trace-cmd.h
> @@ -143,6 +143,7 @@ typedef void (*tracecmd_handle_init_func)(struct tracecmd_input *handle,
>  struct tracecmd_input *tracecmd_alloc(const char *file);
>  struct tracecmd_input *tracecmd_alloc_fd(int fd);
>  struct tracecmd_input *tracecmd_open(const char *file);
> +struct tracecmd_input *tracecmd_open_head(const char *file);
>  struct tracecmd_input *tracecmd_open_fd(int fd);
>  void tracecmd_ref(struct tracecmd_input *handle);
>  void tracecmd_close(struct tracecmd_input *handle);

Hi Tzvetomir,

I was thinking, before we release libtracecmd, we should add
tracecmd_open() and tracecmd_open_fd() to the library. Just because I find
that tracecmd_open_head() is a awkward name to have alone and only makes
sense if there's already a tracecmd_open(). It will also help with
differentiating the different functions.

I know we were going to only have functions that kernelshark uses, but that
may be a little too limiting.

Thanks!

-- Steve

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] trace-cmd: Add new API tracecmd_open_head()
  2021-01-13 22:01   ` Steven Rostedt
@ 2021-01-15  4:54     ` Tzvetomir Stoyanov
  0 siblings, 0 replies; 5+ messages in thread
From: Tzvetomir Stoyanov @ 2021-01-15  4:54 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Linux Trace Devel

On Thu, Jan 14, 2021 at 12:01 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Thu,  9 Apr 2020 16:28:25 +0300
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > Add an API to create a tracecmd_handle from a file,
> > read and parse only the trace headers from the file.
> > This allows to implement opening a trace file on
> > stages - reading the trace headers and reading the trace data.
> >
> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> > ---
> >  include/trace-cmd/trace-cmd.h |  1 +
> >  lib/trace-cmd/trace-input.c   | 28 ++++++++++++++++++++++++++++
> >  2 files changed, 29 insertions(+)
> >
> > diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
> > index 3f96bbde..e22aa251 100644
> > --- a/include/trace-cmd/trace-cmd.h
> > +++ b/include/trace-cmd/trace-cmd.h
> > @@ -143,6 +143,7 @@ typedef void (*tracecmd_handle_init_func)(struct tracecmd_input *handle,
> >  struct tracecmd_input *tracecmd_alloc(const char *file);
> >  struct tracecmd_input *tracecmd_alloc_fd(int fd);
> >  struct tracecmd_input *tracecmd_open(const char *file);
> > +struct tracecmd_input *tracecmd_open_head(const char *file);
> >  struct tracecmd_input *tracecmd_open_fd(int fd);
> >  void tracecmd_ref(struct tracecmd_input *handle);
> >  void tracecmd_close(struct tracecmd_input *handle);
>
> Hi Tzvetomir,
>
> I was thinking, before we release libtracecmd, we should add
> tracecmd_open() and tracecmd_open_fd() to the library. Just because I find
> that tracecmd_open_head() is a awkward name to have alone and only makes
> sense if there's already a tracecmd_open(). It will also help with
> differentiating the different functions.

I'll add tracecmd_open() and tracecmd_open_fd() as APIs. The reason
why tracecmd_open_head()
is used by KernelShark is that tracecmd_open() reads headers + tracing
data and calculates the
timestamps. Thus makes it hard to open multiple tracing files from a
single trace session and adjust
timestamps to be in a same time space. The current KernelShark open flow is:
 - Open a trace file and parse only the headers with tracecmd_open_head()
 - Based on the metadata information from the headers, check if there
is already an opened file from
  the same tracing session. If such is found, pair both files with
tracecmd_pair_peer(), before reading
  the tracing data.
 - Read the tracing data from the file with tracecmd_init_data(). This
will adjust timestamps accordingly,
   if there is a paired peer to the file.

We can rename tracecmd_open_head() to something else, or redesign the
whole flow of opening trace files

>
> I know we were going to only have functions that kernelshark uses, but that
> may be a little too limiting.
>
> Thanks!
>
> -- Steve



-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-09 13:28 [PATCH 0/2] Split reading the trace.dat options from trace data Tzvetomir Stoyanov (VMware)
2020-04-09 13:28 ` [PATCH 1/2] trace-cmd: Move reading of trace.dat options to tracecmd_read_headers() Tzvetomir Stoyanov (VMware)
2020-04-09 13:28 ` [PATCH 2/2] trace-cmd: Add new API tracecmd_open_head() Tzvetomir Stoyanov (VMware)
2021-01-13 22:01   ` Steven Rostedt
2021-01-15  4:54     ` Tzvetomir Stoyanov

Linux-Trace-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-trace-devel/0 linux-trace-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-trace-devel linux-trace-devel/ https://lore.kernel.org/linux-trace-devel \
		linux-trace-devel@vger.kernel.org
	public-inbox-index linux-trace-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-trace-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git