Re: [PATCH] libtraceevent: A new API for trace page size

[Steven Rostedt <rostedt@goodmis.org>]

On Fri,  1 Oct 2021 09:23:38 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Currently, the size of a trace buffer page is equal to the system memory
> page size. This may change in the future, so this assumption may not be
> valid. The proper way to determine the trace buffer page size is by
> using the information from "events/header_page" ftrace file:
>  ...
>  field: char data;	offset:16;	size:4080;	signed:1;
> The trace buffer page size is the size of the "data" filed + its offset.
> 
> A new libtraceevent API is introduced, for getting the trace buffer page
> size using that formula:
>  tep_get_trace_page_size()
> 
> Note, that some old kernels may not have "events/header_page" file. For
> those, the API returns the system memory page size.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  src/event-parse-api.c | 16 ++++++++++++++++
>  src/event-parse.h     |  1 +
>  2 files changed, 17 insertions(+)
> 
> diff --git a/src/event-parse-api.c b/src/event-parse-api.c
> index f8361e4..b5be8d3 100644
> --- a/src/event-parse-api.c
> +++ b/src/event-parse-api.c
> @@ -4,6 +4,7 @@
>   *
>   */
>  
> +#include <unistd.h>
>  #include "event-parse.h"
>  #include "event-parse-local.h"
>  #include "event-utils.h"
> @@ -248,6 +249,21 @@ void tep_set_page_size(struct tep_handle *tep, int _page_size)
>  		tep->page_size = _page_size;
>  }
>  
> +/**
> + * tep_get_trace_page_size - get the size of a trace page on the traced machine
> + * @tep: a handle to the tep_handle
> + *
> + * This returns the size of a trace page on the traced machine
> + * If @tep is NULL or the kernel is old, user space page size is returned.
> + */
> +int tep_get_trace_page_size(struct tep_handle *tep)

Let's call it:

  tep_get_sub_buffer_size()

as it has nothing to do with pages. (the header_page is really a misnomer).

> +{
> +	if (tep && !tep->old_format)

The old format should be updated to produce the same, and not have the
check here. And if tep is NULL, it should return -1;

> +		return tep->header_page_data_size + tep->header_page_data_offset;
> +
> +	return getpagesize();
> +}

That is, in tep_parse_header_page() we should have:

	if (!size) {
		/*
		 * Old kernels did not have header page info.
		 * Sorry but we just use what we find here in user space.
		 */
		tep->header_page_ts_size = sizeof(long long);
		tep->header_page_size_size = long_size;
		tep->header_page_data_offset = sizeof(long long) + long_size;
+		tep->header_page_data_size = getpagesize() - tep->header_page_data_offset;
		tep->old_format = 1;
		return -1;
	}

-- Steve


> +
>  /**
>   * tep_is_file_bigendian - return the endian of the file
>   * @tep: a handle to the tep_handle
> diff --git a/src/event-parse.h b/src/event-parse.h
> index 0833893..ea8df35 100644
> --- a/src/event-parse.h
> +++ b/src/event-parse.h
> @@ -572,6 +572,7 @@ void tep_set_cpus(struct tep_handle *tep, int cpus);
>  int tep_get_long_size(struct tep_handle *tep);
>  void tep_set_long_size(struct tep_handle *tep, int long_size);
>  int tep_get_page_size(struct tep_handle *tep);
> +int tep_get_trace_page_size(struct tep_handle *tep);
>  void tep_set_page_size(struct tep_handle *tep, int _page_size);
>  bool tep_is_file_bigendian(struct tep_handle *tep);
>  void tep_set_file_bigendian(struct tep_handle *tep, enum tep_endian endian);