* [RFC PATCH] libtracefs: New APIs for getting the raw format of synth event
@ 2021-12-06 15:13 Yordan Karadzhov (VMware)
2021-12-07 22:13 ` Steven Rostedt
0 siblings, 1 reply; 4+ messages in thread
From: Yordan Karadzhov (VMware) @ 2021-12-06 15:13 UTC (permalink / raw)
To: linux-trace-devel, rostedt; +Cc: Yordan Karadzhov (VMware)
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@gmail.com>
---
include/tracefs.h | 4 ++
src/tracefs-hist.c | 102 +++++++++++++++++++++++++++++++++++++--------
2 files changed, 88 insertions(+), 18 deletions(-)
diff --git a/include/tracefs.h b/include/tracefs.h
index 804d738..9ad036c 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);
@@ -553,6 +554,9 @@ 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);
+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 41c88d1..338249d 100644
--- a/src/tracefs-hist.c
+++ b/src/tracefs-hist.c
@@ -717,6 +717,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;
@@ -771,6 +773,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);
@@ -1067,6 +1071,9 @@ struct tracefs_synth *tracefs_synth_alloc(struct tep_handle *tep,
synth = NULL;
}
+ synth->start_hist = NULL;
+ synth->end_hist = NULL;
+
return synth;
}
@@ -1836,6 +1843,72 @@ 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_instance *instance,
+ struct tracefs_synth *synth)
+{
+ if (!synth->dyn_event)
+ return -1;
+
+ trace_seq_printf(seq, "%s\n", synth->dyn_event->format);
+ trace_seq_printf(seq, "%s\n", synth->start_hist);
+ trace_seq_printf(seq, "%s\n", synth->end_hist);
+
+ return 0;
+}
+
+/*
+ * tracefs_synth_show_event_row - show the dynamic event used by a synthetic
+ * event
+ * @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 on succes 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
+ * @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 on succes or NULL on error.
+ */
+const char *tracefs_synth_show_start_hist_row(struct tracefs_synth *synth)
+{
+ return synth->start_hist;
+}
+
+/*
+ * tracefs_synth_show_end_hist_row - show the end histogram used by a
+ * synthetic event
+ * @synth: The synthetic event to read format from
+ *
+ * This shows the raw format of the end histogram used by the synthetic event.
+ *
+ * Returns format string on succes or NULL on error.
+ */
+const char *tracefs_synth_show_end_hist_row(struct tracefs_synth *synth)
+{
+ return synth->end_hist;
+}
+
static char *append_filter(char *hist, char *filter, unsigned int parens)
{
int i;
@@ -1974,8 +2047,6 @@ tracefs_synth_get_start_hist(struct tracefs_synth *synth)
*/
int tracefs_synth_create(struct tracefs_synth *synth)
{
- char *start_hist = NULL;
- char *end_hist = NULL;
int ret;
if (!synth) {
@@ -1996,40 +2067,35 @@ int tracefs_synth_create(struct tracefs_synth *synth)
if (tracefs_dynevent_create(synth->dyn_event))
return -1;
- start_hist = create_hist(synth->start_keys, synth->start_vars);
- start_hist = append_filter(start_hist, synth->start_filter,
- synth->start_parens);
- if (!start_hist)
+ synth->start_hist = create_hist(synth->start_keys, synth->start_vars);
+ synth->start_hist = append_filter(synth->start_hist, synth->start_filter,
+ synth->start_parens);
+ if (!synth->start_hist)
goto remove_synthetic;
- end_hist = create_end_hist(synth);
- end_hist = append_filter(end_hist, synth->end_filter,
- synth->end_parens);
- if (!end_hist)
+ synth->end_hist = create_end_hist(synth);
+ synth->end_hist = append_filter(synth->end_hist, synth->end_filter,
+ synth->end_parens);
+ if (!synth->end_hist)
goto remove_synthetic;
ret = tracefs_event_file_append(synth->instance, synth->start_event->system,
synth->start_event->name,
- "trigger", start_hist);
+ "trigger", synth->start_hist);
if (ret < 0)
goto remove_synthetic;
ret = tracefs_event_file_append(synth->instance, synth->end_event->system,
synth->end_event->name,
- "trigger", end_hist);
+ "trigger", synth->end_hist);
if (ret < 0)
goto remove_start_hist;
- free(start_hist);
- free(end_hist);
-
return 0;
remove_start_hist:
- remove_hist(synth->instance, synth->start_event, start_hist);
+ remove_hist(synth->instance, synth->start_event, synth->start_hist);
remove_synthetic:
- free(end_hist);
- free(start_hist);
tracefs_dynevent_destroy(synth->dyn_event, false);
return -1;
}
--
2.32.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] libtracefs: New APIs for getting the raw format of synth event
2021-12-06 15:13 [RFC PATCH] libtracefs: New APIs for getting the raw format of synth event Yordan Karadzhov (VMware)
@ 2021-12-07 22:13 ` Steven Rostedt
2021-12-08 13:31 ` Yordan Karadzhov
0 siblings, 1 reply; 4+ messages in thread
From: Steven Rostedt @ 2021-12-07 22:13 UTC (permalink / raw)
To: Yordan Karadzhov (VMware); +Cc: linux-trace-devel
On Mon, 6 Dec 2021 17:13:52 +0200
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 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.
Please list the APIs.
>
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> ---
> include/tracefs.h | 4 ++
> src/tracefs-hist.c | 102 +++++++++++++++++++++++++++++++++++++--------
> 2 files changed, 88 insertions(+), 18 deletions(-)
>
> diff --git a/include/tracefs.h b/include/tracefs.h
> index 804d738..9ad036c 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);
> @@ -553,6 +554,9 @@ 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);
> +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 41c88d1..338249d 100644
> --- a/src/tracefs-hist.c
> +++ b/src/tracefs-hist.c
> @@ -717,6 +717,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;
> @@ -771,6 +773,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);
> @@ -1067,6 +1071,9 @@ struct tracefs_synth *tracefs_synth_alloc(struct tep_handle *tep,
> synth = NULL;
> }
>
> + synth->start_hist = NULL;
> + synth->end_hist = NULL;
> +
> return synth;
> }
>
> @@ -1836,6 +1843,72 @@ 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_instance *instance,
> + struct tracefs_synth *synth)
> +{
> + if (!synth->dyn_event)
> + return -1;
> +
> + trace_seq_printf(seq, "%s\n", synth->dyn_event->format);
> + trace_seq_printf(seq, "%s\n", synth->start_hist);
> + trace_seq_printf(seq, "%s\n", synth->end_hist);
> +
> + return 0;
> +}
> +
> +/*
> + * tracefs_synth_show_event_row - show the dynamic event used by a synthetic
> + * event
> + * @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 on succes or NULL on error.
Should probably state that the string returned belongs to the synth
descriptor.
> + */
> +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
> + * @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 on succes or NULL on error.
Here too. And typo in all the comments. "success"
> + */
> +const char *tracefs_synth_show_start_hist_row(struct tracefs_synth *synth)
> +{
> + return synth->start_hist;
> +}
> +
> +/*
> + * tracefs_synth_show_end_hist_row - show the end histogram used by a
> + * synthetic event
> + * @synth: The synthetic event to read format from
> + *
> + * This shows the raw format of the end histogram used by the synthetic event.
> + *
> + * Returns format string on succes or NULL on error.
Same here.
> + */
> +const char *tracefs_synth_show_end_hist_row(struct tracefs_synth *synth)
> +{
> + return synth->end_hist;
> +}
> +
> static char *append_filter(char *hist, char *filter, unsigned int parens)
> {
> int i;
> @@ -1974,8 +2047,6 @@ tracefs_synth_get_start_hist(struct tracefs_synth *synth)
> */
> int tracefs_synth_create(struct tracefs_synth *synth)
> {
> - char *start_hist = NULL;
> - char *end_hist = NULL;
> int ret;
>
> if (!synth) {
> @@ -1996,40 +2067,35 @@ int tracefs_synth_create(struct tracefs_synth *synth)
> if (tracefs_dynevent_create(synth->dyn_event))
> return -1;
>
> - start_hist = create_hist(synth->start_keys, synth->start_vars);
> - start_hist = append_filter(start_hist, synth->start_filter,
> - synth->start_parens);
> - if (!start_hist)
> + synth->start_hist = create_hist(synth->start_keys, synth->start_vars);
> + synth->start_hist = append_filter(synth->start_hist, synth->start_filter,
> + synth->start_parens);
> + if (!synth->start_hist)
> goto remove_synthetic;
>
> - end_hist = create_end_hist(synth);
> - end_hist = append_filter(end_hist, synth->end_filter,
> - synth->end_parens);
> - if (!end_hist)
> + synth->end_hist = create_end_hist(synth);
> + synth->end_hist = append_filter(synth->end_hist, synth->end_filter,
> + synth->end_parens);
> + if (!synth->end_hist)
> goto remove_synthetic;
>
> ret = tracefs_event_file_append(synth->instance, synth->start_event->system,
> synth->start_event->name,
> - "trigger", start_hist);
> + "trigger", synth->start_hist);
> if (ret < 0)
> goto remove_synthetic;
>
> ret = tracefs_event_file_append(synth->instance, synth->end_event->system,
> synth->end_event->name,
> - "trigger", end_hist);
> + "trigger", synth->end_hist);
> if (ret < 0)
> goto remove_start_hist;
>
> - free(start_hist);
> - free(end_hist);
> -
> return 0;
>
> remove_start_hist:
> - remove_hist(synth->instance, synth->start_event, start_hist);
> + remove_hist(synth->instance, synth->start_event, synth->start_hist);
> remove_synthetic:
> - free(end_hist);
> - free(start_hist);
> tracefs_dynevent_destroy(synth->dyn_event, false);
I just realized that on error, we do not free the synthetic event
descriptor. So there is a memory leak.
Also, I'm thinking that the specific functions for destroying the synthetic
events and such, should live here, and trace-dynevents.c should call them.
Because all the management for synthetic events should live in this file
(or go to its own file).
-- Steve
> return -1;
> }
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] libtracefs: New APIs for getting the raw format of synth event
2021-12-07 22:13 ` Steven Rostedt
@ 2021-12-08 13:31 ` Yordan Karadzhov
2021-12-08 23:24 ` Steven Rostedt
0 siblings, 1 reply; 4+ messages in thread
From: Yordan Karadzhov @ 2021-12-08 13:31 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-trace-devel
On 8.12.21 г. 0:13 ч., Steven Rostedt wrote:
>> int tracefs_synth_create(struct tracefs_synth *synth)
>> {
>> - char *start_hist = NULL;
>> - char *end_hist = NULL;
>> int ret;
>>
>> if (!synth) {
>> @@ -1996,40 +2067,35 @@ int tracefs_synth_create(struct tracefs_synth *synth)
>> if (tracefs_dynevent_create(synth->dyn_event))
>> return -1;
>>
>> - start_hist = create_hist(synth->start_keys, synth->start_vars);
>> - start_hist = append_filter(start_hist, synth->start_filter,
>> - synth->start_parens);
>> - if (!start_hist)
>> + synth->start_hist = create_hist(synth->start_keys, synth->start_vars);
>> + synth->start_hist = append_filter(synth->start_hist, synth->start_filter,
>> + synth->start_parens);
>> + if (!synth->start_hist)
>> goto remove_synthetic;
>>
>> - end_hist = create_end_hist(synth);
>> - end_hist = append_filter(end_hist, synth->end_filter,
>> - synth->end_parens);
>> - if (!end_hist)
>> + synth->end_hist = create_end_hist(synth);
>> + synth->end_hist = append_filter(synth->end_hist, synth->end_filter,
>> + synth->end_parens);
>> + if (!synth->end_hist)
>> goto remove_synthetic;
>>
>> ret = tracefs_event_file_append(synth->instance, synth->start_event->system,
>> synth->start_event->name,
>> - "trigger", start_hist);
>> + "trigger", synth->start_hist);
>> if (ret < 0)
>> goto remove_synthetic;
>>
>> ret = tracefs_event_file_append(synth->instance, synth->end_event->system,
>> synth->end_event->name,
>> - "trigger", end_hist);
>> + "trigger", synth->end_hist);
>> if (ret < 0)
>> goto remove_start_hist;
>>
>> - free(start_hist);
>> - free(end_hist);
>> -
>> return 0;
>>
>> remove_start_hist:
>> - remove_hist(synth->instance, synth->start_event, start_hist);
>> + remove_hist(synth->instance, synth->start_event, synth->start_hist);
>> remove_synthetic:
>> - free(end_hist);
>> - free(start_hist);
>> tracefs_dynevent_destroy(synth->dyn_event, false);
> I just realized that on error, we do not free the synthetic event
> descriptor. So there is a memory leak.
>
Do we need to free the synthetic event here?
The event gets allocated with 'tracefs_synth_alloc()' and the user is responsible for calling 'tracefs_synth_free()' in
order to free the memory.
The fact that 'tracefs_synth_create()' may fail doesn't change this.
Thanks!
Yordan
> Also, I'm thinking that the specific functions for destroying the synthetic
> events and such, should live here, and trace-dynevents.c should call them.
>
> Because all the management for synthetic events should live in this file
> (or go to its own file).
>
> -- Steve
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] libtracefs: New APIs for getting the raw format of synth event
2021-12-08 13:31 ` Yordan Karadzhov
@ 2021-12-08 23:24 ` Steven Rostedt
0 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2021-12-08 23:24 UTC (permalink / raw)
To: Yordan Karadzhov; +Cc: linux-trace-devel
On Wed, 8 Dec 2021 15:31:26 +0200
Yordan Karadzhov <y.karadz@gmail.com> wrote:
> >> remove_start_hist:
> >> - remove_hist(synth->instance, synth->start_event, start_hist);
> >> + remove_hist(synth->instance, synth->start_event, synth->start_hist);
> >> remove_synthetic:
> >> - free(end_hist);
> >> - free(start_hist);
> >> tracefs_dynevent_destroy(synth->dyn_event, false);
> > I just realized that on error, we do not free the synthetic event
> > descriptor. So there is a memory leak.
> >
>
> Do we need to free the synthetic event here?
>
> The event gets allocated with 'tracefs_synth_alloc()' and the user is responsible for calling 'tracefs_synth_free()' in
> order to free the memory.
> The fact that 'tracefs_synth_create()' may fail doesn't change this.
Ah, you're correct. I was thinking that we allocated it here, but forgot we
separated create and alloc.
-- Steve
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-12-08 23:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-06 15:13 [RFC PATCH] libtracefs: New APIs for getting the raw format of synth event Yordan Karadzhov (VMware)
2021-12-07 22:13 ` Steven Rostedt
2021-12-08 13:31 ` Yordan Karadzhov
2021-12-08 23:24 ` 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).