On Thu, 9 Dec 2021 12:45:24 +0200
"Yordan Karadzhov (VMware)" <y.karadz@xxxxxxxxx> wrote:
> The following APIs are added:
> int tracefs_synth_echo_fmt(struct trace_seq *, struct tracefs_synth *)
> const char *tracefs_synth_show_event_row(struct tracefs_synth *)
> const char *tracefs_synth_show_start_hist_row(struct tracefs_synth *)
> const char *tracefs_synth_show_end_hist_row(struct tracefs_synth *)
OK, I'm going to do a little bikeshedding here ;-)
The word "row" seems off. I wonder if it would be better to just drop it.
tracefs_synth_show_event()
tracefs_synth_show_start_hist()
tracefs_synth_show_end_hist()
As to me "row" means this is a table and related somehow to the
tracefs_sql() that can create a synthetic event. But these have nothing to
do with the rows of those tables.
>
> The new APIs can be useful in the case whene the user wants to
> check what exactly gets passed to the kernel as definition of
> a synth event.
>
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@xxxxxxxxx>
> ---
> include/tracefs.h | 5 +++
> src/tracefs-hist.c | 97 +++++++++++++++++++++++++++++++++++++---------
> 2 files changed, 84 insertions(+), 18 deletions(-)
>
> diff --git a/include/tracefs.h b/include/tracefs.h
> index 967690f..0482b0a 100644
> --- a/include/tracefs.h
> +++ b/include/tracefs.h
> @@ -250,6 +250,7 @@ enum tracefs_dynevent_type {
> TRACEFS_DYNEVENT_SYNTH = 1 << 5,
> TRACEFS_DYNEVENT_MAX = 1 << 6,
> };
> +
> int tracefs_dynevent_create(struct tracefs_dynevent *devent);
> int tracefs_dynevent_destroy(struct tracefs_dynevent *devent, bool force);
> int tracefs_dynevent_destroy_all(unsigned int types, bool force);
> @@ -550,6 +551,10 @@ int tracefs_synth_create(struct tracefs_synth *synth);
> int tracefs_synth_destroy(struct tracefs_synth *synth);
> void tracefs_synth_free(struct tracefs_synth *synth);
> int tracefs_synth_echo_cmd(struct trace_seq *seq, struct tracefs_synth *synth);
> +int tracefs_synth_echo_fmt(struct trace_seq *seq, struct tracefs_synth *synth);
> +const char *tracefs_synth_show_event_row(struct tracefs_synth *synth);
> +const char *tracefs_synth_show_start_hist_row(struct tracefs_synth *synth);
> +const char *tracefs_synth_show_end_hist_row(struct tracefs_synth *synth);
>
> struct tracefs_synth *tracefs_sql(struct tep_handle *tep, const char *name,
> const char *sql_buffer, char **err);
> diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
> index 6051ecc..ad9648f 100644
> --- a/src/tracefs-hist.c
> +++ b/src/tracefs-hist.c
> @@ -683,6 +683,8 @@ struct tracefs_synth {
> struct action *actions;
> struct action **next_action;
> struct tracefs_dynevent *dyn_event;
> + char *start_hist;
> + char *end_hist;
> char *name;
> char **synthetic_fields;
> char **synthetic_args;
> @@ -737,6 +739,8 @@ void tracefs_synth_free(struct tracefs_synth *synth)
> return;
>
> free(synth->name);
> + free(synth->start_hist);
> + free(synth->end_hist);
> tracefs_list_free(synth->synthetic_fields);
> tracefs_list_free(synth->synthetic_args);
> tracefs_list_free(synth->start_keys);
> @@ -1802,6 +1806,70 @@ static char *create_end_hist(struct tracefs_synth *synth)
> return create_actions(end_hist, synth);
> }
>
> +/*
> + * tracefs_synth_echo_fmt - show the raw format of a synthetic event
> + * @seq: A trace_seq to store the format string
> + * @synth: The synthetic event to read format from
> + *
> + * This shows the raw format that describes the synthetic event, including
> + * the format of the dynamic event and the start / end histograms.
> + *
> + * Returns 0 on succes -1 on error.
> + */
> +int tracefs_synth_echo_fmt(struct trace_seq *seq, struct tracefs_synth *synth)
> +{
> + if (!synth->dyn_event)
> + return -1;
> +
> + trace_seq_printf(seq, "%s", synth->dyn_event->format);
> + trace_seq_printf(seq, "\n%s", synth->start_hist);
> + trace_seq_printf(seq, "\n%s", synth->end_hist);
> +
> + return 0;
> +}
> +
> +/*
> + * tracefs_synth_show_event_row - show the dynamic event used by a synthetic
> + * event
FYI, this should be on a single line. A "kernel-doc" starts with "a one
line description". Either shorten it, or just keep it on one line. Linux
has extended the "max line chars" to 100 from 80.
> + * @synth: The synthetic event to read format from
> + *
> + * This shows the raw format of the dynamic event used by the synthetic event.
> + *
> + * Returns format string owned by @synth on success, or NULL on error.
> + */
> +const char *tracefs_synth_show_event_row(struct tracefs_synth *synth)
> +{
> + return synth->dyn_event ? synth->dyn_event->format : NULL;
> +}
> +
> +/*
> + * tracefs_synth_show_start_hist_row - show the start histogram used by a
> + * synthetic event
Here too.
And if we get rid of "_row" it should make it shorter anyway.
-- Steve
> + * @synth: The synthetic event to read format from
> + *
> + * This shows the raw format of the start histogram used by the synthetic event.
> + *
> + * Returns format string owned by @synth on success, or NULL on error.
> + */
> +const char *tracefs_synth_show_start_hist_row(struct tracefs_synth *synth)
> +{
> + return synth->start_hist;
> +}
> +
|