[PATCH] libtracecmd: Fix continuing in tracecmd_iterate_events_reverse()

[PATCH] libtracecmd: Fix continuing in tracecmd_iterate_events_reverse()

[Steven Rostedt]

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

When a callback from tracecmd_iterate_events_reverse() returns non-zero it
cause the function to exit out early. Calling tracecmd_iterate_events_reverse()
again with cont=true, is supposed to restart where it left off. But
because of the way the reverse traverses all the events on a page, by
calling tracecmd_read_data() to find the next task, it does not pick the
next task. It can pick the next task at the start of the page.

If the callback back causes the function to exit out early, record the
page_offset of the last record read, and then set the cursor back to that
event.

Link: https://lore.kernel.org/all/20251121120117.20e82d9e@gandalf.local.home/

Fixes: 56cbc522518a5 ("trace-cmd library: Add tracecmd_iterate_events_reverse() API")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 lib/trace-cmd/trace-input.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index afdbd2aa98b6..8dfeff458485 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -3036,6 +3036,7 @@ int tracecmd_iterate_events_reverse(struct tracecmd_input *handle,
 				    void *callback_data, bool cont)
 {
 	unsigned long long last_timestamp = 0;
+	unsigned long long page_offset = 0;
 	struct tep_record **records;
 	struct tep_record *record;
 	int next_cpu;
@@ -3072,6 +3073,8 @@ int tracecmd_iterate_events_reverse(struct tracecmd_input *handle,
 			record = next_last_event(handle, records, next_cpu);;
 			ret = call_callbacks(handle, record, next_cpu,
 					     callback, callback_data);
+			if (ret)
+				page_offset = record->offset;
 			tracecmd_free_record(record);
 		}
 	} while (next_cpu >= 0 && ret == 0);
@@ -3079,6 +3082,17 @@ int tracecmd_iterate_events_reverse(struct tracecmd_input *handle,
 	free_last_events(handle, records, cpus, cpu_size, max_cpus);
 	free(records);
 
+	/*
+	 * If the callback exited out early, then set the cursor back
+	 * to the location of that record so that if this gets called
+	 * again with cont = true, it will continue where it left off.
+	 */
+	if (page_offset) {
+		/* Set the record to the previous record that was read */
+		record = tracecmd_read_at(handle, page_offset - 4, NULL);
+		tracecmd_free_record(record);
+	}
+
 	return ret;
 }
 
-- 
2.51.0

Re: [PATCH] libtracecmd: Fix continuing in tracecmd_iterate_events_reverse()

[MOESSBAUER, Felix]

On Fri, 2025-11-21 at 12:09 -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> 
> When a callback from tracecmd_iterate_events_reverse() returns non-zero it
> cause the function to exit out early. Calling tracecmd_iterate_events_reverse()
> again with cont=true, is supposed to restart where it left off. But
> because of the way the reverse traverses all the events on a page, by
> calling tracecmd_read_data() to find the next task, it does not pick the
> next task. It can pick the next task at the start of the page.
> 
> If the callback back causes the function to exit out early, record the
> page_offset of the last record read, and then set the cursor back to that
> event.
> 
> Link: https://lore.kernel.org/all/20251121120117.20e82d9e@gandalf.local.home/
> 
> Fixes: 56cbc522518a5 ("trace-cmd library: Add tracecmd_iterate_events_reverse() API")
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  lib/trace-cmd/trace-input.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> index afdbd2aa98b6..8dfeff458485 100644
> --- a/lib/trace-cmd/trace-input.c
> +++ b/lib/trace-cmd/trace-input.c
> @@ -3036,6 +3036,7 @@ int tracecmd_iterate_events_reverse(struct tracecmd_input *handle,
>  				    void *callback_data, bool cont)
>  {
>  	unsigned long long last_timestamp = 0;
> +	unsigned long long page_offset = 0;
>  	struct tep_record **records;
>  	struct tep_record *record;
>  	int next_cpu;
> @@ -3072,6 +3073,8 @@ int tracecmd_iterate_events_reverse(struct tracecmd_input *handle,
>  			record = next_last_event(handle, records, next_cpu);;
>  			ret = call_callbacks(handle, record, next_cpu,
>  					     callback, callback_data);
> +			if (ret)
> +				page_offset = record->offset;
>  			tracecmd_free_record(record);
>  		}
>  	} while (next_cpu >= 0 && ret == 0);
> @@ -3079,6 +3082,17 @@ int tracecmd_iterate_events_reverse(struct tracecmd_input *handle,
>  	free_last_events(handle, records, cpus, cpu_size, max_cpus);
>  	free(records);
>  
> +	/*
> +	 * If the callback exited out early, then set the cursor back
> +	 * to the location of that record so that if this gets called
> +	 * again with cont = true, it will continue where it left off.
> +	 */
> +	if (page_offset) {
> +		/* Set the record to the previous record that was read */
> +		record = tracecmd_read_at(handle, page_offset - 4, NULL);

Hi,

where does the magic "- 4" come from?

> +		tracecmd_free_record(record);

I still don't get why this fixes the issue. Does tracecmd_read_at has a
side-effect to set the the internal cursor? If so, it IMHO would help
to document this side effect in tracecmd_read_at().

Felix

> +	}
> +
>  	return ret;
>  }
>  
> -- 
> 2.51.0

-- 
Siemens AG
Linux Expert Center
Friedrich-Ludwig-Bauer-Str. 3
85748 Garching, Germany