diff mbox series

[v2,34/87] trace-cmd library: Introduce sections in trace file reading logic

Message ID 20210729050959.12263-35-tz.stoyanov@gmail.com (mailing list archive)
State Superseded
Headers show
Series Trace file version 7 | expand

Commit Message

Tzvetomir Stoyanov (VMware) July 29, 2021, 5:09 a.m. UTC 
Trace file version 7 is based on sections. Added an internal sections
database and new helper functions to add, read, open and close file
sections.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/trace-input.c | 71 +++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

Comments

Steven Rostedt Aug. 19, 2021, 5:53 p.m. UTC | #1 
On Thu, 29 Jul 2021 08:09:06 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Trace file version 7 is based on sections. Added an internal sections
> database and new helper functions to add, read, open and close file
> sections.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  lib/trace-cmd/trace-input.c | 71 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> index 6872f6ef..e8eefb5c 100644
> --- a/lib/trace-cmd/trace-input.c
> +++ b/lib/trace-cmd/trace-input.c
> @@ -114,6 +114,14 @@ struct tsc2nsec {
>  	unsigned long long offset;
>  };
>  
> +struct file_section {
> +	int				id;
> +	unsigned long long		section_offset;
> +	unsigned long long		data_offset;

Should we have a size too?

> +	enum tracecmd_section_flags	flags;
> +	struct file_section		*next;
> +};
> +
>  struct tracecmd_input {
>  	struct tep_handle	*pevent;
>  	unsigned long		file_state;
> @@ -154,6 +162,7 @@ struct tracecmd_input {
>  	struct hook_list	*hooks;
>  	struct pid_addr_maps	*pid_maps;
>  	/* file information */
> +	struct file_section	*sections;
>  	size_t			header_files_start;
>  	size_t			ftrace_files_start;
>  	size_t			event_files_start;
> @@ -411,6 +420,61 @@ __hidden int in_uncompress_block(struct tracecmd_input *handle)
>  	return ret;
>  }
>  
> +static struct file_section *get_section(struct tracecmd_input *handle, int id)
> +{
> +	struct file_section *sec = handle->sections;
> +
> +	while (sec) {
> +		if (sec->id == id)
> +			return sec;
> +		sec = sec->next;
> +	}

The above could be a simple for loop.

	for (sec = handle->sections; sec; sec = sec->next) {
		if (sec->id == id);
			return sec;
	}


> +	return NULL;
> +}
> +
> +static struct file_section *open_section(struct tracecmd_input *handle, int id)

I wonder if we should change the names to "section_open()", "section_close()" ...

This way the prefix "section_" is know to be a command for handling sections.

> +{
> +	struct file_section *sec = get_section(handle, id);
> +
> +	if (!sec)
> +		return NULL;
> +
> +	if (lseek64(handle->fd, sec->data_offset, SEEK_SET) == (off64_t)-1)
> +		return NULL;
> +	if ((sec->flags & TRACECMD_SEC_FL_COMPRESS) && in_uncompress_block(handle))
> +		return NULL;
> +	return sec;
> +}
> +
> +static void close_section(struct tracecmd_input *handle, struct file_section *sec)
> +{
> +	if (sec->flags & TRACECMD_SEC_FL_COMPRESS)
> +		in_uncompress_reset(handle);
> +}
> +
> +static int add_section(struct tracecmd_input *handle, int id, int flags,
> +		       unsigned long long soffset, unsigned long long doffset)

I'm still not sure what "soffest' is compared to "doffset".

> +{
> +	struct file_section *sec = get_section(handle, id);
> +
> +	if (!sec) {
> +		sec = calloc(1, sizeof(struct file_section));
> +		if (!sec)
> +			return -1;
> +		sec->next = handle->sections;
> +		handle->sections = sec;
> +	}

Hmm, we should probably have an "add" and "update" because blindly
changing an existing section may not be what was expected. That is,
going with the "section_" prefix.

section_add(...) 

Would error if it already exists.

section_update(..)

Would expect it to already exist.

Could have an:

section_add_or_update(...)

Which would be the above, and lets the caller know that it will ether
add the section, or update it.

-- Steve


> +	sec->id = id;
> +	if (soffset)
> +		sec->section_offset = soffset;
> +	if (doffset)
> +		sec->data_offset = doffset;
> +	if (flags > 0)
> +		sec->flags = flags;
> +	return 0;
> +}
> +
> +
>  static int read_header_files(struct tracecmd_input *handle)
>  {
>  	struct tep_handle *pevent = handle->pevent;
> @@ -3522,6 +3586,7 @@ void tracecmd_ref(struct tracecmd_input *handle)
>   */
>  void tracecmd_close(struct tracecmd_input *handle)
>  {
> +	struct file_section *del_sec;
>  	int i;
>  
>  	if (!handle)
> @@ -3560,6 +3625,11 @@ void tracecmd_close(struct tracecmd_input *handle)
>  	free(handle->version);
>  	close(handle->fd);
>  
> +	while (handle->sections) {
> +		del_sec = handle->sections;
> +		handle->sections = handle->sections->next;
> +		free(del_sec);
> +	}
>  	for (i = 0; i < handle->nr_buffers; i++)
>  		free(handle->buffers[i].name);
>  
> @@ -4006,6 +4076,7 @@ tracecmd_buffer_instance_handle(struct tracecmd_input *handle, int indx)
>  	new_handle->nr_buffers = 0;
>  	new_handle->buffers = NULL;
>  	new_handle->version = NULL;
> +	new_handle->sections = NULL;
>  	new_handle->guest = NULL;
>  	new_handle->ref = 1;
>  	if (handle->trace_clock) {
diff mbox series

Patch

diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index 6872f6ef..e8eefb5c 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -114,6 +114,14 @@  struct tsc2nsec {
 	unsigned long long offset;
 };
 
+struct file_section {
+	int				id;
+	unsigned long long		section_offset;
+	unsigned long long		data_offset;
+	enum tracecmd_section_flags	flags;
+	struct file_section		*next;
+};
+
 struct tracecmd_input {
 	struct tep_handle	*pevent;
 	unsigned long		file_state;
@@ -154,6 +162,7 @@  struct tracecmd_input {
 	struct hook_list	*hooks;
 	struct pid_addr_maps	*pid_maps;
 	/* file information */
+	struct file_section	*sections;
 	size_t			header_files_start;
 	size_t			ftrace_files_start;
 	size_t			event_files_start;
@@ -411,6 +420,61 @@  __hidden int in_uncompress_block(struct tracecmd_input *handle)
 	return ret;
 }
 
+static struct file_section *get_section(struct tracecmd_input *handle, int id)
+{
+	struct file_section *sec = handle->sections;
+
+	while (sec) {
+		if (sec->id == id)
+			return sec;
+		sec = sec->next;
+	}
+	return NULL;
+}
+
+static struct file_section *open_section(struct tracecmd_input *handle, int id)
+{
+	struct file_section *sec = get_section(handle, id);
+
+	if (!sec)
+		return NULL;
+
+	if (lseek64(handle->fd, sec->data_offset, SEEK_SET) == (off64_t)-1)
+		return NULL;
+	if ((sec->flags & TRACECMD_SEC_FL_COMPRESS) && in_uncompress_block(handle))
+		return NULL;
+	return sec;
+}
+
+static void close_section(struct tracecmd_input *handle, struct file_section *sec)
+{
+	if (sec->flags & TRACECMD_SEC_FL_COMPRESS)
+		in_uncompress_reset(handle);
+}
+
+static int add_section(struct tracecmd_input *handle, int id, int flags,
+		       unsigned long long soffset, unsigned long long doffset)
+{
+	struct file_section *sec = get_section(handle, id);
+
+	if (!sec) {
+		sec = calloc(1, sizeof(struct file_section));
+		if (!sec)
+			return -1;
+		sec->next = handle->sections;
+		handle->sections = sec;
+	}
+	sec->id = id;
+	if (soffset)
+		sec->section_offset = soffset;
+	if (doffset)
+		sec->data_offset = doffset;
+	if (flags > 0)
+		sec->flags = flags;
+	return 0;
+}
+
+
 static int read_header_files(struct tracecmd_input *handle)
 {
 	struct tep_handle *pevent = handle->pevent;
@@ -3522,6 +3586,7 @@  void tracecmd_ref(struct tracecmd_input *handle)
  */
 void tracecmd_close(struct tracecmd_input *handle)
 {
+	struct file_section *del_sec;
 	int i;
 
 	if (!handle)
@@ -3560,6 +3625,11 @@  void tracecmd_close(struct tracecmd_input *handle)
 	free(handle->version);
 	close(handle->fd);
 
+	while (handle->sections) {
+		del_sec = handle->sections;
+		handle->sections = handle->sections->next;
+		free(del_sec);
+	}
 	for (i = 0; i < handle->nr_buffers; i++)
 		free(handle->buffers[i].name);
 
@@ -4006,6 +4076,7 @@  tracecmd_buffer_instance_handle(struct tracecmd_input *handle, int indx)
 	new_handle->nr_buffers = 0;
 	new_handle->buffers = NULL;
 	new_handle->version = NULL;
+	new_handle->sections = NULL;
 	new_handle->guest = NULL;
 	new_handle->ref = 1;
 	if (handle->trace_clock) {