linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] libtraceeval task-eval: Updates to evaluate tasks
@ 2023-10-11  3:25 Steven Rostedt
  2023-10-11  3:25 ` [PATCH 01/11] libtraceeval task-eval: Fix naming conventions of variables Steven Rostedt
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Steven Rostedt @ 2023-10-11  3:25 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Ross Zwisler, Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

This series cleans up task-eval and makes it more useful.

Steven Rostedt (Google) (11):
  libtraceeval task-eval: Fix naming conventions of variables
  libtraceeval task-eval: Use "assign_*keys/vals()" helper functions
  libtraceeval task-eval: Add tracking of priority
  libtraceeval task-eval: Add way to read trace.dat buffer instances
  libtraceeval task-eval: Account for ZOMBIE and EXITED states
  libtraceeval task-eval: Show max, min and count of deltas too
  libtraceeval task-eval: Add more comments
  libtraceeval task-eval: Only release results if found
  libtraceeval task-eval: Do not add pdata to non RUNNING tasks
  libtraceveal task-eval: Clean up all traceeval_iterators
  libtraceeval task-evals: Free the libtraceevals

 samples/task-eval.c | 496 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 393 insertions(+), 103 deletions(-)

-- 
2.42.0


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

* [PATCH 01/11] libtraceeval task-eval: Fix naming conventions of variables
  2023-10-11  3:25 [PATCH 00/11] libtraceeval task-eval: Updates to evaluate tasks Steven Rostedt
@ 2023-10-11  3:25 ` Steven Rostedt
  2023-10-11  3:25 ` [PATCH 02/11] libtraceeval task-eval: Use "assign_*keys/vals()" helper functions Steven Rostedt
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2023-10-11  3:25 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Ross Zwisler, Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The naming is a bit confusing and makes it difficult to get the assigning
of the types and values of the traceevals correct. For instance, we have:

  data->teval_tasks

that is defined by process_types and process_keys.

Instead of calling it "process", call it "tasks" to match what its for.
That is, task_types and task_keys.

Also, instead of using "delta_vals" for the cpu_delta, call it
cpu_delta_vals and use that. The type and value definitions should have
the name of the teval it is defining.

Add "cpu_vals" and "thread_vals" instead of using a generic "delta_type"
for both, which is also confusing.

Finally, add a DELTA_NAME macro that is defined as "delta" which is the
name of the delta elements throughout the traceevals. This is the only
element that gets assigned a libtraceeval_stat. As retrieving a stat is
now done by name, it is much easier to just pass DELTA_NAME to it, and
guarantee that you are getting the right stat, instead of passing in the
vals[0].name, which could be wrong (and was in some cases, but luckily
what was used was the wrong value definition, but the right array index
which by luck gave the right name to use).

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 samples/task-eval.c | 66 ++++++++++++++++++++++++++-------------------
 1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/samples/task-eval.c b/samples/task-eval.c
index 9c3bc7c6ef1e..3d748d16ee57 100644
--- a/samples/task-eval.c
+++ b/samples/task-eval.c
@@ -80,6 +80,9 @@ void pdie(const char *fmt, ...)
 	va_end(ap);
 }
 
+/* Used for stats */
+#define DELTA_NAME		"delta"
+
 static struct traceeval_type cpu_delta_keys[] = {
 	{
 		.type = TRACEEVAL_TYPE_NUMBER,
@@ -87,6 +90,13 @@ static struct traceeval_type cpu_delta_keys[] = {
 	},
 };
 
+static struct traceeval_type cpu_delta_vals[] = {
+	{
+		.type = TRACEEVAL_TYPE_NUMBER,
+		.name = "Schedule state",
+	},
+};
+
 static struct traceeval_type cpu_keys[] = {
 	{
 		.type = TRACEEVAL_TYPE_NUMBER,
@@ -98,14 +108,21 @@ static struct traceeval_type cpu_keys[] = {
 	},
 };
 
-static struct traceeval_type process_delta_keys[] = {
+static struct traceeval_type cpu_vals[] = {
+	{
+		.type = TRACEEVAL_TYPE_DELTA,
+		.name = DELTA_NAME,
+	},
+};
+
+static struct traceeval_type task_delta_keys[] = {
 	{
 		.type = TRACEEVAL_TYPE_NUMBER,
 		.name = "PID",
 	},
 };
 
-static struct traceeval_type process_delta_vals[] = {
+static struct traceeval_type task_delta_vals[] = {
 	{
 		.type = TRACEEVAL_TYPE_NUMBER,
 		.name = "Schedule state"
@@ -116,14 +133,7 @@ static struct traceeval_type process_delta_vals[] = {
 	},
 };
 
-static struct traceeval_type delta_vals[] = {
-	{
-		.type = TRACEEVAL_TYPE_NUMBER,
-		.name = "Schedule state"
-	},
-};
-
-static struct traceeval_type process_keys[] = {
+static struct traceeval_type task_keys[] = {
 	{
 		.type = TRACEEVAL_TYPE_STRING,
 		.name = "COMM"
@@ -134,14 +144,14 @@ static struct traceeval_type process_keys[] = {
 	},
 };
 
-static struct traceeval_type process_vals[] = {
+static struct traceeval_type task_vals[] = {
 	{
 		.type = TRACEEVAL_TYPE_POINTER,
 		.name = "data",
 	},
 	{
 		.type = TRACEEVAL_TYPE_DELTA,
-		.name = "delta",
+		.name = DELTA_NAME,
 	},
 };
 
@@ -156,10 +166,10 @@ static struct traceeval_type thread_keys[] = {
 	},
 };
 
-static struct traceeval_type delta_type[] = {
+static struct traceeval_type thread_vals[] = {
 	{
 		.type = TRACEEVAL_TYPE_DELTA,
-		.name = "delta",
+		.name = DELTA_NAME,
 	},
 };
 
@@ -191,11 +201,11 @@ enum command {
 
 static void init_process_data(struct process_data *pdata)
 {
-	pdata->teval_cpus = traceeval_init(cpu_keys, delta_type);
+	pdata->teval_cpus = traceeval_init(cpu_keys, cpu_vals);
 	if (!pdata->teval_cpus)
 		pdie("Creating trace eval cpus");
 
-	pdata->teval_threads = traceeval_init(thread_keys, delta_type);
+	pdata->teval_threads = traceeval_init(thread_keys, thread_vals);
 	if (!pdata->teval_threads)
 		pdie("Creating trace eval threads");
 }
@@ -421,7 +431,7 @@ static void sched_out(struct task_data *tdata, const char *comm,
 
 	ret = tep_read_number_field(prev_pid, record->data, &val);
 	if (ret < 0)
-		die("Could not read sched_switch next_pid for record");
+		die("Could not read sched_switch prev_pid for record");
 	pid = val;
 
 	/* Idle is handled by sched_in() */
@@ -587,11 +597,11 @@ static int compare_pdata(struct traceeval *teval,
 	}
 
 	/* Get the RUNNING values for both processes */
-	statA = traceeval_stat(teval, keysA, delta_type[0].name);
+	statA = traceeval_stat(teval, keysA, DELTA_NAME);
 	if (statA)
 		totalA = traceeval_stat_total(statA);
 
-	statB = traceeval_stat(teval, keysB, delta_type[0].name);
+	statB = traceeval_stat(teval, keysB, DELTA_NAME);
 	if (statB)
 		totalB = traceeval_stat_total(statB);
 
@@ -622,7 +632,7 @@ static void display_cpus(struct traceeval *teval)
 		int state = keys[1].number;
 		int cpu = keys[0].number;
 
-		stat = traceeval_iterator_stat(iter, delta_type[0].name);
+		stat = traceeval_iterator_stat(iter, DELTA_NAME);
 		if (!stat)
 			continue; // die?
 
@@ -688,7 +698,7 @@ static void display_threads(struct traceeval *teval)
 		int state = keys[1].number;
 		int tid = keys[0].number;
 
-		stat = traceeval_iterator_stat(iter, delta_type[0].name);
+		stat = traceeval_iterator_stat(iter, DELTA_NAME);
 		if (!stat)
 			continue; // die?
 
@@ -725,7 +735,7 @@ static void display_process_stats(struct traceeval *teval,
 		TRACEEVAL_SET_NUMBER(keys[1], i);
 
 		delta = 0;
-		stat = traceeval_stat(teval, keys, delta_type[0].name);
+		stat = traceeval_stat(teval, keys, DELTA_NAME);
 		if (stat)
 			delta = traceeval_stat_total(stat);
 		display_state_times(i, delta);
@@ -789,7 +799,7 @@ static void display(struct task_data *tdata)
 	while (traceeval_iterator_next(iter, &keys) > 0) {
 		int state = keys[1].number;
 
-		stat = traceeval_iterator_stat(iter, delta_type[0].name);
+		stat = traceeval_iterator_stat(iter, DELTA_NAME);
 		if (!stat)
 			continue;
 
@@ -895,19 +905,19 @@ int main (int argc, char **argv)
 	if (!handle)
 		pdie("Error opening %s", argv[0]);
 
-	data.teval_tasks = traceeval_init(process_keys, process_vals);
+	data.teval_tasks = traceeval_init(task_keys, task_vals);
 	if (!data.teval_tasks)
 		pdie("Creating trace eval processe data");
 
-	if (traceeval_delta_create(data.teval_tasks, process_delta_keys,
-				   process_delta_vals) < 0)
+	if (traceeval_delta_create(data.teval_tasks, task_delta_keys,
+				   task_delta_vals) < 0)
 		pdie("Creating trace delta threads");
 
-	data.teval_cpus = traceeval_init(cpu_keys, delta_type);
+	data.teval_cpus = traceeval_init(cpu_keys, cpu_vals);
 	if (!data.teval_cpus)
 		pdie("Creating trace eval");
 
-	if (traceeval_delta_create(data.teval_cpus, cpu_delta_keys, delta_vals) < 0)
+	if (traceeval_delta_create(data.teval_cpus, cpu_delta_keys, cpu_delta_vals) < 0)
 		pdie("Creating trace delta cpus");
 
 	tracecmd_follow_event(handle, "sched", "sched_switch", switch_func, &data);
-- 
2.42.0


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

* [PATCH 02/11] libtraceeval task-eval: Use "assign_*keys/vals()" helper functions
  2023-10-11  3:25 [PATCH 00/11] libtraceeval task-eval: Updates to evaluate tasks Steven Rostedt
  2023-10-11  3:25 ` [PATCH 01/11] libtraceeval task-eval: Fix naming conventions of variables Steven Rostedt
@ 2023-10-11  3:25 ` Steven Rostedt
  2023-10-11  3:25 ` [PATCH 03/11] libtraceeval task-eval: Add tracking of priority Steven Rostedt
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2023-10-11  3:25 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Ross Zwisler, Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Assigning the keys and values at each location for the traceevals has
become error prone, and a lot of cut and pasting. Instead, create helper
functions to do this instead, and it simplifies the code as well as making
it less error prone.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 samples/task-eval.c | 122 +++++++++++++++++++++++++++++++-------------
 1 file changed, 86 insertions(+), 36 deletions(-)

diff --git a/samples/task-eval.c b/samples/task-eval.c
index 3d748d16ee57..11ea73ce8da0 100644
--- a/samples/task-eval.c
+++ b/samples/task-eval.c
@@ -90,6 +90,11 @@ static struct traceeval_type cpu_delta_keys[] = {
 	},
 };
 
+static void assign_cpu_delta_keys(struct traceeval_data keys[1], int cpu)
+{
+	TRACEEVAL_SET_NUMBER(keys[0], cpu);
+}
+
 static struct traceeval_type cpu_delta_vals[] = {
 	{
 		.type = TRACEEVAL_TYPE_NUMBER,
@@ -97,6 +102,11 @@ static struct traceeval_type cpu_delta_vals[] = {
 	},
 };
 
+static void assign_cpu_delta_vals(struct traceeval_data vals[1], int state)
+{
+	TRACEEVAL_SET_NUMBER(vals[0], state);
+}
+
 static struct traceeval_type cpu_keys[] = {
 	{
 		.type = TRACEEVAL_TYPE_NUMBER,
@@ -108,6 +118,12 @@ static struct traceeval_type cpu_keys[] = {
 	},
 };
 
+static void assign_cpu_keys(struct traceeval_data keys[2], int cpu, int state)
+{
+	TRACEEVAL_SET_NUMBER(keys[0], cpu);
+	TRACEEVAL_SET_NUMBER(keys[1], state);
+}
+
 static struct traceeval_type cpu_vals[] = {
 	{
 		.type = TRACEEVAL_TYPE_DELTA,
@@ -115,6 +131,13 @@ static struct traceeval_type cpu_vals[] = {
 	},
 };
 
+static void assign_cpu_vals(struct traceeval_data vals[1],
+			    unsigned long long delta,
+			    unsigned long long timestamp)
+{
+	TRACEEVAL_SET_DELTA(vals[0], delta, timestamp);
+}
+
 static struct traceeval_type task_delta_keys[] = {
 	{
 		.type = TRACEEVAL_TYPE_NUMBER,
@@ -122,6 +145,11 @@ static struct traceeval_type task_delta_keys[] = {
 	},
 };
 
+static void assign_task_delta_keys(struct traceeval_data keys[1], int pid)
+{
+	TRACEEVAL_SET_NUMBER(keys[0], pid);
+}
+
 static struct traceeval_type task_delta_vals[] = {
 	{
 		.type = TRACEEVAL_TYPE_NUMBER,
@@ -133,6 +161,13 @@ static struct traceeval_type task_delta_vals[] = {
 	},
 };
 
+static void assign_task_delta_vals(struct traceeval_data vals[2],
+				   int state, const char *comm)
+{
+	TRACEEVAL_SET_NUMBER(vals[0], state);
+	TRACEEVAL_SET_CSTRING(vals[1], comm);
+}
+
 static struct traceeval_type task_keys[] = {
 	{
 		.type = TRACEEVAL_TYPE_STRING,
@@ -144,6 +179,13 @@ static struct traceeval_type task_keys[] = {
 	},
 };
 
+static void assign_task_keys(struct traceeval_data keys[2],
+			     const char *comm, int state)
+{
+	TRACEEVAL_SET_CSTRING(keys[0], comm);
+	TRACEEVAL_SET_NUMBER(keys[1], state);
+}
+
 static struct traceeval_type task_vals[] = {
 	{
 		.type = TRACEEVAL_TYPE_POINTER,
@@ -155,6 +197,14 @@ static struct traceeval_type task_vals[] = {
 	},
 };
 
+static void assign_task_vals(struct traceeval_data vals[2],
+			     void *data, unsigned long long delta,
+			     unsigned long long timestamp)
+{
+	TRACEEVAL_SET_POINTER(vals[0], data);
+	TRACEEVAL_SET_DELTA(vals[1], delta, timestamp);
+}
+
 static struct traceeval_type thread_keys[] = {
 	{
 		.type = TRACEEVAL_TYPE_NUMBER,
@@ -166,6 +216,13 @@ static struct traceeval_type thread_keys[] = {
 	},
 };
 
+static void assign_thread_keys(struct traceeval_data keys[2],
+				int tid, int state)
+{
+	TRACEEVAL_SET_NUMBER(keys[0], tid);
+	TRACEEVAL_SET_NUMBER(keys[1], state);
+}
+
 static struct traceeval_type thread_vals[] = {
 	{
 		.type = TRACEEVAL_TYPE_DELTA,
@@ -173,6 +230,13 @@ static struct traceeval_type thread_vals[] = {
 	},
 };
 
+static void assign_thread_vals(struct traceeval_data vals[1],
+			       unsigned long long delta,
+			       unsigned long long timestamp)
+{
+	TRACEEVAL_SET_DELTA(vals[0], delta, timestamp);
+}
+
 enum sched_state {
 	RUNNING,
 	BLOCKED,
@@ -226,8 +290,7 @@ void set_process_data(struct task_data *tdata, const char *comm, void *data)
 	if (ret < 0)
 		pdie("Could not query process data");
 
-	TRACEEVAL_SET_POINTER(new_vals[0], data);
-	TRACEEVAL_SET_DELTA(new_vals[1], 0, 0);
+	assign_task_vals(new_vals, data, 0, 0);
 
 	ret = traceeval_insert(tdata->teval_tasks, keys, new_vals);
 	if (ret < 0)
@@ -278,10 +341,8 @@ static void update_cpu_data(struct task_data *tdata, int cpu, int state,
 	struct traceeval_data cpu_keys[2];
 	struct traceeval_data vals[1];
 
-	TRACEEVAL_SET_NUMBER(cpu_keys[0], cpu);
-	TRACEEVAL_SET_NUMBER(cpu_keys[1], state);
-
-	TRACEEVAL_SET_DELTA(vals[0], delta, ts);
+	assign_cpu_keys(cpu_keys, cpu, state);
+	assign_cpu_vals(vals, delta, ts);
 
 	traceeval_insert(tdata->teval_cpus, cpu_keys, vals);
 }
@@ -296,7 +357,7 @@ static void update_cpu_to_idle(struct task_data *tdata, struct tep_record *recor
 	int ret;
 
 	/* Finish previous run */
-	TRACEEVAL_SET_NUMBER(delta_keys[0], record->cpu);
+	assign_cpu_delta_keys(delta_keys, record->cpu);
 
 	ret = traceeval_delta_stop(tdata->teval_cpus, delta_keys, &results,
 				   record->ts, &delta, NULL);
@@ -308,7 +369,7 @@ static void update_cpu_to_idle(struct task_data *tdata, struct tep_record *recor
 	}
 
 	/* Start the next state */
-	TRACEEVAL_SET_NUMBER(vals[0], IDLE);
+	assign_cpu_delta_vals(vals, IDLE);
 	traceeval_delta_start(tdata->teval_cpus, delta_keys, vals,
 			      record->ts);
 }
@@ -322,9 +383,7 @@ static void update_cpu_to_running(struct task_data *tdata, struct tep_record *re
 	unsigned long long delta;
 	int ret;
 
-	/* Initialize the delta and cpu keys to point to this CPU */
-	TRACEEVAL_SET_NUMBER(delta_keys[0], record->cpu);
-	TRACEEVAL_SET_NUMBER(cpu_keys[0], record->cpu);
+	assign_cpu_delta_keys(delta_keys, record->cpu);
 
 	/* Test if the CPU was idle */
 	ret = traceeval_delta_query(tdata->teval_cpus, delta_keys, &results);
@@ -333,13 +392,13 @@ static void update_cpu_to_running(struct task_data *tdata, struct tep_record *re
 		traceeval_delta_stop(tdata->teval_cpus, delta_keys, NULL,
 				     record->ts, &delta, NULL);
 		/* Update the idle teval */
-		TRACEEVAL_SET_NUMBER(cpu_keys[1], IDLE);
-		TRACEEVAL_SET_DELTA(vals[0], delta, record->ts);
+		assign_cpu_keys(cpu_keys, record->cpu, IDLE);
+		assign_cpu_vals(vals, delta, record->ts);
 		traceeval_insert(tdata->teval_cpus, cpu_keys, vals);
 	}
 
 	/* Continue with the CPU running */
-	TRACEEVAL_SET_NUMBER(vals[0], RUNNING);
+	assign_cpu_delta_vals(vals, RUNNING);
 	traceeval_delta_continue(tdata->teval_cpus, delta_keys, vals,
 				 record->ts);
 }
@@ -355,18 +414,14 @@ static void update_thread(struct task_data *tdata, int pid, const char *comm,
 
 		pdata = get_process_data(tdata, comm);
 
-		TRACEEVAL_SET_NUMBER(keys[0], pid);
-		TRACEEVAL_SET_NUMBER(keys[1], state);
-
-		TRACEEVAL_SET_DELTA(vals[0], delta, ts);
+		assign_thread_keys(keys, pid, state);
+		assign_thread_vals(vals, delta, ts);
 
 		traceeval_insert(pdata->teval_threads, keys, vals);
 
 		/* Also update the process */
-		TRACEEVAL_SET_CSTRING(keys[0], comm);
-
-		TRACEEVAL_SET_POINTER(pvals[0], pdata);
-		TRACEEVAL_SET_DELTA(pvals[1], delta, ts);
+		assign_task_keys(keys, comm, state);
+		assign_task_vals(pvals, pdata, delta, ts);
 
 		traceeval_insert(tdata->teval_tasks, keys, pvals);
 }
@@ -382,7 +437,7 @@ static void start_running_thread(struct task_data *tdata,
 	unsigned long long val;
 	int ret;
 
-	TRACEEVAL_SET_NUMBER(delta_keys[0], pid);
+	assign_task_delta_keys(delta_keys, pid);
 
 	/* Find the previous stop state of this task */
 	ret = traceeval_delta_stop(tdata->teval_tasks, delta_keys,
@@ -396,8 +451,7 @@ static void start_running_thread(struct task_data *tdata,
 		traceeval_results_release(tdata->teval_tasks, results);
 	}
 
-	TRACEEVAL_SET_NUMBER(vals[0], RUNNING);
-	TRACEEVAL_SET_CSTRING(vals[1], comm);
+	assign_task_delta_vals(vals, RUNNING, comm);
 
 	traceeval_delta_start(tdata->teval_tasks, delta_keys, vals, record->ts);
 }
@@ -443,13 +497,12 @@ static void sched_out(struct task_data *tdata, const char *comm,
 		die("Could not read sched_switch next_pid for record");
 	state = get_stop_state(val);
 
-	TRACEEVAL_SET_NUMBER(delta_keys[0], pid);
+	assign_task_delta_keys(delta_keys, pid);
 
 	ret = traceeval_delta_stop(tdata->teval_tasks, delta_keys, &results,
 				   record->ts, &delta, &val);
 
-	TRACEEVAL_SET_NUMBER(task_vals[0], state);
-	TRACEEVAL_SET_CSTRING(task_vals[1], comm);
+	assign_task_delta_vals(task_vals, state, comm);
 
 	if (ret > 0)
 		old_state = results[0].number;
@@ -463,23 +516,20 @@ static void sched_out(struct task_data *tdata, const char *comm,
 		die("Not running %d from %lld to %lld",
 		    old_state, val, record->ts);
 
-	TRACEEVAL_SET_CSTRING(task_keys[0], comm);
-	TRACEEVAL_SET_NUMBER(task_keys[1], RUNNING);
+	assign_task_keys(task_keys, comm, RUNNING);
 
 	pdata = get_process_data(tdata, comm);
 
-	TRACEEVAL_SET_POINTER(task_vals[0], pdata);
-	TRACEEVAL_SET_DELTA(task_vals[1], delta, record->ts);
+	assign_task_vals(task_vals, pdata, delta, record->ts);
 
 	traceeval_insert(tdata->teval_tasks, task_keys, task_vals);
 
-	TRACEEVAL_SET_NUMBER(task_keys[0], pid);
-
-	TRACEEVAL_SET_DELTA(vals[0], delta, record->ts);
+	assign_thread_keys(task_keys, pid, RUNNING);
+	assign_thread_vals(vals, delta, record->ts);
 
 	traceeval_insert(pdata->teval_threads, task_keys, vals);
 
-	TRACEEVAL_SET_NUMBER(task_keys[0], record->cpu);
+	assign_cpu_keys(task_keys, record->cpu, RUNNING);
 
 	traceeval_insert(pdata->teval_cpus, task_keys, vals);
 }
-- 
2.42.0


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

* [PATCH 03/11] libtraceeval task-eval: Add tracking of priority
  2023-10-11  3:25 [PATCH 00/11] libtraceeval task-eval: Updates to evaluate tasks Steven Rostedt
  2023-10-11  3:25 ` [PATCH 01/11] libtraceeval task-eval: Fix naming conventions of variables Steven Rostedt
  2023-10-11  3:25 ` [PATCH 02/11] libtraceeval task-eval: Use "assign_*keys/vals()" helper functions Steven Rostedt
@ 2023-10-11  3:25 ` Steven Rostedt
  2023-10-11  3:25 ` [PATCH 04/11] libtraceeval task-eval: Add way to read trace.dat buffer instances Steven Rostedt
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2023-10-11  3:25 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Ross Zwisler, Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Have the sample code of task-eval also keep track of the priority of a
task. If the priority is less than 120 (not SCHED_OTHER), then split it up
in the output.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 samples/task-eval.c | 105 ++++++++++++++++++++++++++++++++------------
 1 file changed, 76 insertions(+), 29 deletions(-)

diff --git a/samples/task-eval.c b/samples/task-eval.c
index 11ea73ce8da0..ad3e2424acc8 100644
--- a/samples/task-eval.c
+++ b/samples/task-eval.c
@@ -159,13 +159,18 @@ static struct traceeval_type task_delta_vals[] = {
 		.type = TRACEEVAL_TYPE_STRING,
 		.name = "COMM",
 	},
+	{
+		.type = TRACEEVAL_TYPE_NUMBER,
+		.name = "Prio",
+	},
 };
 
-static void assign_task_delta_vals(struct traceeval_data vals[2],
-				   int state, const char *comm)
+static void assign_task_delta_vals(struct traceeval_data vals[3],
+				   int state, const char *comm, int prio)
 {
 	TRACEEVAL_SET_NUMBER(vals[0], state);
 	TRACEEVAL_SET_CSTRING(vals[1], comm);
+	TRACEEVAL_SET_NUMBER(vals[2], prio);
 }
 
 static struct traceeval_type task_keys[] = {
@@ -214,13 +219,18 @@ static struct traceeval_type thread_keys[] = {
 		.type = TRACEEVAL_TYPE_NUMBER,
 		.name = "Schedule state",
 	},
+	{
+		.type = TRACEEVAL_TYPE_NUMBER,
+		.name = "Prio",
+	},
 };
 
-static void assign_thread_keys(struct traceeval_data keys[2],
-				int tid, int state)
+static void assign_thread_keys(struct traceeval_data keys[3],
+				int tid, int state, int prio)
 {
 	TRACEEVAL_SET_NUMBER(keys[0], tid);
 	TRACEEVAL_SET_NUMBER(keys[1], state);
+	TRACEEVAL_SET_NUMBER(keys[2], prio);
 }
 
 static struct traceeval_type thread_vals[] = {
@@ -404,35 +414,36 @@ static void update_cpu_to_running(struct task_data *tdata, struct tep_record *re
 }
 
 static void update_thread(struct task_data *tdata, int pid, const char *comm,
-			  enum sched_state state, unsigned long long delta,
+			  enum sched_state state, int prio, unsigned long long delta,
 			  unsigned long long ts)
 {
-		struct traceeval_data keys[2];
+		struct traceeval_data task_keys[2];
+		struct traceeval_data thread_keys[3];
 		struct traceeval_data pvals[2];
 		struct traceeval_data vals[1];
 		struct process_data *pdata;
 
 		pdata = get_process_data(tdata, comm);
 
-		assign_thread_keys(keys, pid, state);
+		assign_thread_keys(thread_keys, pid, state, prio);
 		assign_thread_vals(vals, delta, ts);
 
-		traceeval_insert(pdata->teval_threads, keys, vals);
+		traceeval_insert(pdata->teval_threads, thread_keys, vals);
 
 		/* Also update the process */
-		assign_task_keys(keys, comm, state);
+		assign_task_keys(task_keys, comm, state);
 		assign_task_vals(pvals, pdata, delta, ts);
 
-		traceeval_insert(tdata->teval_tasks, keys, pvals);
+		traceeval_insert(tdata->teval_tasks, task_keys, pvals);
 }
 
 static void start_running_thread(struct task_data *tdata,
 				 struct tep_record *record,
-				 const char *comm, int pid)
+				 const char *comm, int pid, int prio)
 {
 	const struct traceeval_data *results;
 	struct traceeval_data delta_keys[1];
-	struct traceeval_data vals[2];
+	struct traceeval_data vals[3];
 	unsigned long long delta;
 	unsigned long long val;
 	int ret;
@@ -447,11 +458,11 @@ static void start_running_thread(struct task_data *tdata,
 
 		if (state == RUNNING)
 			die("State %d is running! %lld -> %lld", pid, val, record->ts);
-		update_thread(tdata, pid, comm, state, delta, record->ts);
+		update_thread(tdata, pid, comm, state, prio, delta, record->ts);
 		traceeval_results_release(tdata->teval_tasks, results);
 	}
 
-	assign_task_delta_vals(vals, RUNNING, comm);
+	assign_task_delta_vals(vals, RUNNING, comm, prio);
 
 	traceeval_delta_start(tdata->teval_tasks, delta_keys, vals, record->ts);
 }
@@ -468,11 +479,14 @@ static int get_stop_state(unsigned long long val)
 static void sched_out(struct task_data *tdata, const char *comm,
 		      struct tep_event *event,
 		      struct tep_record *record, struct tep_format_field *prev_pid,
-		      struct tep_format_field *prev_state)
+		      struct tep_format_field *prev_state,
+		      struct tep_format_field *prev_prio)
 {
 	struct traceeval_data delta_keys[1];
 	struct traceeval_data task_keys[2];
+	struct traceeval_data task_delta_vals[3];
 	struct traceeval_data task_vals[2];
+	struct traceeval_data thread_keys[3];
 	struct traceeval_data vals[1];
 	struct process_data *pdata;
 	const struct traceeval_data *results;
@@ -480,6 +494,7 @@ static void sched_out(struct task_data *tdata, const char *comm,
 	unsigned long long val;
 	int state;
 	int old_state;
+	int prio;
 	int pid;
 	int ret;
 
@@ -492,6 +507,11 @@ static void sched_out(struct task_data *tdata, const char *comm,
 	if (!pid)
 		return;
 
+	ret = tep_read_number_field(prev_prio, record->data, &val);
+	if (ret < 0)
+		die("Could not read sched_switch prev_prio for record");
+	prio = val;
+
 	ret = tep_read_number_field(prev_state, record->data, &val);
 	if (ret < 0)
 		die("Could not read sched_switch next_pid for record");
@@ -502,13 +522,13 @@ static void sched_out(struct task_data *tdata, const char *comm,
 	ret = traceeval_delta_stop(tdata->teval_tasks, delta_keys, &results,
 				   record->ts, &delta, &val);
 
-	assign_task_delta_vals(task_vals, state, comm);
+	assign_task_delta_vals(task_delta_vals, state, comm, prio);
 
 	if (ret > 0)
 		old_state = results[0].number;
 
 	/* Start recording why this task is off the CPU */
-	traceeval_delta_start(tdata->teval_tasks, delta_keys, task_vals, record->ts);
+	traceeval_delta_start(tdata->teval_tasks, delta_keys, task_delta_vals, record->ts);
 	if (ret <= 0)
 		return;
 
@@ -524,10 +544,10 @@ static void sched_out(struct task_data *tdata, const char *comm,
 
 	traceeval_insert(tdata->teval_tasks, task_keys, task_vals);
 
-	assign_thread_keys(task_keys, pid, RUNNING);
+	assign_thread_keys(thread_keys, pid, RUNNING, prio);
 	assign_thread_vals(vals, delta, record->ts);
 
-	traceeval_insert(pdata->teval_threads, task_keys, vals);
+	traceeval_insert(pdata->teval_threads, thread_keys, vals);
 
 	assign_cpu_keys(task_keys, record->cpu, RUNNING);
 
@@ -536,11 +556,14 @@ static void sched_out(struct task_data *tdata, const char *comm,
 
 static void sched_in(struct task_data *tdata, const char *comm,
 		     struct tep_event *event,
-		     struct tep_record *record, struct tep_format_field *next_pid)
+		     struct tep_record *record,
+		     struct tep_format_field *next_pid,
+		     struct tep_format_field *next_prio)
 {
 	unsigned long long val;
-	int ret;
+	int prio;
 	int pid;
+	int ret;
 
 	ret = tep_read_number_field(next_pid, record->data, &val);
 	if (ret < 0)
@@ -553,9 +576,14 @@ static void sched_in(struct task_data *tdata, const char *comm,
 		return;
 	}
 
+	ret = tep_read_number_field(next_prio, record->data, &val);
+	if (ret < 0)
+		die("Could not read sched_switch next_prio for record");
+	prio = val;
+
 	/* Continue measuring CPU running time */
 	update_cpu_to_running(tdata, record);
-	start_running_thread(tdata, record, comm, pid);
+	start_running_thread(tdata, record, comm, pid, prio);
 }
 
 static struct tep_format_field *get_field(struct tep_event *event, const char *name)
@@ -576,8 +604,10 @@ static int switch_func(struct tracecmd_input *handle, struct tep_event *event,
 	static struct tep_format_field *prev_comm;
 	static struct tep_format_field *prev_pid;
 	static struct tep_format_field *prev_state;
+	static struct tep_format_field *prev_prio;
 	static struct tep_format_field *next_comm;
 	static struct tep_format_field *next_pid;
+	static struct tep_format_field *next_prio;
 	struct task_data *tdata = data;
 	const char *comm;
 
@@ -585,18 +615,20 @@ static int switch_func(struct tracecmd_input *handle, struct tep_event *event,
 		prev_comm = get_field(event, "prev_comm");
 		prev_pid = get_field(event, "prev_pid");
 		prev_state = get_field(event, "prev_state");
+		prev_prio = get_field(event, "prev_prio");
 
 		next_comm = get_field(event, "next_comm");
 		next_pid = get_field(event, "next_pid");
+		next_prio = get_field(event, "next_prio");
 	}
 
 	comm = record->data + prev_comm->offset;
 	if (!tdata->comm || strcmp(comm, tdata->comm) == 0)
-		sched_out(tdata, comm, event, record, prev_pid, prev_state);
+		sched_out(tdata, comm, event, record, prev_pid, prev_state, prev_prio);
 
 	comm = record->data + next_comm->offset;
 	if (!tdata->comm || strcmp(comm, tdata->comm) == 0)
-		sched_in(tdata, comm, event, record, next_pid);
+		sched_in(tdata, comm, event, record, next_pid, next_prio);
 
 	return 0;
 }
@@ -740,22 +772,35 @@ static void display_threads(struct traceeval *teval)
 	const struct traceeval_data *keys;
 	struct traceeval_stat *stat;
 	int last_tid = -1;
+	int last_prio = -1;
 
+	/* PID */
 	traceeval_iterator_sort(iter, thread_keys[0].name, 0, true);
-	traceeval_iterator_sort(iter, thread_keys[1].name, 1, true);
+
+	/* PRIO */
+	traceeval_iterator_sort(iter, thread_keys[2].name, 1, true);
+
+	/* STATE */
+	traceeval_iterator_sort(iter, thread_keys[1].name, 2, true);
 
 	while (traceeval_iterator_next(iter, &keys) > 0) {
-		int state = keys[1].number;
 		int tid = keys[0].number;
+		int state = keys[1].number;
+		int prio = keys[2].number;
 
 		stat = traceeval_iterator_stat(iter, DELTA_NAME);
 		if (!stat)
 			continue; // die?
 
-		if (last_tid != keys[0].number)
-			printf("\n    thread id: %d\n", tid);
+		if (last_tid != tid || last_prio != prio) {
+			if (prio < 120)
+				printf("\n    thread id: %d [ prio: %d ]\n", tid, prio);
+			else
+				printf("\n    thread id: %d\n", tid);
+		}
 
 		last_tid = tid;
+		last_prio = prio;
 
 		display_state_times(state, traceeval_stat_total(stat));
 	}
@@ -888,6 +933,7 @@ static void finish_leftovers(struct task_data *data)
 	unsigned long long delta;
 	enum sched_state state;
 	const char *comm;
+	int prio;
 	int pid;
 
 	iter = traceeval_iterator_delta_start_get(data->teval_tasks);
@@ -899,8 +945,9 @@ static void finish_leftovers(struct task_data *data)
 
 		state = results[0].number;
 		comm = results[1].cstring;
+		prio = results[2].number;
 
-		update_thread(data, pid, comm, state, delta, data->last_ts);
+		update_thread(data, pid, comm, state, prio, delta, data->last_ts);
 	}
 	traceeval_iterator_put(iter);
 
-- 
2.42.0


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

* [PATCH 04/11] libtraceeval task-eval: Add way to read trace.dat buffer instances
  2023-10-11  3:25 [PATCH 00/11] libtraceeval task-eval: Updates to evaluate tasks Steven Rostedt
                   ` (2 preceding siblings ...)
  2023-10-11  3:25 ` [PATCH 03/11] libtraceeval task-eval: Add tracking of priority Steven Rostedt
@ 2023-10-11  3:25 ` Steven Rostedt
  2023-10-11  3:25 ` [PATCH 05/11] libtraceeval task-eval: Account for ZOMBIE and EXITED states Steven Rostedt
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2023-10-11  3:25 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Ross Zwisler, Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

If a trace.dat file has data in a buffer instance, then task-eval should
be able to read that too. Add a "-B foo" option to task-eval that lets the
user specify a buffer instance that is within a trace.dat file.

This is used for when the trace.dat file is recorded as:

  trace-cmd record -B foo ...

The "foo" buffer is stored separately in the trace.dat file and requires
to be retrieved to be seen by libtracecmd.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 samples/task-eval.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/samples/task-eval.c b/samples/task-eval.c
index ad3e2424acc8..40e7345a27c4 100644
--- a/samples/task-eval.c
+++ b/samples/task-eval.c
@@ -41,6 +41,7 @@ static void usage(void)
 	       "  Run this on the resulting trace.dat file\n"
 	       "\n"
 	       "-c comm - to look at only a specific process called 'comm'\n"
+	       "-B instance - read a buffer instance in the trace.dat file\n"
 	       "\n",p);
 	exit(-1);
 }
@@ -975,17 +976,21 @@ int main (int argc, char **argv)
 {
 	struct tracecmd_input *handle;
 	struct task_data data;
+	const char *buffer = NULL;
 	int c;
 
 	memset(&data, 0, sizeof(data));
 
 	argv0 = argv[0];
 
-	while ((c = getopt(argc, argv, "c:h")) >= 0) {
+	while ((c = getopt(argc, argv, "c:B:h")) >= 0) {
 		switch (c) {
 		case 'c':
 			data.comm = optarg;
 			break;
+		case 'B':
+			buffer = optarg;
+			break;
 		case 'h':
 		default:
 			usage();
@@ -1002,6 +1007,24 @@ int main (int argc, char **argv)
 	if (!handle)
 		pdie("Error opening %s", argv[0]);
 
+	if (buffer) {
+		int bufs;
+		int i;
+
+		bufs = tracecmd_buffer_instances(handle);
+		for (i = 0; i < bufs; i++) {
+			const char *name;
+
+			name = tracecmd_buffer_instance_name(handle, i);
+			if (name && strcmp(name, buffer) == 0) {
+				handle = tracecmd_buffer_instance_handle(handle, i);
+				break;
+			}
+		}
+		if (i == bufs)
+			die("Can not find instance %s\n", buffer);
+	}
+
 	data.teval_tasks = traceeval_init(task_keys, task_vals);
 	if (!data.teval_tasks)
 		pdie("Creating trace eval processe data");
-- 
2.42.0


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

* [PATCH 05/11] libtraceeval task-eval: Account for ZOMBIE and EXITED states
  2023-10-11  3:25 [PATCH 00/11] libtraceeval task-eval: Updates to evaluate tasks Steven Rostedt
                   ` (3 preceding siblings ...)
  2023-10-11  3:25 ` [PATCH 04/11] libtraceeval task-eval: Add way to read trace.dat buffer instances Steven Rostedt
@ 2023-10-11  3:25 ` Steven Rostedt
  2023-10-11  3:25 ` [PATCH 06/11] libtraceeval task-eval: Show max, min and count of deltas too Steven Rostedt
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2023-10-11  3:25 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Ross Zwisler, Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Currently, if a sched_switch happens with a task in he ZOMBIE or EXITED
state, task-eval will treat it as being preempted in a running state. This
is incorrect, and should be updated to reflect it.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 samples/task-eval.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/samples/task-eval.c b/samples/task-eval.c
index 40e7345a27c4..061d27374e66 100644
--- a/samples/task-eval.c
+++ b/samples/task-eval.c
@@ -254,6 +254,8 @@ enum sched_state {
 	PREEMPT,
 	SLEEP,
 	IDLE,
+	ZOMBIE,
+	EXITED,
 	OTHER
 };
 
@@ -474,6 +476,10 @@ static int get_stop_state(unsigned long long val)
 		return SLEEP;
 	if (val & 2)
 		return BLOCKED;
+	if (val & 0x10)
+		return ZOMBIE;
+	if (val & 0x20)
+		return EXITED;
 	return PREEMPT;
 }
 
-- 
2.42.0


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

* [PATCH 06/11] libtraceeval task-eval: Show max, min and count of deltas too
  2023-10-11  3:25 [PATCH 00/11] libtraceeval task-eval: Updates to evaluate tasks Steven Rostedt
                   ` (4 preceding siblings ...)
  2023-10-11  3:25 ` [PATCH 05/11] libtraceeval task-eval: Account for ZOMBIE and EXITED states Steven Rostedt
@ 2023-10-11  3:25 ` Steven Rostedt
  2023-10-11  3:25 ` [PATCH 07/11] libtraceeval task-eval: Add more comments Steven Rostedt
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2023-10-11  3:25 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Ross Zwisler, Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Instead of just showing the total numbers of the delta output, also show
the maximum, minimum and count, as well as the timestamps of where the max
and min occurred.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 samples/task-eval.c | 64 +++++++++++++++++++++++++++++++++------------
 1 file changed, 48 insertions(+), 16 deletions(-)

diff --git a/samples/task-eval.c b/samples/task-eval.c
index 061d27374e66..4a500227cf11 100644
--- a/samples/task-eval.c
+++ b/samples/task-eval.c
@@ -646,9 +646,15 @@ static void print_microseconds(int idx, unsigned long long nsecs)
 
 	usecs = nsecs / 1000;
 	if (!nsecs || usecs)
-		printf("%*lld\n", idx, usecs);
+		printf("%*lld", idx, usecs);
 	else
-		printf("%*d.%03lld\n", idx, 0, nsecs);
+		printf("%*d.%03lld", idx, 0, nsecs);
+}
+
+static void print_microseconds_nl(int idx, unsigned long long nsecs)
+{
+	print_microseconds(idx, nsecs);
+	printf("\n");
 }
 
 /*
@@ -743,7 +749,7 @@ static void display_cpus(struct traceeval *teval)
 			break;
 		}
 		printf(" time (us):");
-		print_microseconds(12, traceeval_stat_total(stat));
+		print_microseconds_nl(12, traceeval_stat_total(stat));
 
 		last_cpu = cpu;
 	}
@@ -752,24 +758,54 @@ static void display_cpus(struct traceeval *teval)
 		die("No result for CPUs\n");
 }
 
-static void display_state_times(int state, unsigned long long total)
+static void print_stats(int idx, struct traceeval_stat *stat)
+{
+	unsigned long long total, max, min, cnt, max_ts, min_ts;
+
+	if (stat) {
+		total = traceeval_stat_total(stat);
+		max = traceeval_stat_max_timestamp(stat, &max_ts);
+		min = traceeval_stat_min_timestamp(stat, &min_ts);
+		cnt = traceeval_stat_count(stat);
+	} else {
+		total = max = max_ts = min = min_ts = cnt = 0;
+	}
+
+	if (!cnt) {
+		print_microseconds_nl(idx, total);
+	} else if (cnt == 1) {
+		print_microseconds(idx, total);
+		printf("\tat: %lld\n", max_ts);
+	} else {
+		print_microseconds_nl(idx, total);
+		printf("%*s%*lld\n", 40 - idx, "count:", idx, cnt);
+		printf("%*s", 40 - idx, "max:");
+		print_microseconds(idx, max);
+		printf("\tat: %lld\n", max_ts);
+		printf("%*s", 40 - idx, "min:");
+		print_microseconds(idx, min);
+		printf("\tat: %lld\n", min_ts);
+	}
+}
+
+static void display_state_times(int state, struct traceeval_stat *stat)
 {
 	switch (state) {
 	case RUNNING:
 		printf("      Total run time (us):");
-		print_microseconds(14, total);
+		print_stats(14, stat);
 		break;
 	case BLOCKED:
 		printf("      Total blocked time (us):");
-		print_microseconds(10, total);
+		print_stats(10, stat);
 		break;
 	case PREEMPT:
 		printf("      Total preempt time (us):");
-		print_microseconds(10, total);
+		print_stats(10, stat);
 		break;
 	case SLEEP:
 		printf("      Total sleep time (us):");
-		print_microseconds(12, total);
+		print_stats(12, stat);
 	}
 }
 
@@ -809,7 +845,7 @@ static void display_threads(struct traceeval *teval)
 		last_tid = tid;
 		last_prio = prio;
 
-		display_state_times(state, traceeval_stat_total(stat));
+		display_state_times(state, stat);
 	}
 
 	if (last_tid < 0)
@@ -827,7 +863,6 @@ static void display_process_stats(struct traceeval *teval,
 				  struct process_data *pdata, const char *comm)
 {
 	struct traceeval_stat *stat;
-	unsigned long long delta;
 	struct traceeval_data keys[] = {
 		DEFINE_TRACEEVAL_CSTRING(	comm		),
 		DEFINE_TRACEEVAL_NUMBER(	RUNNING		),
@@ -836,11 +871,8 @@ static void display_process_stats(struct traceeval *teval,
 	for (int i = 0; i < OTHER; i++) {
 		TRACEEVAL_SET_NUMBER(keys[1], i);
 
-		delta = 0;
 		stat = traceeval_stat(teval, keys, DELTA_NAME);
-		if (stat)
-			delta = traceeval_stat_total(stat);
-		display_state_times(i, delta);
+		display_state_times(i, stat);
 	}
 }
 
@@ -918,9 +950,9 @@ static void display(struct task_data *tdata)
 	}
 
 	printf("  Total  run time (us):");
-	print_microseconds(16, total_time);
+	print_microseconds_nl(16, total_time);
 	printf("  Total idle time (us):");
-	print_microseconds(16, idle_time);
+	print_microseconds_nl(16, idle_time);
 
 	display_cpus(tdata->teval_cpus);
 
-- 
2.42.0


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

* [PATCH 07/11] libtraceeval task-eval: Add more comments
  2023-10-11  3:25 [PATCH 00/11] libtraceeval task-eval: Updates to evaluate tasks Steven Rostedt
                   ` (5 preceding siblings ...)
  2023-10-11  3:25 ` [PATCH 06/11] libtraceeval task-eval: Show max, min and count of deltas too Steven Rostedt
@ 2023-10-11  3:25 ` Steven Rostedt
  2023-10-11  3:25 ` [PATCH 08/11] libtraceeval task-eval: Only release results if found Steven Rostedt
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2023-10-11  3:25 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Ross Zwisler, Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Add more comments explaining what is going on in task-eval.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 samples/task-eval.c | 74 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/samples/task-eval.c b/samples/task-eval.c
index 4a500227cf11..aa61ec38244f 100644
--- a/samples/task-eval.c
+++ b/samples/task-eval.c
@@ -84,6 +84,11 @@ void pdie(const char *fmt, ...)
 /* Used for stats */
 #define DELTA_NAME		"delta"
 
+/*
+ * Keep track of when a CPU is running tasks and when it is
+ * idle. Use the CPU number to match the timings in the
+ * sched_switch event.
+ */
 static struct traceeval_type cpu_delta_keys[] = {
 	{
 		.type = TRACEEVAL_TYPE_NUMBER,
@@ -96,6 +101,12 @@ static void assign_cpu_delta_keys(struct traceeval_data keys[1], int cpu)
 	TRACEEVAL_SET_NUMBER(keys[0], cpu);
 }
 
+/*
+ * When scheduling, record the state the CPU was in.
+ * It only cares about IDLE vs RUNNING. If the idle task is being
+ * scheduled in, mark it the staet as IDLE, otherwise mark it
+ * as RUNNING.
+ */
 static struct traceeval_type cpu_delta_vals[] = {
 	{
 		.type = TRACEEVAL_TYPE_NUMBER,
@@ -108,6 +119,9 @@ static void assign_cpu_delta_vals(struct traceeval_data vals[1], int state)
 	TRACEEVAL_SET_NUMBER(vals[0], state);
 }
 
+/*
+ * The output will show all the CPUs and their IDLE vs RUNNING states.
+ */
 static struct traceeval_type cpu_keys[] = {
 	{
 		.type = TRACEEVAL_TYPE_NUMBER,
@@ -125,6 +139,10 @@ static void assign_cpu_keys(struct traceeval_data keys[2], int cpu, int state)
 	TRACEEVAL_SET_NUMBER(keys[1], state);
 }
 
+/*
+ * The mapping of CPU and state will track the timings of how long the
+ * CPU was in that state.
+ */
 static struct traceeval_type cpu_vals[] = {
 	{
 		.type = TRACEEVAL_TYPE_DELTA,
@@ -139,6 +157,10 @@ static void assign_cpu_vals(struct traceeval_data vals[1],
 	TRACEEVAL_SET_DELTA(vals[0], delta, timestamp);
 }
 
+/*
+ * When tracking tasks and threads, remember the task id (PID)
+ * when scheduling out (for sleep state) or in (for running state).
+ */
 static struct traceeval_type task_delta_keys[] = {
 	{
 		.type = TRACEEVAL_TYPE_NUMBER,
@@ -151,6 +173,11 @@ static void assign_task_delta_keys(struct traceeval_data keys[1], int pid)
 	TRACEEVAL_SET_NUMBER(keys[0], pid);
 }
 
+/*
+ * When finishing the timings, will need the name of the task, the
+ * state it was in:  (RUNNING, PREEMPTED, BLOCKED, IDLE, or other)
+ * and the priority it had. This will be saved for the output.
+ */
 static struct traceeval_type task_delta_vals[] = {
 	{
 		.type = TRACEEVAL_TYPE_NUMBER,
@@ -174,6 +201,12 @@ static void assign_task_delta_vals(struct traceeval_data vals[3],
 	TRACEEVAL_SET_NUMBER(vals[2], prio);
 }
 
+/*
+ * Will output all the processes by their names. This means the two
+ * tasks with the same name will be grouped together (even though they
+ * may not be threads). The tasks will also be broken up by what state
+ * they were in: RUNNING, BLOCKED, PREEMPTED, SLEEPING.
+ */
 static struct traceeval_type task_keys[] = {
 	{
 		.type = TRACEEVAL_TYPE_STRING,
@@ -192,6 +225,14 @@ static void assign_task_keys(struct traceeval_data keys[2],
 	TRACEEVAL_SET_NUMBER(keys[1], state);
 }
 
+/*
+ * For each state the process is in, record the time delta for
+ * that state. Also, only for the RUNNING state, this will
+ * daisy chain another traceeval for each thread. That is,
+ * for each unique thread id (PID), there will be a traceeval
+ * histogram of those threads denoted by the teval_thread, and
+ * that will be saved in the "data" field.
+ */
 static struct traceeval_type task_vals[] = {
 	{
 		.type = TRACEEVAL_TYPE_POINTER,
@@ -211,6 +252,12 @@ static void assign_task_vals(struct traceeval_data vals[2],
 	TRACEEVAL_SET_DELTA(vals[1], delta, timestamp);
 }
 
+/*
+ * Each recorded process will have a traceeval to save all the
+ * threads within it. The threads will be mapped by their TID (PID)
+ * the state they were in: RUNNING, BLOCKED, PREEMPTED, SLEEPING
+ * and their priority.
+ */
 static struct traceeval_type thread_keys[] = {
 	{
 		.type = TRACEEVAL_TYPE_NUMBER,
@@ -234,6 +281,9 @@ static void assign_thread_keys(struct traceeval_data keys[3],
 	TRACEEVAL_SET_NUMBER(keys[2], prio);
 }
 
+/*
+ * Save the timings of the thread/state/prio keys.
+ */
 static struct traceeval_type thread_vals[] = {
 	{
 		.type = TRACEEVAL_TYPE_DELTA,
@@ -326,6 +376,12 @@ static struct process_data *alloc_pdata(struct task_data *tdata, const char *com
 	return pdata;
 }
 
+/*
+ * Each process will have a traceeval for all their threads. The
+ * thread traceeval descriptor will be saved in the process/RUNNING
+ * field. If a process is never running, then it will not have any
+ * threads!
+ */
 static struct process_data *
 get_process_data(struct task_data *tdata, const char *comm)
 {
@@ -360,6 +416,10 @@ static void update_cpu_data(struct task_data *tdata, int cpu, int state,
 	traceeval_insert(tdata->teval_cpus, cpu_keys, vals);
 }
 
+/*
+ * When a CPU is scheduling in idle, record the running state,
+ * and start the idle timings.
+ */
 static void update_cpu_to_idle(struct task_data *tdata, struct tep_record *record)
 {
 
@@ -387,6 +447,10 @@ static void update_cpu_to_idle(struct task_data *tdata, struct tep_record *recor
 			      record->ts);
 }
 
+/*
+ * When a CPU is scheduling a task, if idle is scheduling out, stop
+ * the idle timings and start or continue the running timings.
+ */
 static void update_cpu_to_running(struct task_data *tdata, struct tep_record *record)
 {
 	struct traceeval_data delta_keys[1];
@@ -465,6 +529,7 @@ static void start_running_thread(struct task_data *tdata,
 		traceeval_results_release(tdata->teval_tasks, results);
 	}
 
+	/* This task is running, so start timing the running portion */
 	assign_task_delta_vals(vals, RUNNING, comm, prio);
 
 	traceeval_delta_start(tdata->teval_tasks, delta_keys, vals, record->ts);
@@ -524,6 +589,7 @@ static void sched_out(struct task_data *tdata, const char *comm,
 		die("Could not read sched_switch next_pid for record");
 	state = get_stop_state(val);
 
+	/* Stop the RUNNING timings of the tasks that is scheduling out */
 	assign_task_delta_keys(delta_keys, pid);
 
 	ret = traceeval_delta_stop(tdata->teval_tasks, delta_keys, &results,
@@ -539,10 +605,12 @@ static void sched_out(struct task_data *tdata, const char *comm,
 	if (ret <= 0)
 		return;
 
+	/* What is scheduling out should be considered "running" */
 	if (old_state != RUNNING)
 		die("Not running %d from %lld to %lld",
 		    old_state, val, record->ts);
 
+	/* Now start the "running" timings of the task scheduling in */
 	assign_task_keys(task_keys, comm, RUNNING);
 
 	pdata = get_process_data(tdata, comm);
@@ -551,6 +619,7 @@ static void sched_out(struct task_data *tdata, const char *comm,
 
 	traceeval_insert(tdata->teval_tasks, task_keys, task_vals);
 
+	/* Update the individual thread as well */
 	assign_thread_keys(thread_keys, pid, RUNNING, prio);
 	assign_thread_vals(vals, delta, record->ts);
 
@@ -964,6 +1033,11 @@ static void free_tdata(struct task_data *tdata)
 {
 }
 
+/*
+ * When the trace ended, there was likely tasks still running on
+ * the CPU (or sleeping). Just stop all the timings and put them
+ * into the database as well.
+ */
 static void finish_leftovers(struct task_data *data)
 {
 	const struct traceeval_data *results;
-- 
2.42.0


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

* [PATCH 08/11] libtraceeval task-eval: Only release results if found
  2023-10-11  3:25 [PATCH 00/11] libtraceeval task-eval: Updates to evaluate tasks Steven Rostedt
                   ` (6 preceding siblings ...)
  2023-10-11  3:25 ` [PATCH 07/11] libtraceeval task-eval: Add more comments Steven Rostedt
@ 2023-10-11  3:25 ` Steven Rostedt
  2023-10-11  3:25 ` [PATCH 09/11] libtraceeval task-eval: Do not add pdata to non RUNNING tasks Steven Rostedt
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2023-10-11  3:25 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Ross Zwisler, Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

In set_process_data(), the results are release even if they were not
found. But when they are not found, results is undefined.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 samples/task-eval.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/samples/task-eval.c b/samples/task-eval.c
index aa61ec38244f..79039580ce78 100644
--- a/samples/task-eval.c
+++ b/samples/task-eval.c
@@ -348,8 +348,10 @@ void set_process_data(struct task_data *tdata, const char *comm, void *data)
 	int ret;
 
 	ret = traceeval_query(tdata->teval_tasks, keys, &results);
-	if (ret > 0)
-		goto out; /* It already exists ? */
+	if (ret > 0) {
+		traceeval_results_release(tdata->teval_tasks, results);
+		return; /* It already exists ? */
+	}
 	if (ret < 0)
 		pdie("Could not query process data");
 
@@ -358,9 +360,6 @@ void set_process_data(struct task_data *tdata, const char *comm, void *data)
 	ret = traceeval_insert(tdata->teval_tasks, keys, new_vals);
 	if (ret < 0)
 		pdie("Failed to set process data");
-
- out:
-	traceeval_results_release(tdata->teval_tasks, results);
 }
 
 static struct process_data *alloc_pdata(struct task_data *tdata, const char *comm)
-- 
2.42.0


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

* [PATCH 09/11] libtraceeval task-eval: Do not add pdata to non RUNNING tasks
  2023-10-11  3:25 [PATCH 00/11] libtraceeval task-eval: Updates to evaluate tasks Steven Rostedt
                   ` (7 preceding siblings ...)
  2023-10-11  3:25 ` [PATCH 08/11] libtraceeval task-eval: Only release results if found Steven Rostedt
@ 2023-10-11  3:25 ` Steven Rostedt
  2023-10-11  3:25 ` [PATCH 10/11] libtraceveal task-eval: Clean up all traceeval_iterators Steven Rostedt
  2023-10-11  3:25 ` [PATCH 11/11] libtraceeval task-evals: Free the libtraceevals Steven Rostedt
  10 siblings, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2023-10-11  3:25 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Ross Zwisler, Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The pdata is used to daisy chain all the threads for each task with the
same name. But since the tasks saving is broken up for each of their
scheduling states, the pdata is saved in just the RUNNING state. But since
it also needs to be freed, each pdata only needs to be saved once in the
traceeval for tasks.

Make sure that when updating the tasks, the pdata for the task is only
saved with the RUNNING state, and all other states just have NULL.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 samples/task-eval.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/samples/task-eval.c b/samples/task-eval.c
index 79039580ce78..420d87f498cc 100644
--- a/samples/task-eval.c
+++ b/samples/task-eval.c
@@ -496,6 +496,10 @@ static void update_thread(struct task_data *tdata, int pid, const char *comm,
 
 		traceeval_insert(pdata->teval_threads, thread_keys, vals);
 
+		/* Only the RUNNING state keeps pdata */
+		if (state != RUNNING)
+			pdata = NULL;
+
 		/* Also update the process */
 		assign_task_keys(task_keys, comm, state);
 		assign_task_vals(pvals, pdata, delta, ts);
-- 
2.42.0


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

* [PATCH 10/11] libtraceveal task-eval: Clean up all traceeval_iterators
  2023-10-11  3:25 [PATCH 00/11] libtraceeval task-eval: Updates to evaluate tasks Steven Rostedt
                   ` (8 preceding siblings ...)
  2023-10-11  3:25 ` [PATCH 09/11] libtraceeval task-eval: Do not add pdata to non RUNNING tasks Steven Rostedt
@ 2023-10-11  3:25 ` Steven Rostedt
  2023-10-11  3:25 ` [PATCH 11/11] libtraceeval task-evals: Free the libtraceevals Steven Rostedt
  10 siblings, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2023-10-11  3:25 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Ross Zwisler, Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

When traceeval_iterator_get() is used, it must be accompanied with a
traceeval_iterator_put().

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 samples/task-eval.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/samples/task-eval.c b/samples/task-eval.c
index 420d87f498cc..25032293c9a3 100644
--- a/samples/task-eval.c
+++ b/samples/task-eval.c
@@ -828,6 +828,8 @@ static void display_cpus(struct traceeval *teval)
 
 	if (last_cpu < 0)
 		die("No result for CPUs\n");
+
+	traceeval_iterator_put(iter);
 }
 
 static void print_stats(int idx, struct traceeval_stat *stat)
@@ -920,6 +922,8 @@ static void display_threads(struct traceeval *teval)
 		display_state_times(state, stat);
 	}
 
+	traceeval_iterator_put(iter);
+
 	if (last_tid < 0)
 		die("No result for threads\n");
 }
@@ -982,12 +986,13 @@ static void display_processes(struct traceeval *teval)
 		if (pdata)
 			display_process(pdata);
 	}
+	traceeval_iterator_put(iter);
 }
 
 static void display(struct task_data *tdata)
 {
 	struct traceeval *teval = tdata->teval_cpus;
-	struct traceeval_iterator *iter = traceeval_iterator_get(teval);
+	struct traceeval_iterator *iter;
 	const struct traceeval_data *keys;
 	struct traceeval_stat *stat;
 	unsigned long long total_time = 0;
@@ -997,6 +1002,8 @@ static void display(struct task_data *tdata)
 		return display_processes(tdata->teval_tasks);
 	}
 
+	iter = traceeval_iterator_get(teval);
+
 	printf("Total:\n");
 
 	if (!iter)
@@ -1021,6 +1028,8 @@ static void display(struct task_data *tdata)
 		}
 	}
 
+	traceeval_iterator_put(iter);
+
 	printf("  Total  run time (us):");
 	print_microseconds_nl(16, total_time);
 	printf("  Total idle time (us):");
-- 
2.42.0


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

* [PATCH 11/11] libtraceeval task-evals: Free the libtraceevals
  2023-10-11  3:25 [PATCH 00/11] libtraceeval task-eval: Updates to evaluate tasks Steven Rostedt
                   ` (9 preceding siblings ...)
  2023-10-11  3:25 ` [PATCH 10/11] libtraceveal task-eval: Clean up all traceeval_iterators Steven Rostedt
@ 2023-10-11  3:25 ` Steven Rostedt
  10 siblings, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2023-10-11  3:25 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Ross Zwisler, Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Add code to free the allocated traceveals including the ones that are
daisy chained in the teval_tasks.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 samples/task-eval.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/samples/task-eval.c b/samples/task-eval.c
index 25032293c9a3..382e30514ec3 100644
--- a/samples/task-eval.c
+++ b/samples/task-eval.c
@@ -225,6 +225,20 @@ static void assign_task_keys(struct traceeval_data keys[2],
 	TRACEEVAL_SET_NUMBER(keys[1], state);
 }
 
+static void release_data(const struct traceeval_type *type,
+			  struct traceeval_data *data);
+
+static int copy_data(const struct traceeval_type *type,
+		     struct traceeval_data *dst,
+		     const struct traceeval_data *src)
+{
+	if (dst->pointer && dst->pointer != src->pointer)
+		die("Pointers do not match!");
+	/* This prevents releases of data */
+	*dst = *src;
+	return 0;
+}
+
 /*
  * For each state the process is in, record the time delta for
  * that state. Also, only for the RUNNING state, this will
@@ -237,6 +251,8 @@ static struct traceeval_type task_vals[] = {
 	{
 		.type = TRACEEVAL_TYPE_POINTER,
 		.name = "data",
+		.release = release_data,
+		.copy = copy_data,
 	},
 	{
 		.type = TRACEEVAL_TYPE_DELTA,
@@ -326,6 +342,21 @@ enum command {
 	STOP
 };
 
+static void release_data(const struct traceeval_type *type,
+			  struct traceeval_data *data)
+{
+	struct process_data *pdata;
+
+	if (!data || !data->pointer)
+		return;
+
+	pdata = data->pointer;
+	traceeval_release(pdata->teval_cpus);
+	traceeval_release(pdata->teval_threads);
+	free(pdata);
+	data->pointer = NULL;
+}
+
 static void init_process_data(struct process_data *pdata)
 {
 	pdata->teval_cpus = traceeval_init(cpu_keys, cpu_vals);
@@ -1043,6 +1074,11 @@ static void display(struct task_data *tdata)
 
 static void free_tdata(struct task_data *tdata)
 {
+	if (!tdata)
+		return;
+
+	traceeval_release(tdata->teval_cpus);
+	traceeval_release(tdata->teval_tasks);
 }
 
 /*
-- 
2.42.0


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

end of thread, other threads:[~2023-10-11  3:25 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-11  3:25 [PATCH 00/11] libtraceeval task-eval: Updates to evaluate tasks Steven Rostedt
2023-10-11  3:25 ` [PATCH 01/11] libtraceeval task-eval: Fix naming conventions of variables Steven Rostedt
2023-10-11  3:25 ` [PATCH 02/11] libtraceeval task-eval: Use "assign_*keys/vals()" helper functions Steven Rostedt
2023-10-11  3:25 ` [PATCH 03/11] libtraceeval task-eval: Add tracking of priority Steven Rostedt
2023-10-11  3:25 ` [PATCH 04/11] libtraceeval task-eval: Add way to read trace.dat buffer instances Steven Rostedt
2023-10-11  3:25 ` [PATCH 05/11] libtraceeval task-eval: Account for ZOMBIE and EXITED states Steven Rostedt
2023-10-11  3:25 ` [PATCH 06/11] libtraceeval task-eval: Show max, min and count of deltas too Steven Rostedt
2023-10-11  3:25 ` [PATCH 07/11] libtraceeval task-eval: Add more comments Steven Rostedt
2023-10-11  3:25 ` [PATCH 08/11] libtraceeval task-eval: Only release results if found Steven Rostedt
2023-10-11  3:25 ` [PATCH 09/11] libtraceeval task-eval: Do not add pdata to non RUNNING tasks Steven Rostedt
2023-10-11  3:25 ` [PATCH 10/11] libtraceveal task-eval: Clean up all traceeval_iterators Steven Rostedt
2023-10-11  3:25 ` [PATCH 11/11] libtraceeval task-evals: Free the libtraceevals Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).