[PATCH v2 0/7] trace-cmd: split: Handle splitting files with multiple instances

[PATCH v2 0/7] trace-cmd: split: Handle splitting files with multiple instances

[Pierre Gondois]

V2:
- Add 'trace-cmd split: Enable support for buffer selection' to
  handle instance selection with the 'trace-cmd' split command.
- Rename patches as 'trace-cmd: split' -> 'trace-cmd split'.
- Correctly free allocated memory in different lists.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218357

Splitting trace files having multiple buffer instances discards
side-buffers . I.e., only the main buffer is preserved in the
resulting split trace files. This patch-set intends to fix this.

As a continuation of the bugzilla, support to allow selecting
buffer instances is added to the 'trace-cmd split' command [1].
This support implies adding new '-B/--top' parameters.

Also:
- Fix a side issue preventing to provide start/end timestamps
  along with a time-window parameters.
- Update trace-cmd split usage

[1] https://lore.kernel.org/all/20240119122511.440d8f0a@gandalf.local.home/

Pierre Gondois (7):
  trace-cmd split: Small fixes
  trace-cmd split: Correctly split with start/end/time-window parameters
  trace-cmd split: Store instances in local list
  trace-cmd split: Add functions to generate temp files
  trace-cmd split: Handle splitting files with multiple instances
  trace-cmd split: Enable support for buffer selection
  trace-cmd split: Update usage

 tracecmd/trace-split.c | 517 +++++++++++++++++++++++++++++++++--------
 tracecmd/trace-usage.c |   9 +-
 2 files changed, 434 insertions(+), 92 deletions(-)

-- 
2.25.1

[PATCH v2 1/7] trace-cmd split: Small fixes

[Pierre Gondois]

Small fixes:
- Remove a useless assignment to 'current' variable.
- Fix returned type of parse_file()

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 tracecmd/trace-split.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tracecmd/trace-split.c b/tracecmd/trace-split.c
index 4fda7813..6ccda2fc 100644
--- a/tracecmd/trace-split.c
+++ b/tracecmd/trace-split.c
@@ -347,11 +347,12 @@ static int parse_cpu(struct tracecmd_input *handle,
 	return 0;
 }
 
-static double parse_file(struct tracecmd_input *handle,
-			 const char *output_file,
-			 unsigned long long start,
-			 unsigned long long end, int percpu, int only_cpu,
-			 int count, enum split_types type)
+static unsigned long long parse_file(struct tracecmd_input *handle,
+				     const char *output_file,
+				     unsigned long long start,
+				     unsigned long long end, int percpu,
+				     int only_cpu, int count,
+				     enum split_types type)
 {
 	unsigned long long current;
 	struct tracecmd_output *ohandle;
@@ -551,7 +552,6 @@ void trace_split (int argc, char **argv)
 		strcat(output, ".1");
 	}
 
-	current = start_ns;
 	output_file = malloc(strlen(output) + 50);
 	if (!output_file)
 		die("Failed to allocate for %s", output);
-- 
2.25.1

[PATCH v2 2/7] trace-cmd split: Correctly split with start/end/time-window parameters

[Pierre Gondois]

With a trace.dat file having events timestamped between
[284443.0, 284448.0] seconds, the following command:
  trace-cmd split -i trace.dat -o trace2.dat -r -m 100 284444 284445
should produce ten trace2.dat.XXX files between [284444.0, 284445.0],
each one lasting 100ms.

Currently only one trace2.dat.001 file is produced, with events
within the [284444.0, 284445.1] time window.

Don't stop splitting the input file after the first iteration.
Add a end_reached to detect when the end timestamp is reached.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 tracecmd/trace-split.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/tracecmd/trace-split.c b/tracecmd/trace-split.c
index 6ccda2fc..b6c056b5 100644
--- a/tracecmd/trace-split.c
+++ b/tracecmd/trace-split.c
@@ -198,7 +198,7 @@ static int parse_cpu(struct tracecmd_input *handle,
 		     unsigned long long start,
 		     unsigned long long end,
 		     int count_limit, int percpu, int cpu,
-		     enum split_types type)
+		     enum split_types type, bool *end_reached)
 {
 	struct tep_record *record;
 	struct tep_handle *pevent;
@@ -325,6 +325,9 @@ static int parse_cpu(struct tracecmd_input *handle,
 		}
 	}
 
+	if (record && (record->ts > end))
+		*end_reached = true;
+
 	if (record)
 		tracecmd_free_record(record);
 
@@ -352,7 +355,8 @@ static unsigned long long parse_file(struct tracecmd_input *handle,
 				     unsigned long long start,
 				     unsigned long long end, int percpu,
 				     int only_cpu, int count,
-				     enum split_types type)
+				     enum split_types type,
+				     bool *end_reached)
 {
 	unsigned long long current;
 	struct tracecmd_output *ohandle;
@@ -396,14 +400,14 @@ static unsigned long long parse_file(struct tracecmd_input *handle,
 
 	if (only_cpu >= 0) {
 		parse_cpu(handle, cpu_data, start, end, count,
-			  1, only_cpu, type);
+			  1, only_cpu, type, end_reached);
 	} else if (percpu) {
 		for (cpu = 0; cpu < cpus; cpu++)
 			parse_cpu(handle, cpu_data, start,
-				  end, count, percpu, cpu, type);
+				  end, count, percpu, cpu, type, end_reached);
 	} else
 		parse_cpu(handle, cpu_data, start,
-			  end, count, percpu, -1, type);
+			  end, count, percpu, -1, type, end_reached);
 
 	cpu_list = malloc(sizeof(*cpu_list) * cpus);
 	if (!cpu_list)
@@ -415,7 +419,6 @@ static unsigned long long parse_file(struct tracecmd_input *handle,
 	if (tracecmd_append_cpu_data(ohandle, cpus, cpu_list) < 0)
 		die("Failed to append tracing data\n");
 
-	current = end;
 	for (cpu = 0; cpu < cpus; cpu++) {
 		/* Set the tracecmd cursor to the next set of records */
 		if (cpu_data[cpu].offset) {
@@ -440,6 +443,7 @@ void trace_split (int argc, char **argv)
 	struct tracecmd_input *handle;
 	unsigned long long start_ns = 0, end_ns = 0;
 	unsigned long long current;
+	bool end_reached = false;
 	double start, end;
 	char *endptr;
 	char *output = NULL;
@@ -564,11 +568,12 @@ void trace_split (int argc, char **argv)
 			strcpy(output_file, output);
 			
 		current = parse_file(handle, output_file, start_ns, end_ns,
-				     percpu, cpu, count, type);
+				     percpu, cpu, count, type, &end_reached);
+
 		if (!repeat)
 			break;
 		start_ns = 0;
-	} while (current && (!end_ns || current < end_ns));
+	} while (!end_reached && (current && (!end_ns || current < end_ns)));
 
 	free(output);
 	free(output_file);
-- 
2.25.1

Re: [PATCH v2 2/7] trace-cmd split: Correctly split with start/end/time-window parameters

[Steven Rostedt]

On Mon, 22 Jan 2024 17:43:31 +0100
Pierre Gondois <pierre.gondois@arm.com> wrote:

> With a trace.dat file having events timestamped between
> [284443.0, 284448.0] seconds, the following command:
>   trace-cmd split -i trace.dat -o trace2.dat -r -m 100 284444 284445
> should produce ten trace2.dat.XXX files between [284444.0, 284445.0],
> each one lasting 100ms.
> 
> Currently only one trace2.dat.001 file is produced, with events
> within the [284444.0, 284445.1] time window.
> 
> Don't stop splitting the input file after the first iteration.
> Add a end_reached to detect when the end timestamp is reached.
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>

FYI, I applied the first two patches of your series. You don't need to
include them in future versions.

-- Steve

Re: [PATCH v2 2/7] trace-cmd split: Correctly split with start/end/time-window parameters

[Steven Rostedt]

On Mon, 22 Jan 2024 17:43:31 +0100
Pierre Gondois <pierre.gondois@arm.com> wrote:

>  	if (!cpu_list)
> @@ -415,7 +419,6 @@ static unsigned long long parse_file(struct tracecmd_input *handle,
>  	if (tracecmd_append_cpu_data(ohandle, cpus, cpu_list) < 0)
>  		die("Failed to append tracing data\n");
>  
> -	current = end;
>  	for (cpu = 0; cpu < cpus; cpu++) {
>  		/* Set the tracecmd cursor to the next set of records */
>  		if (cpu_data[cpu].offset) {

I tried running the code with valgrind, and it it went into an infinite
loop, saying that the code at line 728 had a conditional jump based on an
uninitialized variable. That line is here:

	do {
		if (repeat)
			sprintf(output_file, "%s.%04d", output, c++);
		else
			strcpy(output_file, output);
			
		current = parse_file(handle, output_file, start_ns, end_ns,
				     percpu, cpu, count, type, &end_reached);

		if (!repeat)
			break;
		start_ns = 0;
	} while (!end_reached && (current && (!end_ns || current < end_ns)));  <<<<-- Line 728

Debugging it, I found that the above "current = end;" removal removed the
only initialization of current.

Just an FYI, I'll write a fix.

Thanks,

-- Steve

[PATCH v2 3/7] trace-cmd split: Store instances in local list

[Pierre Gondois]

To prepare handling of multiple instances, store instance
handles in a local list, similarly to what is currently
done in tracecmd/trace-read.c.

To help achieve this goal, add a 'struct handle_list' and
add_handle()/free_handles() functions. 'struct handle' elements
are added to the static list, but not used in this patch.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 tracecmd/trace-split.c | 60 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/tracecmd/trace-split.c b/tracecmd/trace-split.c
index b6c056b5..f46813d1 100644
--- a/tracecmd/trace-split.c
+++ b/tracecmd/trace-split.c
@@ -19,10 +19,12 @@
 #include <ctype.h>
 #include <errno.h>
 
+#include "list.h"
 #include "trace-local.h"
 
 static unsigned int page_size;
 static const char *default_input_file = DEFAULT_INPUT_FILE;
+static const char *default_top_instance_name = "top";
 static const char *input_file;
 
 enum split_types {
@@ -49,6 +51,46 @@ struct cpu_data {
 	char				*file;
 };
 
+struct handle_list {
+	struct list_head		list;
+	const char			*name;
+	int				index;
+
+	/* Identify the top instance in the input trace. */
+	bool				was_top_instance;
+
+	/* Identify the top instance in each output trace. */
+	bool				is_top_instance;
+};
+
+static struct list_head handle_list;
+
+static void add_handle(const char *name, int index, bool was_top_instance)
+{
+	struct handle_list *item;
+
+	item = calloc(1, sizeof(*item));
+	if (!item)
+		die("Failed to allocate handle item");
+
+	item->name = strdup(name);
+	item->index = index;
+	item->was_top_instance = was_top_instance;
+	list_add_tail(&item->list, &handle_list);
+}
+
+static void free_handles(struct list_head *list)
+{
+	struct handle_list *item;
+
+	while (!list_empty(list)) {
+		item = container_of(list->next, struct handle_list, list);
+		list_del(&item->list);
+		free((char*)item->name);
+		free(item);
+	}
+}
+
 static int create_type_len(struct tep_handle *pevent, int time, int len)
 {
 	static int bigendian = -1;
@@ -450,6 +492,7 @@ void trace_split (int argc, char **argv)
 	char *output_file;
 	enum split_types split_type = SPLIT_NONE;
 	enum split_types type = SPLIT_NONE;
+	int instances;
 	int count;
 	int repeat = 0;
 	int percpu = 0;
@@ -457,6 +500,8 @@ void trace_split (int argc, char **argv)
 	int ac;
 	int c;
 
+	list_head_init(&handle_list);
+
 	if (strcmp(argv[1], "split") != 0)
 		usage(argv);
 
@@ -561,6 +606,20 @@ void trace_split (int argc, char **argv)
 		die("Failed to allocate for %s", output);
 	c = 1;
 
+	add_handle(default_top_instance_name, -1, true);
+	instances = tracecmd_buffer_instances(handle);
+	if (instances) {
+		const char *name;
+		int i;
+
+		for (i = 0; i < instances; i++) {
+			name = tracecmd_buffer_instance_name(handle, i);
+			if (!name)
+				die("error in reading buffer instance");
+			add_handle(name, i, false);
+		}
+	}
+
 	do {
 		if (repeat)
 			sprintf(output_file, "%s.%04d", output, c++);
@@ -579,6 +638,7 @@ void trace_split (int argc, char **argv)
 	free(output_file);
 
 	tracecmd_close(handle);
+	free_handles(&handle_list);
 
 	return;
 }
-- 
2.25.1

Re: [PATCH v2 3/7] trace-cmd split: Store instances in local list

[Steven Rostedt]

On Mon, 22 Jan 2024 17:43:32 +0100
Pierre Gondois <pierre.gondois@arm.com> wrote:

> To prepare handling of multiple instances, store instance
> handles in a local list, similarly to what is currently
> done in tracecmd/trace-read.c.
> 
> To help achieve this goal, add a 'struct handle_list' and
> add_handle()/free_handles() functions. 'struct handle' elements
> are added to the static list, but not used in this patch.
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  tracecmd/trace-split.c | 60 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/tracecmd/trace-split.c b/tracecmd/trace-split.c
> index b6c056b5..f46813d1 100644
> --- a/tracecmd/trace-split.c
> +++ b/tracecmd/trace-split.c
> @@ -19,10 +19,12 @@
>  #include <ctype.h>
>  #include <errno.h>
>  
> +#include "list.h"
>  #include "trace-local.h"
>  
>  static unsigned int page_size;
>  static const char *default_input_file = DEFAULT_INPUT_FILE;
> +static const char *default_top_instance_name = "top";
>  static const char *input_file;
>  
>  enum split_types {
> @@ -49,6 +51,46 @@ struct cpu_data {
>  	char				*file;
>  };
>  
> +struct handle_list {
> +	struct list_head		list;
> +	const char			*name;
> +	int				index;
> +
> +	/* Identify the top instance in the input trace. */
> +	bool				was_top_instance;
> +
> +	/* Identify the top instance in each output trace. */
> +	bool				is_top_instance;
> +};
> +
> +static struct list_head handle_list;
> +
> +static void add_handle(const char *name, int index, bool was_top_instance)
> +{
> +	struct handle_list *item;
> +
> +	item = calloc(1, sizeof(*item));
> +	if (!item)
> +		die("Failed to allocate handle item");
> +
> +	item->name = strdup(name);

Nit, need to check for strdup() errors.

-- Steve

> +	item->index = index;
> +	item->was_top_instance = was_top_instance;
> +	list_add_tail(&item->list, &handle_list);
> +}
> +

[PATCH v2 4/7] trace-cmd split: Add functions to generate temp files

[Pierre Gondois]

To prepare handling of multiple instances and storing them
in temporary files, add utility functions generating file names,
removing files, creating files:
- get_temp_file()
- delete_temp_file()
- put_temp_file()
- touch_file()

Also make use these functions.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 tracecmd/trace-split.c | 70 +++++++++++++++++++++++++++++++++---------
 1 file changed, 55 insertions(+), 15 deletions(-)

diff --git a/tracecmd/trace-split.c b/tracecmd/trace-split.c
index f46813d1..5f3ed940 100644
--- a/tracecmd/trace-split.c
+++ b/tracecmd/trace-split.c
@@ -392,6 +392,52 @@ static int parse_cpu(struct tracecmd_input *handle,
 	return 0;
 }
 
+static char *get_temp_file(const char *output_file, const char *name, int cpu)
+{
+	const char *dot;
+	char *file = NULL;
+	char *output;
+	char *base;
+	char *dir;
+	int ret;
+
+	if (name)
+		dot = ".";
+	else
+		dot = name = "";
+
+	output = strdup(output_file);
+	/* Extract basename() first, as dirname() truncates output */
+	base = basename(output);
+	dir = dirname(output);
+
+	ret = asprintf(&file, "%s/.tmp.%s.%s%s%d", dir, base, name, dot, cpu);
+	if (ret < 0)
+		die("Failed to allocate file for %s %s %s %d", dir, base, name, cpu);
+	free(output);
+	return file;
+}
+
+static void delete_temp_file(const char *name)
+{
+	unlink(name);
+}
+
+static void put_temp_file(char *file)
+{
+	free(file);
+}
+
+static void touch_file(const char *file)
+{
+	int fd;
+
+	fd = open(file, O_WRONLY | O_CREAT | O_TRUNC, 0644);
+	if (fd < 0)
+		die("could not create file %s\n", file);
+	close(fd);
+}
+
 static unsigned long long parse_file(struct tracecmd_input *handle,
 				     const char *output_file,
 				     unsigned long long start,
@@ -405,19 +451,11 @@ static unsigned long long parse_file(struct tracecmd_input *handle,
 	struct cpu_data *cpu_data;
 	struct tep_record *record;
 	char **cpu_list;
-	char *output;
-	char *base;
 	char *file;
-	char *dir;
 	int cpus;
 	int cpu;
 	int fd;
 
-	output = strdup(output_file);
-	/* Extract basename() first, as dirname() truncates output */
-	base = basename(output);
-	dir = dirname(output);
-
 	ohandle = tracecmd_copy(handle, output_file, TRACECMD_FILE_CMD_LINES, 0, NULL);
 
 	cpus = tracecmd_cpus(handle);
@@ -426,11 +464,9 @@ static unsigned long long parse_file(struct tracecmd_input *handle,
 		die("Failed to allocate cpu_data for %d cpus", cpus);
 
 	for (cpu = 0; cpu < cpus; cpu++) {
-		int ret;
+		file = get_temp_file(output_file, NULL, cpu);
+		touch_file(file);
 
-		ret = asprintf(&file, "%s/.tmp.%s.%d", dir, base, cpu);
-		if (ret < 0)
-			die("Failed to allocate file for %s %s %d", dir, base, cpu);
 		fd = open(file, O_WRONLY | O_CREAT | O_TRUNC | O_LARGEFILE, 0644);
 		cpu_data[cpu].cpu = cpu;
 		cpu_data[cpu].fd = fd;
@@ -469,12 +505,16 @@ static unsigned long long parse_file(struct tracecmd_input *handle,
 				current = record->ts + 1;
 			tracecmd_free_record(record);
 		}
-		unlink(cpu_data[cpu].file);
-		free(cpu_data[cpu].file);
+	}
+
+	for (cpu = 0; cpu < cpus; cpu++) {
+		close(cpu_data[cpu].fd);
+		delete_temp_file(cpu_data[cpu].file);
+		put_temp_file(cpu_data[cpu].file);
 	}
 	free(cpu_data);
 	free(cpu_list);
-	free(output);
+
 	tracecmd_output_close(ohandle);
 
 	return current;
-- 
2.25.1

Re: [PATCH v2 4/7] trace-cmd split: Add functions to generate temp files

[Steven Rostedt]

On Mon, 22 Jan 2024 17:43:33 +0100
Pierre Gondois <pierre.gondois@arm.com> wrote:

> To prepare handling of multiple instances and storing them
> in temporary files, add utility functions generating file names,
> removing files, creating files:
> - get_temp_file()
> - delete_temp_file()
> - put_temp_file()
> - touch_file()
> 
> Also make use these functions.
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  tracecmd/trace-split.c | 70 +++++++++++++++++++++++++++++++++---------
>  1 file changed, 55 insertions(+), 15 deletions(-)
> 
> diff --git a/tracecmd/trace-split.c b/tracecmd/trace-split.c
> index f46813d1..5f3ed940 100644
> --- a/tracecmd/trace-split.c
> +++ b/tracecmd/trace-split.c
> @@ -392,6 +392,52 @@ static int parse_cpu(struct tracecmd_input *handle,
>  	return 0;
>  }
>  
> +static char *get_temp_file(const char *output_file, const char *name, int cpu)
> +{
> +	const char *dot;
> +	char *file = NULL;
> +	char *output;
> +	char *base;
> +	char *dir;
> +	int ret;
> +
> +	if (name)
> +		dot = ".";
> +	else
> +		dot = name = "";
> +
> +	output = strdup(output_file);

Nit on strdup() error checking again.

I know that I have forgotten to do this too, but lets not add more ;-)
and lets add the checks if they were missing when we modify that code.

Doing a clean up on that is on my todo list.

-- Steve


> +	/* Extract basename() first, as dirname() truncates output */
> +	base = basename(output);
> +	dir = dirname(output);
> +
> +	ret = asprintf(&file, "%s/.tmp.%s.%s%s%d", dir, base, name, dot, cpu);
> +	if (ret < 0)
> +		die("Failed to allocate file for %s %s %s %d", dir, base, name, cpu);
> +	free(output);
> +	return file;
> +}

[PATCH v2 5/7] trace-cmd split: Handle splitting files with multiple instances

[Pierre Gondois]

trace-cmd can record events in multiple instances:
  $ trace-cmd record -e sched_wakeup -B test_instance -e sched_switch

When trying to split a trace.dat file recorded with the above command,
only the events located in the main buffer seems to be split. The
events recorded in the test_instance buffer seem to be discarded:
  $ trace-cmd split -i trace.dat -o trace_2.dat 284443 284444
  $ trace-cmd report trace_2.dat
    cpus=8
           <...>-525991 [004] 284443.173879: sched_wakeup: [...]
           <...>-525991 [004] 284443.173879: sched_wakeup: [...]
           <...>-525990 [007] 284443.173885: sched_wakeup: [...]
           <...>-525990 [007] 284443.173885: sched_wakeup: [...]
(no sign of sched_switch events)

Keep all instances/buffers of a trace when it is split.
Also add a 'get_handle()' function.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218357
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 tracecmd/trace-split.c | 137 ++++++++++++++++++++++++++---------------
 1 file changed, 87 insertions(+), 50 deletions(-)

diff --git a/tracecmd/trace-split.c b/tracecmd/trace-split.c
index 5f3ed940..9b37c75e 100644
--- a/tracecmd/trace-split.c
+++ b/tracecmd/trace-split.c
@@ -91,6 +91,29 @@ static void free_handles(struct list_head *list)
 	}
 }
 
+/**
+ * get_handle - Obtain a handle that must be closed once finished.
+ */
+static struct tracecmd_input *get_handle(struct handle_list *item)
+{
+	struct tracecmd_input *top_handle, *handle;
+
+	top_handle = tracecmd_open(input_file, 0);
+	if (!top_handle)
+		die("Error reading %s", input_file);
+
+	if (item->was_top_instance) {
+		return top_handle;
+	} else {
+		handle = tracecmd_buffer_instance_handle(top_handle, item->index);
+		if (!handle)
+			warning("Could not retrieve handle %s", item->name);
+
+		tracecmd_close(top_handle);
+		return handle;
+	}
+}
+
 static int create_type_len(struct tep_handle *pevent, int time, int len)
 {
 	static int bigendian = -1;
@@ -447,6 +470,7 @@ static unsigned long long parse_file(struct tracecmd_input *handle,
 				     bool *end_reached)
 {
 	unsigned long long current;
+	struct handle_list *handle_entry;
 	struct tracecmd_output *ohandle;
 	struct cpu_data *cpu_data;
 	struct tep_record *record;
@@ -454,66 +478,79 @@ static unsigned long long parse_file(struct tracecmd_input *handle,
 	char *file;
 	int cpus;
 	int cpu;
+	int ret;
 	int fd;
 
 	ohandle = tracecmd_copy(handle, output_file, TRACECMD_FILE_CMD_LINES, 0, NULL);
+	tracecmd_set_out_clock(ohandle, tracecmd_get_trace_clock(handle));
 
-	cpus = tracecmd_cpus(handle);
-	cpu_data = malloc(sizeof(*cpu_data) * cpus);
-	if (!cpu_data)
-		die("Failed to allocate cpu_data for %d cpus", cpus);
-
-	for (cpu = 0; cpu < cpus; cpu++) {
-		file = get_temp_file(output_file, NULL, cpu);
-		touch_file(file);
-
-		fd = open(file, O_WRONLY | O_CREAT | O_TRUNC | O_LARGEFILE, 0644);
-		cpu_data[cpu].cpu = cpu;
-		cpu_data[cpu].fd = fd;
-		cpu_data[cpu].file = file;
-		cpu_data[cpu].offset = 0;
-		if (start)
-			tracecmd_set_cpu_to_timestamp(handle, cpu, start);
-	}
+	list_for_each_entry(handle_entry, &handle_list, list) {
+		struct tracecmd_input *curr_handle;
+
+		curr_handle = get_handle(handle_entry);
+		cpus = tracecmd_cpus(curr_handle);
+		cpu_data = malloc(sizeof(*cpu_data) * cpus);
+		if (!cpu_data)
+			die("Failed to allocate cpu_data for %d cpus", cpus);
 
-	if (only_cpu >= 0) {
-		parse_cpu(handle, cpu_data, start, end, count,
-			  1, only_cpu, type, end_reached);
-	} else if (percpu) {
+		for (cpu = 0; cpu < cpus; cpu++) {
+			file = get_temp_file(output_file, handle_entry->name, cpu);
+			touch_file(file);
+
+			fd = open(file, O_WRONLY | O_CREAT | O_TRUNC | O_LARGEFILE, 0644);
+			cpu_data[cpu].cpu = cpu;
+			cpu_data[cpu].fd = fd;
+			cpu_data[cpu].file = file;
+			cpu_data[cpu].offset = 0;
+			if (start)
+				tracecmd_set_cpu_to_timestamp(curr_handle, cpu, start);
+		}
+
+		if (only_cpu >= 0) {
+			parse_cpu(curr_handle, cpu_data, start, end, count,
+				  1, only_cpu, type, end_reached);
+		} else if (percpu) {
+			for (cpu = 0; cpu < cpus; cpu++)
+				parse_cpu(curr_handle, cpu_data, start,
+					  end, count, percpu, cpu, type, end_reached);
+		} else {
+			parse_cpu(curr_handle, cpu_data, start,
+				  end, count, percpu, -1, type, end_reached);
+		}
+
+		cpu_list = malloc(sizeof(*cpu_list) * cpus);
+		if (!cpu_list)
+			die("Failed to allocate cpu_list for %d cpus", cpus);
 		for (cpu = 0; cpu < cpus; cpu++)
-			parse_cpu(handle, cpu_data, start,
-				  end, count, percpu, cpu, type, end_reached);
-	} else
-		parse_cpu(handle, cpu_data, start,
-			  end, count, percpu, -1, type, end_reached);
-
-	cpu_list = malloc(sizeof(*cpu_list) * cpus);
-	if (!cpu_list)
-		die("Failed to allocate cpu_list for %d cpus", cpus);
-	for (cpu = 0; cpu < cpus; cpu ++)
-		cpu_list[cpu] = cpu_data[cpu].file;
+			cpu_list[cpu] = cpu_data[cpu].file;
 
-	tracecmd_set_out_clock(ohandle, tracecmd_get_trace_clock(handle));
-	if (tracecmd_append_cpu_data(ohandle, cpus, cpu_list) < 0)
-		die("Failed to append tracing data\n");
-
-	for (cpu = 0; cpu < cpus; cpu++) {
-		/* Set the tracecmd cursor to the next set of records */
-		if (cpu_data[cpu].offset) {
-			record = tracecmd_read_at(handle, cpu_data[cpu].offset, NULL);
-			if (record && (!current || record->ts > current))
-				current = record->ts + 1;
-			tracecmd_free_record(record);
+		if (handle_entry->is_top_instance)
+			ret = tracecmd_append_cpu_data(ohandle, cpus, cpu_list);
+		else
+			ret = tracecmd_append_buffer_cpu_data(ohandle, handle_entry->name, cpus,
+							      cpu_list);
+		if (ret < 0)
+			die("Failed to append tracing data\n");
+
+		for (cpu = 0; cpu < cpus; cpu++) {
+			/* Set the tracecmd cursor to the next set of records */
+			if (cpu_data[cpu].offset) {
+				record = tracecmd_read_at(curr_handle, cpu_data[cpu].offset, NULL);
+				if (record && (!current || record->ts > current))
+					current = record->ts + 1;
+				tracecmd_free_record(record);
+			}
 		}
-	}
 
-	for (cpu = 0; cpu < cpus; cpu++) {
-		close(cpu_data[cpu].fd);
-		delete_temp_file(cpu_data[cpu].file);
-		put_temp_file(cpu_data[cpu].file);
+		for (cpu = 0; cpu < cpus; cpu++) {
+			close(cpu_data[cpu].fd);
+			delete_temp_file(cpu_data[cpu].file);
+			put_temp_file(cpu_data[cpu].file);
+		}
+		free(cpu_data);
+		free(cpu_list);
+		tracecmd_close(curr_handle);
 	}
-	free(cpu_data);
-	free(cpu_list);
 
 	tracecmd_output_close(ohandle);
 
-- 
2.25.1

Re: [PATCH v2 5/7] trace-cmd split: Handle splitting files with multiple instances

[Steven Rostedt]

On Mon, 22 Jan 2024 17:43:34 +0100
Pierre Gondois <pierre.gondois@arm.com> wrote:

> trace-cmd can record events in multiple instances:
>   $ trace-cmd record -e sched_wakeup -B test_instance -e sched_switch
> 
> When trying to split a trace.dat file recorded with the above command,
> only the events located in the main buffer seems to be split. The
> events recorded in the test_instance buffer seem to be discarded:
>   $ trace-cmd split -i trace.dat -o trace_2.dat 284443 284444
>   $ trace-cmd report trace_2.dat
>     cpus=8
>            <...>-525991 [004] 284443.173879: sched_wakeup: [...]
>            <...>-525991 [004] 284443.173879: sched_wakeup: [...]
>            <...>-525990 [007] 284443.173885: sched_wakeup: [...]
>            <...>-525990 [007] 284443.173885: sched_wakeup: [...]
> (no sign of sched_switch events)
> 
> Keep all instances/buffers of a trace when it is split.
> Also add a 'get_handle()' function.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218357
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>

I had two trace.dat files I was checking on. One that just had the top
level instance being traced, and the other that only had an instance.

Until now, my instance trace.dat file did not have any output when doing a
split. Now it does. BUT! I tried the command you had shown in a previous patch:

  ./tracecmd/trace-cmd split -i trace.dat -o /tmp/trace2.dat -r -m 100 3122478 3122479

and it it kept going!

That is, this change broke the fix of the previous change.

-- Steve

Re: [PATCH v2 5/7] trace-cmd split: Handle splitting files with multiple instances

[Pierre Gondois]

Hello Steven,

On 1/23/24 01:23, Steven Rostedt wrote:
> On Mon, 22 Jan 2024 17:43:34 +0100
> Pierre Gondois <pierre.gondois@arm.com> wrote:
> 
>> trace-cmd can record events in multiple instances:
>>    $ trace-cmd record -e sched_wakeup -B test_instance -e sched_switch
>>
>> When trying to split a trace.dat file recorded with the above command,
>> only the events located in the main buffer seems to be split. The
>> events recorded in the test_instance buffer seem to be discarded:
>>    $ trace-cmd split -i trace.dat -o trace_2.dat 284443 284444
>>    $ trace-cmd report trace_2.dat
>>      cpus=8
>>             <...>-525991 [004] 284443.173879: sched_wakeup: [...]
>>             <...>-525991 [004] 284443.173879: sched_wakeup: [...]
>>             <...>-525990 [007] 284443.173885: sched_wakeup: [...]
>>             <...>-525990 [007] 284443.173885: sched_wakeup: [...]
>> (no sign of sched_switch events)
>>
>> Keep all instances/buffers of a trace when it is split.
>> Also add a 'get_handle()' function.
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218357
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>
> 
> I had two trace.dat files I was checking on. One that just had the top
> level instance being traced, and the other that only had an instance.
> 
> Until now, my instance trace.dat file did not have any output when doing a
> split. Now it does. BUT! I tried the command you had shown in a previous patch:
> 
>    ./tracecmd/trace-cmd split -i trace.dat -o /tmp/trace2.dat -r -m 100 3122478 3122479
> 
> and it it kept going!
> 
> That is, this change broke the fix of the previous change.

Yes indeed,
The problem should be fixed in the v3. Thanks for the tests,

Regards,
Pierre

> 
> -- Steve

[PATCH v2 6/7] trace-cmd split: Enable support for buffer selection

[Pierre Gondois]

The 'trace-cmd split' command conserves all buffers/instances
of the input .dat file. Add support to select the instances to
keep and to place as the top instance in an output .dat file.

Multiple .dat files can be generated with different combinations
of the instances to keep. The first instance in the command line
is selected as the top instance in an output .dat file.

For example, with a trace recorded with:
  $ trace-cmd record -e sched_wakeup -B switch_instance \
    -e sched_switch -B timer_instance -e timer

Creating a test.dat file containing the top instance and
the switch_instance:
  $ trace-cmd split --top -B switch_instance -o test.dat

Creating a test.dat file containing the switch_instance as
the top instance, and the initial top instance as an instance
named 'old_top':
  $ trace-cmd split -B switch_instance --top=old_top -o test.dat

Splitting all instances in different .dat files:
  $ trace-cmd split --top -o top.dat -B switch_instance \
    -o switch.dat -B timer_instance -o timer.dat

To achieve this, new 'struct name_list', 'struct output_file_list'
structures are created, with associated functions.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 tracecmd/trace-split.c | 275 +++++++++++++++++++++++++++++++++++------
 1 file changed, 234 insertions(+), 41 deletions(-)

diff --git a/tracecmd/trace-split.c b/tracecmd/trace-split.c
index 9b37c75e..2de7daa1 100644
--- a/tracecmd/trace-split.c
+++ b/tracecmd/trace-split.c
@@ -51,10 +51,85 @@ struct cpu_data {
 	char				*file;
 };
 
+struct name_list {
+	struct list_head list;
+	const char *name;
+
+	/* Identify the top instance in the input trace. */
+	bool was_top_instance;
+
+	/* Identify the top instance in each output trace. */
+	bool is_top_instance;
+};
+
+static void free_names(struct list_head *list) {
+	struct name_list *item;
+
+	while (!list_empty(list)) {
+		item = container_of(list->next, struct name_list, list);
+		list_del(&item->list);
+		free((char *)item->name);
+		free(item);
+	}
+}
+
+struct output_file_list {
+	struct list_head list;
+	const char *output;
+	char *output_file;
+	struct list_head instance_list;
+	struct list_head handle_list;
+};
+
+static struct list_head output_file_list;
+
+static struct output_file_list *add_output_file(void) {
+	struct output_file_list *item;
+
+	item = calloc(1, sizeof(*item));
+	if (!item)
+		die("Failed to allocate output_file item");
+
+	list_head_init(&item->instance_list);
+	list_head_init(&item->handle_list);
+	list_add_tail(&item->list, &output_file_list);
+	return item;
+}
+
+static void add_output_file_instance(struct output_file_list *output_file, const char *instance,
+				     bool was_top_instance, bool is_top_instance) {
+	struct name_list *item;
+
+	item = calloc(1, sizeof(*item));
+	if (!item)
+		die("Failed to allocate output_file item");
+	item->name = instance;
+	item->was_top_instance = was_top_instance;
+	item->is_top_instance = is_top_instance;
+	list_add_tail(&item->list, &output_file->instance_list);
+}
+
+static void free_handles(struct list_head *list);
+
+static void free_output_files(struct list_head *list) {
+	struct output_file_list *item;
+
+	while (!list_empty(list)) {
+		item = container_of(list->next, struct output_file_list, list);
+		list_del(&item->list);
+		free_names(&item->instance_list);
+		free_handles(&item->handle_list);
+		free((char *)item->output);
+		free((char *)item->output_file);
+		free(item);
+	}
+}
+
 struct handle_list {
 	struct list_head		list;
 	const char			*name;
 	int				index;
+	const char *output_file;
 
 	/* Identify the top instance in the input trace. */
 	bool				was_top_instance;
@@ -79,6 +154,18 @@ static void add_handle(const char *name, int index, bool was_top_instance)
 	list_add_tail(&item->list, &handle_list);
 }
 
+static void add_handle_copy(struct handle_list *item, struct name_list *name_entry,
+			    struct list_head *list) {
+	struct handle_list *copied_item;
+
+	copied_item = calloc(1, sizeof(*copied_item));
+	copied_item->name = strdup(name_entry->name);
+	copied_item->index = item->index;
+	copied_item->was_top_instance = item->was_top_instance;
+	copied_item->is_top_instance = name_entry->is_top_instance;
+	list_add_tail(&copied_item->list, list);
+}
+
 static void free_handles(struct list_head *list)
 {
 	struct handle_list *item;
@@ -87,6 +174,7 @@ static void free_handles(struct list_head *list)
 		item = container_of(list->next, struct handle_list, list);
 		list_del(&item->list);
 		free((char*)item->name);
+		free((char *)item->output_file);
 		free(item);
 	}
 }
@@ -462,13 +550,10 @@ static void touch_file(const char *file)
 }
 
 static unsigned long long parse_file(struct tracecmd_input *handle,
-				     const char *output_file,
-				     unsigned long long start,
-				     unsigned long long end, int percpu,
-				     int only_cpu, int count,
-				     enum split_types type,
-				     bool *end_reached)
-{
+				     struct output_file_list *output_file_entry,
+				     unsigned long long start, unsigned long long end, int percpu,
+				     int only_cpu, int count, enum split_types type,
+				     bool *end_reached) {
 	unsigned long long current;
 	struct handle_list *handle_entry;
 	struct tracecmd_output *ohandle;
@@ -481,10 +566,11 @@ static unsigned long long parse_file(struct tracecmd_input *handle,
 	int ret;
 	int fd;
 
-	ohandle = tracecmd_copy(handle, output_file, TRACECMD_FILE_CMD_LINES, 0, NULL);
+	ohandle =
+	    tracecmd_copy(handle, output_file_entry->output_file, TRACECMD_FILE_CMD_LINES, 0, NULL);
 	tracecmd_set_out_clock(ohandle, tracecmd_get_trace_clock(handle));
 
-	list_for_each_entry(handle_entry, &handle_list, list) {
+	list_for_each_entry(handle_entry, &output_file_entry->handle_list, list) {
 		struct tracecmd_input *curr_handle;
 
 		curr_handle = get_handle(handle_entry);
@@ -494,7 +580,8 @@ static unsigned long long parse_file(struct tracecmd_input *handle,
 			die("Failed to allocate cpu_data for %d cpus", cpus);
 
 		for (cpu = 0; cpu < cpus; cpu++) {
-			file = get_temp_file(output_file, handle_entry->name, cpu);
+			file =
+			    get_temp_file(output_file_entry->output_file, handle_entry->name, cpu);
 			touch_file(file);
 
 			fd = open(file, O_WRONLY | O_CREAT | O_TRUNC | O_LARGEFILE, 0644);
@@ -557,16 +644,63 @@ static unsigned long long parse_file(struct tracecmd_input *handle,
 	return current;
 }
 
+/**
+ * map_handle_output - Map the buffer/instances to the output_files.
+ *
+ * The trace-cmd split command can create multiple .dat files, each containing
+ * various combinations of the buffers available in the input .dat file.
+ * Associate the list of buffers/instances to keep to each output file. A
+ * buffer/instance can be present in multiple output files.
+ */
+static void map_handle_output(void) {
+	struct output_file_list *output_file_entry;
+	struct handle_list *handle_entry;
+	struct name_list *name_entry;
+	bool found_match = false;
+
+	list_for_each_entry(output_file_entry, &output_file_list, list) {
+		list_for_each_entry(name_entry, &output_file_entry->instance_list, list) {
+			list_for_each_entry(handle_entry, &handle_list, list) {
+				if ((name_entry->was_top_instance &&
+				     handle_entry->was_top_instance) ||
+				    !strcmp(handle_entry->name, name_entry->name)) {
+					found_match = true;
+					goto found;
+				}
+			}
+
+		found:
+			if (!found_match) {
+				warning("Buffer %s requested, but not found.", name_entry->name);
+				continue;
+			}
+
+			/*
+			 * As a handle can be requested in multiple output files,
+			 * the name_entry must be copied (and not de/en-queued).
+			 */
+			add_handle_copy(handle_entry, name_entry, &output_file_entry->handle_list);
+			found_match = false;
+		}
+	}
+}
+
+enum { OPT_top = 237,
+};
+
 void trace_split (int argc, char **argv)
 {
 	struct tracecmd_input *handle;
+	struct output_file_list *output_file_param = NULL;
+	struct output_file_list *output_file_entry;
 	unsigned long long start_ns = 0, end_ns = 0;
 	unsigned long long current;
+	bool was_top_instance;
+	bool is_top_instance;
 	bool end_reached = false;
 	double start, end;
 	char *endptr;
 	char *output = NULL;
-	char *output_file;
 	enum split_types split_type = SPLIT_NONE;
 	enum split_types type = SPLIT_NONE;
 	int instances;
@@ -577,12 +711,26 @@ void trace_split (int argc, char **argv)
 	int ac;
 	int c;
 
+	static struct option long_options[] = {
+		{"top", optional_argument, NULL, OPT_top},
+		{NULL, 0, NULL, 0},
+	};
+	int option_index = 0;
+
+	/* The first instance is the new top buffer of the output file. */
+	is_top_instance = true;
+	was_top_instance = false;
+
 	list_head_init(&handle_list);
+	list_head_init(&output_file_list);
+
+	output_file_param = add_output_file();
 
 	if (strcmp(argv[1], "split") != 0)
 		usage(argv);
 
-	while ((c = getopt(argc-1, argv+1, "+ho:i:s:m:u:e:p:rcC:")) >= 0) {
+	while ((c = getopt_long(argc - 1, argv + 1, "+ho:i:s:m:u:e:p:rcC:B:",
+				long_options, &option_index)) >= 0) {
 		switch (c) {
 		case 'h':
 			usage(argv);
@@ -618,13 +766,44 @@ void trace_split (int argc, char **argv)
 			cpu = atoi(optarg);
 			break;
 		case 'o':
-			if (output)
+			if (!output_file_param)
+				die("Cannot have two output files without an instance in the "
+				    "middle");
+
+			if (output_file_param->output)
 				die("only one output file allowed");
-			output = strdup(optarg);
+
+			output_file_param->output = strdup(optarg);
+
+			/* New output file. */
+			output_file_param = NULL;
 			break;
 		case 'i':
 			input_file = optarg;
 			break;
+		case OPT_top:
+			was_top_instance = true;
+			if (!optarg)
+				output = (char *)default_top_instance_name;
+			else
+				output = optarg;
+			/* Fall through */
+		case 'B':
+			if (!output_file_param) {
+				/* The first name is the main instance. */
+				is_top_instance = true;
+				output_file_param = add_output_file();
+			}
+			if (!output)
+				output = optarg;
+
+			add_output_file_instance(output_file_param, strdup(output),
+						 was_top_instance, is_top_instance);
+
+			output = NULL;
+			was_top_instance = false;
+			is_top_instance = false;
+			break;
 		default:
 			usage(argv);
 		}
@@ -670,19 +849,6 @@ void trace_split (int argc, char **argv)
 
 	page_size = tracecmd_page_size(handle);
 
-	if (!output)
-		output = strdup(input_file);
-
-	if (!repeat && strcmp(output, input_file) == 0) {
-		output = realloc(output, strlen(output) + 3);
-		strcat(output, ".1");
-	}
-
-	output_file = malloc(strlen(output) + 50);
-	if (!output_file)
-		die("Failed to allocate for %s", output);
-	c = 1;
-
 	add_handle(default_top_instance_name, -1, true);
 	instances = tracecmd_buffer_instances(handle);
 	if (instances) {
@@ -697,25 +863,52 @@ void trace_split (int argc, char **argv)
 		}
 	}
 
-	do {
-		if (repeat)
-			sprintf(output_file, "%s.%04d", output, c++);
-		else
-			strcpy(output_file, output);
-			
-		current = parse_file(handle, output_file, start_ns, end_ns,
-				     percpu, cpu, count, type, &end_reached);
+	if (output_file_param && !output_file_param->output)
+		output_file_param->output = strdup(input_file);
 
-		if (!repeat)
-			break;
-		start_ns = 0;
-	} while (!end_reached && (current && (!end_ns || current < end_ns)));
+	map_handle_output();
 
-	free(output);
-	free(output_file);
+	list_for_each_entry(output_file_entry, &output_file_list, list) {
+		output = (char *)output_file_entry->output;
+
+		if (!repeat && strcmp(output, input_file) == 0) {
+			output = realloc(output, strlen(output) + 3);
+			strcat(output, ".1");
+		}
+
+		output_file_entry->output_file = malloc(strlen(output) + 50);
+		if (!output_file_entry->output_file)
+			die("Failed to allocate for %s", output);
+
+		output_file_entry->output = output;
+	}
+
+	list_for_each_entry(output_file_entry, &output_file_list, list) {
+		c = 1;
+		do {
+			if (repeat)
+				sprintf(output_file_entry->output_file, "%s.%04d",
+					output_file_entry->output, c++);
+			else
+				strcpy(output_file_entry->output_file, output_file_entry->output);
+
+			current = parse_file(handle, output_file_entry, start_ns, end_ns, percpu,
+					     cpu, count, type, &end_reached);
+
+			if (!repeat)
+				break;
+			start_ns = 0;
+		} while (!end_reached && (current && (!end_ns || current < end_ns)));
+	}
+
+	list_for_each_entry(output_file_entry, &output_file_list, list) {
+		free((char *)output_file_entry->output);
+		free(output_file_entry->output_file);
+	}
 
 	tracecmd_close(handle);
 	free_handles(&handle_list);
+	free_output_files(&output_file_list);
 
 	return;
 }
-- 
2.25.1

Re: [PATCH v2 6/7] trace-cmd split: Enable support for buffer selection

[Steven Rostedt]

On Mon, 22 Jan 2024 17:43:35 +0100
Pierre Gondois <pierre.gondois@arm.com> wrote:

> The 'trace-cmd split' command conserves all buffers/instances
> of the input .dat file. Add support to select the instances to
> keep and to place as the top instance in an output .dat file.
> 
> Multiple .dat files can be generated with different combinations
> of the instances to keep. The first instance in the command line
> is selected as the top instance in an output .dat file.
> 
> For example, with a trace recorded with:
>   $ trace-cmd record -e sched_wakeup -B switch_instance \
>     -e sched_switch -B timer_instance -e timer

I tried this with a trace.dat file that has a instance called "tast" and it
didn't work. Note, the top instance had no data.

  $ trace-cmd split -i /work/traces/trace-tast.dat -o /tmp/trace-tast2.dat -B tast

  $ trace-cmd report /tmp/trace-tast2.dat
cpus=2
tast:        trace-cmd-3559  [001] 59413.154884: sched_waking:         comm=kworker/u4:2 pid=1324 prio=120 target_cpu=000
tast:        trace-cmd-3564  [000] 59413.154900: sched_switch:         trace-cmd:3564 [120] R ==> kworker/u4:2:1324 [120]
tast:        trace-cmd-3559  [001] 59413.154907: sched_switch:         trace-cmd:3559 [120] S ==> trace-cmd:3565 [120]
tast:     kworker/u4:2-1324  [000] 59413.154911: sched_waking:         comm=sshd pid=18725 prio=120 target_cpu=001
tast:     kworker/u4:2-1324  [000] 59413.154929: sched_switch:         kworker/u4:2:1324 [120] I ==> sshd:18725 [120]
tast:             sshd-18725 [000] 59413.155120: sched_waking:         comm=sslh-fork pid=18724 prio=120 target_cpu=001
tast:             sshd-18725 [000] 59413.155178: sched_waking:         comm=kworker/0:1 pid=815 prio=120 target_cpu=000
tast:             sshd-18725 [000] 59413.155189: sched_switch:         sshd:18725 [120] S ==> sslh-fork:18724 [120]
tast:        sslh-fork-18724 [000] 59413.155303: sched_switch:         sslh-fork:18724 [120] S ==> kworker/0:1:815 [120]

Still has "tast" as an instance and not the top.

-- Steve


> 
> Creating a test.dat file containing the top instance and
> the switch_instance:
>   $ trace-cmd split --top -B switch_instance -o test.dat

Re: [PATCH v2 6/7] trace-cmd split: Enable support for buffer selection

[Steven Rostedt]

On Wed, 24 Jan 2024 14:51:16 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Mon, 22 Jan 2024 17:43:35 +0100
> Pierre Gondois <pierre.gondois@arm.com> wrote:
> 
> > The 'trace-cmd split' command conserves all buffers/instances
> > of the input .dat file. Add support to select the instances to
> > keep and to place as the top instance in an output .dat file.
> > 
> > Multiple .dat files can be generated with different combinations
> > of the instances to keep. The first instance in the command line
> > is selected as the top instance in an output .dat file.
> > 
> > For example, with a trace recorded with:
> >   $ trace-cmd record -e sched_wakeup -B switch_instance \
> >     -e sched_switch -B timer_instance -e timer  
> 
> I tried this with a trace.dat file that has a instance called "tast" and it
> didn't work. Note, the top instance had no data.
> 
>   $ trace-cmd split -i /work/traces/trace-tast.dat -o /tmp/trace-tast2.dat -B tast
> 
>   $ trace-cmd report /tmp/trace-tast2.dat
> cpus=2
> tast:        trace-cmd-3559  [001] 59413.154884: sched_waking:         comm=kworker/u4:2 pid=1324 prio=120 target_cpu=000
> tast:        trace-cmd-3564  [000] 59413.154900: sched_switch:         trace-cmd:3564 [120] R ==> kworker/u4:2:1324 [120]
> tast:        trace-cmd-3559  [001] 59413.154907: sched_switch:         trace-cmd:3559 [120] S ==> trace-cmd:3565 [120]
> tast:     kworker/u4:2-1324  [000] 59413.154911: sched_waking:         comm=sshd pid=18725 prio=120 target_cpu=001
> tast:     kworker/u4:2-1324  [000] 59413.154929: sched_switch:         kworker/u4:2:1324 [120] I ==> sshd:18725 [120]
> tast:             sshd-18725 [000] 59413.155120: sched_waking:         comm=sslh-fork pid=18724 prio=120 target_cpu=001
> tast:             sshd-18725 [000] 59413.155178: sched_waking:         comm=kworker/0:1 pid=815 prio=120 target_cpu=000
> tast:             sshd-18725 [000] 59413.155189: sched_switch:         sshd:18725 [120] S ==> sslh-fork:18724 [120]
> tast:        sslh-fork-18724 [000] 59413.155303: sched_switch:         sslh-fork:18724 [120] S ==> kworker/0:1:815 [120]
> 
> Still has "tast" as an instance and not the top.

I replied to the wrong email. This should have been sent to the v3 version.

-- Steve

[PATCH v2 7/7] trace-cmd split: Update usage

[Pierre Gondois]

Update usage for 'trace-cmd split' command:
- Add missing description of -i/-C options
- Add description of the newly enabled -B/--top options

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 tracecmd/trace-usage.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
index 6944a2c7..e5b84733 100644
--- a/tracecmd/trace-usage.c
+++ b/tracecmd/trace-usage.c
@@ -304,13 +304,20 @@ static struct usage_help usage_help[] = {
 	{
 		"split",
 		"parse a trace.dat file into smaller file(s)",
-		" %s split [options] -o file [start [end]]\n"
+		" %s split [-i file] [options] [-B buffer] [--top[=name]] -o file [start [end]]\n"
 		"          -o output file to write to (file.1, file.2, etc)\n"
 		"          -s n  split file up by n seconds\n"
 		"          -m n  split file up by n milliseconds\n"
 		"          -u n  split file up by n microseconds\n"
 		"          -e n  split file up by n events\n"
 		"          -p n  split file up by n pages\n"
+		"          -C n  select CPU n\n"
+		"          -B buffer    keep buffer in resulting .dat file\n"
+		"                       The first buffer specified will be the top buffer.\n"
+		"                       Use --top option to select the top buffer of the input\n"
+		"                       .dat file.\n"
+		"          --top[=name] keep top buffer in resulting .dat file.\n"
+		"                       If specified, rename the top buffer to TOP.\n"
 		"          -r    repeat from start to end\n"
 		"          -c    per cpu, that is -p 2 will be 2 pages for each CPU\n"
 		"          if option is specified, it will split the file\n"
-- 
2.25.1