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

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

[Pierre Gondois]

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.

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

Pierre Gondois (5):
  trace-cmd: split: Small fixes
  trace-cmd: usage: Update usage for trace-cmd split
  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

 tracecmd/trace-split.c | 231 +++++++++++++++++++++++++++++------------
 tracecmd/trace-usage.c |   3 +-
 2 files changed, 167 insertions(+), 67 deletions(-)

-- 
2.25.1

[PATCH 1/5] 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 2/5] trace-cmd: usage: Update usage for trace-cmd split

[Pierre Gondois]

Update usage for 'trace-cmd split' command.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 tracecmd/trace-split.c | 21 +++++++++++++--------
 tracecmd/trace-usage.c |  3 ++-
 2 files changed, 15 insertions(+), 9 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);
diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
index 61acce5f..433928d3 100644
--- a/tracecmd/trace-usage.c
+++ b/tracecmd/trace-usage.c
@@ -303,13 +303,14 @@ 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] -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"
 		"          -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

[PATCH 3/5] 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 a
add_handle() function.

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

diff --git a/tracecmd/trace-split.c b/tracecmd/trace-split.c
index b6c056b5..f2edfbfc 100644
--- a/tracecmd/trace-split.c
+++ b/tracecmd/trace-split.c
@@ -19,6 +19,7 @@
 #include <ctype.h>
 #include <errno.h>
 
+#include "list.h"
 #include "trace-local.h"
 
 static unsigned int page_size;
@@ -49,6 +50,26 @@ struct cpu_data {
 	char				*file;
 };
 
+struct handle_list {
+	struct list_head		list;
+	const char			*name;
+	struct tracecmd_input		*handle;
+};
+
+static struct list_head handle_list;
+
+static void add_handle(struct tracecmd_input *handle, const char *name)
+{
+	struct handle_list *item;
+
+	item = calloc(1, sizeof(*item));
+	if (!item)
+		die("Failed ot allocate");
+	item->handle = handle;
+	item->name = name;
+	list_add_tail(&item->list, &handle_list);
+}
+
 static int create_type_len(struct tep_handle *pevent, int time, int len)
 {
 	static int bigendian = -1;
@@ -450,6 +471,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 +479,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 +585,26 @@ void trace_split (int argc, char **argv)
 		die("Failed to allocate for %s", output);
 	c = 1;
 
+	add_handle(handle, NULL);
+	instances = tracecmd_buffer_instances(handle);
+	if (instances) {
+		struct tracecmd_input *new_handle;
+		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");
+			new_handle = tracecmd_buffer_instance_handle(handle, i);
+			if (!new_handle) {
+				warning("could not retrieve handle %s", name);
+				continue;
+			}
+			add_handle(new_handle, name);
+		}
+	}
+
 	do {
 		if (repeat)
 			sprintf(output_file, "%s.%04d", output, c++);
-- 
2.25.1

[PATCH 4/5] 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 | 71 +++++++++++++++++++++++++++++++++---------
 1 file changed, 56 insertions(+), 15 deletions(-)

diff --git a/tracecmd/trace-split.c b/tracecmd/trace-split.c
index f2edfbfc..6ab138a2 100644
--- a/tracecmd/trace-split.c
+++ b/tracecmd/trace-split.c
@@ -371,6 +371,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,
@@ -384,19 +430,12 @@ 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 ret;
 	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);
@@ -405,11 +444,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;
@@ -448,12 +485,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

[PATCH 5/5] 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)

Make use of the previous patches to split all the instances of
a trace.

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

diff --git a/tracecmd/trace-split.c b/tracecmd/trace-split.c
index 6ab138a2..37ce4176 100644
--- a/tracecmd/trace-split.c
+++ b/tracecmd/trace-split.c
@@ -426,6 +426,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;
@@ -437,63 +438,71 @@ static unsigned long long parse_file(struct tracecmd_input *handle,
 	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) {
+		cpus = tracecmd_cpus(handle_entry->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(handle_entry->handle, cpu, start);
+		}
+
+		if (only_cpu >= 0) {
+			parse_cpu(handle_entry->handle, cpu_data, start, end, count,
+				  1, only_cpu, type, end_reached);
+		} else if (percpu) {
+			for (cpu = 0; cpu < cpus; cpu++)
+				parse_cpu(handle_entry->handle, cpu_data, start,
+					  end, count, percpu, cpu, type, end_reached);
+		} else {
+			parse_cpu(handle_entry->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->name)
+			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(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);
 	}
-	free(cpu_data);
-	free(cpu_list);
 
 	tracecmd_output_close(ohandle);
 
-- 
2.25.1

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

[Steven Rostedt]

On Fri, 12 Jan 2024 09:39:40 +0100
Pierre Gondois <pierre.gondois@arm.com> wrote:

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

Hi Pierre!

Thanks for sending these.

> 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.
> 
> Also:
> - Fix a side issue preventing to provide start/end timestamps
>   along with a time-window parameters.
> - Update trace-cmd split usage
> 
> Pierre Gondois (5):
>   trace-cmd: split: Small fixes

One small nit, and you do not need to resend for this, but for future
patches. You do not need the colon after trace-cmd. That is:


  trace-cmd split: Small fixes

is good enough. In Linux some sub-systems use this method for when a patch
affects two sub-systems.

  <subsystemA>: <subsystemB>: Subject

But here it's representing the actual command.

    "trace-cmd split" is being fixed.


>   trace-cmd: usage: Update usage for trace-cmd split

If just fixing the usage of a single command, I would have that be:

   trace-cmd split: Update usage to include input file and CPU select

-- Steve

>   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
> 
>  tracecmd/trace-split.c | 231 +++++++++++++++++++++++++++++------------
>  tracecmd/trace-usage.c |   3 +-
>  2 files changed, 167 insertions(+), 67 deletions(-)
> 

Re: [PATCH 2/5] trace-cmd: usage: Update usage for trace-cmd split

[Steven Rostedt]

On Fri, 12 Jan 2024 09:39:42 +0100
Pierre Gondois <pierre.gondois@arm.com> wrote:

> Update usage for 'trace-cmd split' command.

I mentioned in my reply to the cover letter about the subject, but also
having more detail in the change log is good. What was missing from the
usage that needed to be fixed?

> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  tracecmd/trace-split.c | 21 +++++++++++++--------
>  tracecmd/trace-usage.c |  3 ++-
>  2 files changed, 15 insertions(+), 9 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);

The above looks like it has nothing to do with the usage, and was perhaps
accidentally merged in this patch?

-- Steve


> diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
> index 61acce5f..433928d3 100644
> --- a/tracecmd/trace-usage.c
> +++ b/tracecmd/trace-usage.c
> @@ -303,13 +303,14 @@ 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] -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"
>  		"          -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"

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

[Steven Rostedt]

On Fri, 12 Jan 2024 09:39:45 +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)
> 


> Make use of the previous patches to split all the instances of
> a trace.

This shouldn't be in the change log. As the change log is for history.
Think about reading this 5 years from now. Would it make sense about
"previous patches"?

Thanks for doing this, I'll try to set some time next week to review them.
I want to release 3.3 soon.

Would you be able to add a way to split out an instance into its own
trace.dat file?  Or to choose what you want.

 $ trace-cmd record -e sched_wakeup -B test_instance -e sched_switch

 $ trace-cmd split -B test_instance -o test.dat

Would make test_instance the main buffer in test.dat.

If you add more than one instance:

 $ trace-cmd record -e sched_wakeup -B test_instance -e sched_switch -B timer_instance -e timer

 $ trace-cmd split -B test_instance -o test.dat -B timer_instance

This would make "test_instance" the main buffer, and keep "timer_instance"
as an instance.

The "-o file" placement is important. It will make whatever came before it
the main buffer. And perhaps even split out more than one!

 $ trace-cmd split -B test_instance -o test.dat -B timer_instance -o timer.dat

Would place the "test_instance" as the main instance in test.dat, and the
"timer_instance" as the main instance in timer.dat.

Thoughts?

-- Steve



> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218357
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>

Re: [PATCH 2/5] trace-cmd: usage: Update usage for trace-cmd split

[Pierre Gondois]

Hello Steven,

On 1/12/24 16:37, Steven Rostedt wrote:
> On Fri, 12 Jan 2024 09:39:42 +0100
> Pierre Gondois <pierre.gondois@arm.com> wrote:
> 
>> Update usage for 'trace-cmd split' command.
> 
> I mentioned in my reply to the cover letter about the subject, but also
> having more detail in the change log is good. What was missing from the
> usage that needed to be fixed?

The patch just intended to add available parameters that were missing
from the documentation.

> 
>>
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>   tracecmd/trace-split.c | 21 +++++++++++++--------
>>   tracecmd/trace-usage.c |  3 ++-
>>   2 files changed, 15 insertions(+), 9 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);
> 
> The above looks like it has nothing to do with the usage, and was perhaps
> accidentally merged in this patch?

Yes right,
something wrong happened while rebasing, sorry for that...

> 
> -- Steve
> 
> 
>> diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
>> index 61acce5f..433928d3 100644
>> --- a/tracecmd/trace-usage.c
>> +++ b/tracecmd/trace-usage.c
>> @@ -303,13 +303,14 @@ 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] -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"
>>   		"          -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"
> 

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

[Pierre Gondois]

Hello Steven,

On 1/12/24 17:18, Steven Rostedt wrote:
> On Fri, 12 Jan 2024 09:39:45 +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)
>>
> 
> 
>> Make use of the previous patches to split all the instances of
>> a trace.
> 
> This shouldn't be in the change log. As the change log is for history.
> Think about reading this 5 years from now. Would it make sense about
> "previous patches"?

I thought it was acceptable to do so as the previous patches would
be just before in the git log, but ok I will write something better.

> 
> Thanks for doing this, I'll try to set some time next week to review them.
> I want to release 3.3 soon.
> 
> Would you be able to add a way to split out an instance into its own
> trace.dat file?  Or to choose what you want.
> 
>   $ trace-cmd record -e sched_wakeup -B test_instance -e sched_switch
> 
>   $ trace-cmd split -B test_instance -o test.dat
> 
> Would make test_instance the main buffer in test.dat.
> 
> If you add more than one instance:
> 
>   $ trace-cmd record -e sched_wakeup -B test_instance -e sched_switch -B timer_instance -e timer
> 
>   $ trace-cmd split -B test_instance -o test.dat -B timer_instance
> 
> This would make "test_instance" the main buffer, and keep "timer_instance"
> as an instance.
> 
> The "-o file" placement is important. It will make whatever came before it
> the main buffer. And perhaps even split out more than one!
> 
>   $ trace-cmd split -B test_instance -o test.dat -B timer_instance -o timer.dat
> 
> Would place the "test_instance" as the main instance in test.dat, and the
> "timer_instance" as the main instance in timer.dat.
> 
> Thoughts?

This should be possible, I will try to make an update in this direction,

Regards,
Pierre

> 
> -- Steve
> 
> 
> 
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218357
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>

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

[Steven Rostedt]

On Mon, 15 Jan 2024 18:25:46 +0100
Pierre Gondois <pierre.gondois@arm.com> wrote:

> >   
> >> Make use of the previous patches to split all the instances of
> >> a trace.  
> > 
> > This shouldn't be in the change log. As the change log is for history.
> > Think about reading this 5 years from now. Would it make sense about
> > "previous patches"?  
> 
> I thought it was acceptable to do so as the previous patches would
> be just before in the git log, but ok I will write something better.

The thing is, what does the previous patches do that's important for
this change? If I see this in a change log, I may look at the previous
patches, but note that the order is not always the same. I could have
imported a patch with a date on it that happens to go between the two
and cause the history to be different. But that's usually in merged
commits which I avoid here, but still.

Sometimes in a change log I'll put:

 "Now that X was done, we can do Y"

Where X is done by previous patches, and if someone is curious, they
can go and see X. But just saying "Make use of previous patches"
doesn't tell me what those previous patches did. And going back and
looking at them, I still don't know exactly how those previous patches
are related to this change, except that it did some clean up work.

-- Steve

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

[Pierre Gondois]

Hello Steven,

On 1/12/24 17:18, Steven Rostedt wrote:
> On Fri, 12 Jan 2024 09:39:45 +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)
>>
> 
> 
>> Make use of the previous patches to split all the instances of
>> a trace.
> 
> This shouldn't be in the change log. As the change log is for history.
> Think about reading this 5 years from now. Would it make sense about
> "previous patches"?
> 
> Thanks for doing this, I'll try to set some time next week to review them.
> I want to release 3.3 soon.
> 
> Would you be able to add a way to split out an instance into its own
> trace.dat file?  Or to choose what you want.
> 
>   $ trace-cmd record -e sched_wakeup -B test_instance -e sched_switch
> 
>   $ trace-cmd split -B test_instance -o test.dat
> 
> Would make test_instance the main buffer in test.dat.
> 
> If you add more than one instance:
> 
>   $ trace-cmd record -e sched_wakeup -B test_instance -e sched_switch -B timer_instance -e timer
> 
>   $ trace-cmd split -B test_instance -o test.dat -B timer_instance
> 
> This would make "test_instance" the main buffer, and keep "timer_instance"
> as an instance.
> 
> The "-o file" placement is important. It will make whatever came before it
> the main buffer. And perhaps even split out more than one!
> 
>   $ trace-cmd split -B test_instance -o test.dat -B timer_instance -o timer.dat
> 
> Would place the "test_instance" as the main instance in test.dat, and the
> "timer_instance" as the main instance in timer.dat.

I think it might be difficult to select the main instance and name it in
the output file this way. As it has no specific name. I suggest to also add a '-b' parameter to designate
the main buffer. So with the following record command:

	$ trace-cmd record -e sched_wakeup -B switch_instance -e sched_switch -B timer_instance -e timer

	$ trace-cmd split -B switch_instance -B timer_instance -o test.dat
test.dat:
- switch_instance as the main instance
- timer_instance as a side instance (still named 'timer_instance')

	$ trace-cmd split -b -B switch_instance -o test.dat
test.dat:
- main instance as the main instance
- switch_instance as a side instance (still named 'switch_instance')

	$ trace-cmd split -B switch_instance -b -o test.dat
test.dat:
- switch_instance as the main instance
- main instance as a side instance (named 'main' [1])

	$ trace-cmd split -B switch_instance -o test.dat -B timer_instance
test.dat:
- switch_instance as the main instance
trace.dat.1
- timer_instance as the main instance.
   No output file is specified, so the default name is used as the output name.

	$ trace-cmd split -B switch_instance -o test.dat -b
test.dat:
- switch_instance as the main instance
trace.dat.1:
- main instance as the main instance.
   No output file is specified, so the default name is used as the output name.

	$ trace-cmd split -B switch_instance -o switch.dat -b -o main.dat -B timer_instance -o timer.dat
switch.dat:
- switch_instance as the main instance
main.dat:
- main instance as the main instance
timer.dat:
- timer_instance as the main instance

[1]
As the main buffer doesn't have a default name, it might be necessary
to hardcode the name, as 'main', or as 'main.X' (X being an increasing number)
if there is already a main instance in the trace

Does it seems ok to you ?

Regards,
Pierre

> 
> Thoughts?
> 
> -- Steve
> 
> 
> 
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218357
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>

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

[Pierre Gondois]

On 1/19/24 17:41, Pierre Gondois wrote:
> Hello Steven,
> 
> On 1/12/24 17:18, Steven Rostedt wrote:
>> On Fri, 12 Jan 2024 09:39:45 +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)
>>>
>>
>>
>>> Make use of the previous patches to split all the instances of
>>> a trace.
>>
>> This shouldn't be in the change log. As the change log is for history.
>> Think about reading this 5 years from now. Would it make sense about
>> "previous patches"?
>>
>> Thanks for doing this, I'll try to set some time next week to review them.
>> I want to release 3.3 soon.
>>
>> Would you be able to add a way to split out an instance into its own
>> trace.dat file?  Or to choose what you want.
>>
>>    $ trace-cmd record -e sched_wakeup -B test_instance -e sched_switch
>>
>>    $ trace-cmd split -B test_instance -o test.dat
>>
>> Would make test_instance the main buffer in test.dat.
>>
>> If you add more than one instance:
>>
>>    $ trace-cmd record -e sched_wakeup -B test_instance -e sched_switch -B timer_instance -e timer
>>
>>    $ trace-cmd split -B test_instance -o test.dat -B timer_instance
>>
>> This would make "test_instance" the main buffer, and keep "timer_instance"
>> as an instance.
>>
>> The "-o file" placement is important. It will make whatever came before it
>> the main buffer. And perhaps even split out more than one!
>>
>>    $ trace-cmd split -B test_instance -o test.dat -B timer_instance -o timer.dat
>>
>> Would place the "test_instance" as the main instance in test.dat, and the
>> "timer_instance" as the main instance in timer.dat.
> 
> I think it might be difficult to select the main instance and name it in
> the output file this way. As it has no specific name. I suggest to also add a '-b' parameter to designate
> the main buffer. So with the following record command:
> 
> 	$ trace-cmd record -e sched_wakeup -B switch_instance -e sched_switch -B timer_instance -e timer
> 
> 	$ trace-cmd split -B switch_instance -B timer_instance -o test.dat
> test.dat:
> - switch_instance as the main instance
> - timer_instance as a side instance (still named 'timer_instance')
> 
> 	$ trace-cmd split -b -B switch_instance -o test.dat
> test.dat:
> - main instance as the main instance
> - switch_instance as a side instance (still named 'switch_instance')
> 
> 	$ trace-cmd split -B switch_instance -b -o test.dat
> test.dat:
> - switch_instance as the main instance
> - main instance as a side instance (named 'main' [1])
> 
> 	$ trace-cmd split -B switch_instance -o test.dat -B timer_instance
> test.dat:
> - switch_instance as the main instance
> trace.dat.1
> - timer_instance as the main instance.
>     No output file is specified, so the default name is used as the output name.
> 
> 	$ trace-cmd split -B switch_instance -o test.dat -b
> test.dat:
> - switch_instance as the main instance
> trace.dat.1:
> - main instance as the main instance.
>     No output file is specified, so the default name is used as the output name.
> 
> 	$ trace-cmd split -B switch_instance -o switch.dat -b -o main.dat -B timer_instance -o timer.dat
> switch.dat:
> - switch_instance as the main instance
> main.dat:
> - main instance as the main instance
> timer.dat:
> - timer_instance as the main instance
> 
> [1]
> As the main buffer doesn't have a default name, it might be necessary
> to hardcode the name, as 'main', or as 'main.X' (X being an increasing number)
> if there is already a main instance in the trace


It would also mean that:
	$ trace-cmd split -B switch_instance -b -o switch.dat -B timer_instance -b -o timer.dat
switch.dat:
- switch_instance as the main instance
- main instance as a side instance, named 'main'
timer.dat:
- timer_instance as the main instance
- main instance as a side instance, named 'main'

And the usage of the function would be:
	$ trace-cmd split [-i file] [options] [[-b -B instance] -o file] [start [end]]
I.e. the options and start/end parameters would be common to all instances/output files.

> 
> Does it seems ok to you ?
> 
> Regards,
> Pierre
> 
>>
>> Thoughts?
>>
>> -- Steve
>>
>>
>>
>>>
>>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218357
>>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>>> ---
>>>

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

[Steven Rostedt]

On Fri, 19 Jan 2024 18:06:43 +0100
Pierre Gondois <pierre.gondois@arm.com> wrote:

> > 	$ trace-cmd split -B switch_instance -o switch.dat -b -o main.dat -B timer_instance -o timer.dat
> > switch.dat:
> > - switch_instance as the main instance
> > main.dat:
> > - main instance as the main instance
> > timer.dat:
> > - timer_instance as the main instance
> > 
> > [1]
> > As the main buffer doesn't have a default name, it might be necessary
> > to hardcode the name, as 'main', or as 'main.X' (X being an increasing number)
> > if there is already a main instance in the trace  
> 
> 
> It would also mean that:
> 	$ trace-cmd split -B switch_instance -b -o switch.dat -B timer_instance -b -o timer.dat
> switch.dat:
> - switch_instance as the main instance
> - main instance as a side instance, named 'main'
> timer.dat:
> - timer_instance as the main instance
> - main instance as a side instance, named 'main'
> 
> And the usage of the function would be:
> 	$ trace-cmd split [-i file] [options] [[-b -B instance] -o file] [start [end]]
> I.e. the options and start/end parameters would be common to all instances/output files.

I usually use the term "top" not "main" for the top instance. But I
wouldn't want to have a default name anyway.

I would just add "--top" instead of '-b' an the main instance. I say "top"
as that's the common terminology I have used in man pages. Although, I have
used "main" too, I usually append "(top level)" when I do so.

We could call it "--main" too, if it sounds better, but should definitely
document that it means the "top level instance". Hmm, maybe even '-M' could
work.

As for naming, if someone wants to make the main instance an instance, then
they must also supply a name.

  $ trace-cmd split -B switch_instance -M --name 'old_main' -o trace-spit.dat

And that would switch the main and switch_instance in trace-split.dat.

Doesn't mean we can't have both "-M" and "--main" mean the same thing. I
want to keep instance single options capitalized.

Thanks,

-- Steve

Re: [PATCH 2/5] trace-cmd: usage: Update usage for trace-cmd split

[Steven Rostedt]

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

> > The above looks like it has nothing to do with the usage, and was perhaps
> > accidentally merged in this patch?  
> 
> Yes right,
> something wrong happened while rebasing, sorry for that...

Ah can you send a v2 on this patch series.

For the fixes I can pull them immediately. The updates to the split
functionality can come in a separate patch set.

-- Steve