diff mbox series

[1/3] libtracefs: New API for getting the raw format of a histogram

Message ID 20211206150855.361202-2-y.karadz@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series libtracefs: APIs for inspecting of hist and synth objects | expand

Commit Message

Yordan Karadzhov Dec. 6, 2021, 3:08 p.m. UTC
The new API is similar to tracefs_hist_echo_cmd(), but prints just
the raw format descriptor of the histogram. It can be useful in the
case whene the user wants to check what exactly gets passed to the
kernel as definition of the histogram.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 Documentation/libtracefs-hist.txt | 11 ++++++++--
 include/tracefs.h                 |  3 +++
 src/tracefs-hist.c                | 34 +++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 2 deletions(-)

Comments

Steven Rostedt Dec. 6, 2021, 11:58 p.m. UTC | #1
On Mon,  6 Dec 2021 17:08:53 +0200
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> +/*
> + * tracefs_hist_echo_fmt - show the raw format of the histogram
> + * @seq: A trace_seq to store the format string
> + * @hist: The histogram to read format from
> + *
> + * This show the raw format that describes the state of the histogram.
> + *
> + * Returns the size of the format string on succes -1 on error.
> + */
> +int tracefs_hist_echo_fmt(struct trace_seq *seq,
> +			  struct tracefs_instance *instance,
> +			  struct tracefs_hist *hist)
> +{
> +	const char *event = hist->event_name;
> +	const char *system = hist->system;
> +	int fmt_size;
> +	char *path;
> +	char *fmt;
> +
> +	path = tracefs_event_get_file(instance, system, event, "trigger");
> +	if (!path)
> +		return -1;
> +
> +	fmt = tracefs_event_file_read(instance, system, event, path,
> +				      &fmt_size);

First, path should just be "trigger" as tracefs_event_file_read() is a
short cut to get to the files for the event. I'm surprised that this even
worked.

> +	if (!fmt)
> +		return -1;
> +
> +	trace_seq_puts(seq, fmt);

Second, just reading the trigger file is not useful, as the trigger file
holds triggers, not just histograms. And you can have more than one
histogram in the trigger file. For instance, I could do:

 # cd /sys/kernel/debug/tracing/events/sched/sched_switch
 # echo stacktrace > trigger
 # echo 'hist:keys=prev_pid' >> trigger
 # echo 'hist:keys=next_pid' >> trigger
 # cat trigger
stacktrace:unlimited
hist:keys=prev_pid:vals=hitcount:sort=hitcount:size=2048 [active]
hist:keys=next_pid:vals=hitcount:sort=hitcount:size=2048 [active]

Is this really what you want?

-- Steve

> +	tracefs_put_tracing_file(path);
> +
> +	return fmt_size;
> +}
> +
>  /*
>   * tracefs_hist_command - Create, start, pause, destroy a histogram for an event
>   * @instance: The instance the histogram will be in (NULL for toplevel)
Yordan Karadzhov Dec. 7, 2021, 1:52 p.m. UTC | #2
On 7.12.21 г. 1:58 ч., Steven Rostedt wrote:
> On Mon,  6 Dec 2021 17:08:53 +0200
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
>> +/*
>> + * tracefs_hist_echo_fmt - show the raw format of the histogram
>> + * @seq: A trace_seq to store the format string
>> + * @hist: The histogram to read format from
>> + *
>> + * This show the raw format that describes the state of the histogram.
>> + *
>> + * Returns the size of the format string on succes -1 on error.
>> + */
>> +int tracefs_hist_echo_fmt(struct trace_seq *seq,
>> +			  struct tracefs_instance *instance,
>> +			  struct tracefs_hist *hist)
>> +{
>> +	const char *event = hist->event_name;
>> +	const char *system = hist->system;
>> +	int fmt_size;
>> +	char *path;
>> +	char *fmt;
>> +
>> +	path = tracefs_event_get_file(instance, system, event, "trigger");
>> +	if (!path)
>> +		return -1;
>> +
>> +	fmt = tracefs_event_file_read(instance, system, event, path,
>> +				      &fmt_size);
> 
> First, path should just be "trigger" as tracefs_event_file_read() is a
> short cut to get to the files for the event. I'm surprised that this even
> worked.
> 
>> +	if (!fmt)
>> +		return -1;
>> +
>> +	trace_seq_puts(seq, fmt);
> 
> Second, just reading the trigger file is not useful, as the trigger file
> holds triggers, not just histograms. And you can have more than one
> histogram in the trigger file. For instance, I could do:
> 
>   # cd /sys/kernel/debug/tracing/events/sched/sched_switch
>   # echo stacktrace > trigger
>   # echo 'hist:keys=prev_pid' >> trigger
>   # echo 'hist:keys=next_pid' >> trigger
>   # cat trigger
> stacktrace:unlimited
> hist:keys=prev_pid:vals=hitcount:sort=hitcount:size=2048 [active]
> hist:keys=next_pid:vals=hitcount:sort=hitcount:size=2048 [active]
> 
> Is this really what you want?
> 

No, it isn’t :( I have to rethink this.

Thanks a lot!
Y.


> -- Steve
> 
>> +	tracefs_put_tracing_file(path);
>> +
>> +	return fmt_size;
>> +}
>> +
>>   /*
>>    * tracefs_hist_command - Create, start, pause, destroy a histogram for an event
>>    * @instance: The instance the histogram will be in (NULL for toplevel)
>
diff mbox series

Patch

diff --git a/Documentation/libtracefs-hist.txt b/Documentation/libtracefs-hist.txt
index 2eef718..c8230e5 100644
--- a/Documentation/libtracefs-hist.txt
+++ b/Documentation/libtracefs-hist.txt
@@ -4,7 +4,8 @@  libtracefs(3)
 NAME
 ----
 tracefs_hist_alloc, tracefs_hist_free, tracefs_hist_add_key, tracefs_hist_add_value, tracefs_hist_add_name, tracefs_hist_start,
-tracefs_hist_destory, tracefs_hist_add_sort_key, tracefs_hist_set_sort_key,tracefs_hist_sort_key_direction - Create and update event histograms
+tracefs_hist_destory, tracefs_hist_add_sort_key, tracefs_hist_set_sort_key, tracefs_hist_sort_key_direction
+tracefs_hist_echo_cmd, tracefs_hist_echo_fmt - Create and update event histograms
 
 SYNOPSIS
 --------
@@ -69,6 +70,9 @@  int tracefs_hist_command(struct tracefs_instance pass:[*]instance,
 			 struct tracefs_hist pass:[*]hist,
 			 enum tracefs_hist_command command);
 
+int tracefs_hist_echo_fmt(struct trace_seq pass:[*]s, struct tracefs_instance pass:[*]instance,
+			  struct tracefs_hist pass:[*]hist);
+
 --
 
 DESCRIPTION
@@ -178,11 +182,14 @@  _field_, _compare_, and _val_ are ignored unless _type_ is equal to
 
 *TRACEFS_COMPARE_AND* - _field_ & _val_ : where _field_ is a flags field.
 
-*trace_hist_echo_cmd*() prints the commands needed to create the given histogram
+*tracefs_hist_echo_cmd*() prints the commands needed to create the given histogram
 in the given _instance_, or NULL for the top level, into the _seq_.
 The command that is printed is described by _command_ and shows the functionality
 that would be done by *tracefs_hist_command*(3).
 
+*tracefs_hist_echo_fmt*() prints the raw format that describes the state of the
+histogram in the given _instance_, or NULL for the top level, into the _seq_.
+
 *tracefs_hist_command*() is called to process a command on the histogram
 _hist_ for its event in the given _instance_, or NULL for the top level.
 The _cmd_ can be one of:
diff --git a/include/tracefs.h b/include/tracefs.h
index 3ac3d9c..c2a6db7 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -377,6 +377,9 @@  int tracefs_hist_echo_cmd(struct trace_seq *seq,  struct tracefs_instance *insta
 			  struct tracefs_hist *hist, enum tracefs_hist_command command);
 int tracefs_hist_command(struct tracefs_instance *instance,
 			 struct tracefs_hist *hist, enum tracefs_hist_command cmd);
+int tracefs_hist_echo_fmt(struct trace_seq *seq,
+			  struct tracefs_instance *instance,
+			  struct tracefs_hist *hist);
 
 /**
  * tracefs_hist_start - enable a histogram
diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
index ea9e127..9beafe0 100644
--- a/src/tracefs-hist.c
+++ b/src/tracefs-hist.c
@@ -142,6 +142,40 @@  tracefs_hist_echo_cmd(struct trace_seq *seq, struct tracefs_instance *instance,
 	return 0;
 }
 
+/*
+ * tracefs_hist_echo_fmt - show the raw format of the histogram
+ * @seq: A trace_seq to store the format string
+ * @hist: The histogram to read format from
+ *
+ * This show the raw format that describes the state of the histogram.
+ *
+ * Returns the size of the format string on succes -1 on error.
+ */
+int tracefs_hist_echo_fmt(struct trace_seq *seq,
+			  struct tracefs_instance *instance,
+			  struct tracefs_hist *hist)
+{
+	const char *event = hist->event_name;
+	const char *system = hist->system;
+	int fmt_size;
+	char *path;
+	char *fmt;
+
+	path = tracefs_event_get_file(instance, system, event, "trigger");
+	if (!path)
+		return -1;
+
+	fmt = tracefs_event_file_read(instance, system, event, path,
+				      &fmt_size);
+	if (!fmt)
+		return -1;
+
+	trace_seq_puts(seq, fmt);
+	tracefs_put_tracing_file(path);
+
+	return fmt_size;
+}
+
 /*
  * tracefs_hist_command - Create, start, pause, destroy a histogram for an event
  * @instance: The instance the histogram will be in (NULL for toplevel)