diff mbox series

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

Message ID 20240122164336.167256-3-pierre.gondois@arm.com (mailing list archive)
State Superseded
Commit 1439b8f5189414b259905102da23446ba78db033
Headers show
Series trace-cmd: split: Handle splitting files with multiple instances | expand

Commit Message

Pierre Gondois Jan. 22, 2024, 4:43 p.m. UTC 
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(-)

Comments

Steven Rostedt Jan. 23, 2024, 1:51 a.m. UTC | #1 
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
Steven Rostedt Jan. 24, 2024, 5:28 p.m. UTC | #2 
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
diff mbox series

Patch

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);