All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-trace-devel@vger.kernel.org
Cc: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Subject: [PATCH v2 1/7] libtracefs: Change the tracefs_hist API to not take an instance immediately
Date: Tue,  3 Aug 2021 12:48:05 -0400	[thread overview]
Message-ID: <20210803164811.693731-2-rostedt@goodmis.org> (raw)
In-Reply-To: <20210803164811.693731-1-rostedt@goodmis.org>

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

After implementing tracefs_sql(), it became apparent that passing the
instance into the tracefs_hist API to create the descriptor was the wrong
approach. It's better to just pass in a tep_handle for verification
purposes, and then have the user pass in the instance when creating the
histogram. This also allows to easily create the same histogram in
different instances if need be.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/libtracefs-hist.txt | 40 +++++++++++-------
 include/tracefs.h                 | 16 ++++----
 src/tracefs-hist.c                | 68 +++++++++++++++++--------------
 3 files changed, 71 insertions(+), 53 deletions(-)

diff --git a/Documentation/libtracefs-hist.txt b/Documentation/libtracefs-hist.txt
index 0254c5faea82..28b4c3d65cde 100644
--- a/Documentation/libtracefs-hist.txt
+++ b/Documentation/libtracefs-hist.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 --
 *#include <tracefs.h>*
 
-struct tracefs_hist pass:[*]tracefs_hist_alloc(struct tracefs_instance pass:[*] instance,
+struct tracefs_hist pass:[*]tracefs_hist_alloc(struct tracefs_tep pass:[*] tep,
 			const char pass:[*]system, const char pass:[*]event,
 			const char pass:[*]key, enum tracefs_hist_key_type type);
 void tracefs_hist_free(struct tracefs_hist pass:[*]hist);
@@ -25,8 +25,8 @@ int tracefs_hist_sort_key_direction(struct tracefs_hist pass:[*]hist,
 				    const char pass:[*]sort_key,
 				    enum tracefs_hist_sort_direction dir);
 int tracefs_hist_add_name(struct tracefs_hist pass:[*]hist, const char pass:[*]name);
-int tracefs_hist_start(struct tracefs_hist pass:[*]hist);
-int tracefs_hist_destory(struct tracefs_hist pass:[*]hist);
+int tracefs_hist_start(struct tracefs_instance pass:[*]instance, struct tracefs_hist pass:[*]hist);
+int tracefs_hist_destory(struct tracefs_instance pass:[*]instance, struct tracefs_hist pass:[*]hist);
 --
 
 DESCRIPTION
@@ -38,10 +38,11 @@ See https://www.kernel.org/doc/html/latest/trace/histogram.html for more informa
 
 *tracefs_hist_alloc*() allocates a "struct tracefs_hist" descriptor and returns
 the address of it. This descriptor must be freed by *tracefs_hist_free*().
-The _instance_ is the instance that contains the event that the histogram will be
-attached to. The _system_ is the system or group of the event. The _event_ is the event
-to attach the histogram to. The _key_ is a field of the event that will be used as
-the key of the histogram. The _type_ is the type of the _key_. See KEY TYPES below.
+The _tep_ is a trace event handle (see *tracefs_local_events*(3)), that holds the
+_system_ and _event_ that the histogram will be attached to. The _system_ is the
+system or group of the event. The _event_ is the event to attach the histogram to.
+The _key_ is a field of the event that will be used as the key of the histogram.
+The _type_ is the type of the _key_. See KEY TYPES below.
 
 *tracefs_hist_free*() frees the _tracefs_hist_ descriptor. Note, it does not stop
 or disable the running histogram if it was started. *tracefs_hist_destroy*() needs
@@ -81,15 +82,24 @@ into a single histogram (shown by either event's hist file). The _hist_
 is the histogram to name, and the _name_ is the name to give it.
 
 *tracefs_hist_start* is called to actually start the histogram _hist_.
+The _instance_ is the instance to start the histogram in, NULL if it
+should start at the top level.
 
 *tracefs_hist_pause* is called to pause the histogram _hist_.
+The _instance_ is the instance to pause the histogram in, NULL if it
+is in the top level.
 
 *tracefs_hist_continue* is called to continue a paused histogram _hist_.
+The _instance_ is the instance to continue the histogram, NULL if it
+is in the top level.
 
 *tracefs_hist_reset* is called to clear / reset the histogram _hist_.
+The _instance_ is the instance to clear the histogram, NULL if it
+is in the top level.
 
 *tracefs_hist_destory* is called to delete the histogram where it will no longer
-exist.
+exist.  The _instance_ is the instance to delete the histogram from, NULL if it
+is in the top level.
 
 
 KEY TYPES
@@ -152,6 +162,7 @@ int main (int argc, char **argv, char **env)
 {
 	struct tracefs_instance *instance;
 	struct tracefs_hist *hist;
+	struct tep_handle *tep;
 	enum commands cmd;
 	char *cmd_str;
 	int ret;
@@ -186,7 +197,8 @@ int main (int argc, char **argv, char **env)
 		exit(-1);
 	}
 
-	hist = tracefs_hist_alloc(instance, "kmem", "kmalloc",
+	tep = tracefs_local_events(NULL);
+	hist = tracefs_hist_alloc(tep, "kmem", "kmalloc",
 				  "call_site", TRACEFS_HIST_KEY_SYM);
 	if (!hist) {
 		fprintf(stderr, "Failed hist create\n");
@@ -208,7 +220,7 @@ int main (int argc, char **argv, char **env)
 
 	switch (cmd) {
 	case START:
-		ret = tracefs_hist_start(hist);
+		ret = tracefs_hist_start(instance, hist);
 		if (ret) {
 			char *err = tracefs_error_last(instance);
 			if (err)
@@ -216,16 +228,16 @@ int main (int argc, char **argv, char **env)
 		}
 		break;
 	case PAUSE:
-		ret = tracefs_hist_pause(hist);
+		ret = tracefs_hist_pause(instance, hist);
 		break;
 	case CONT:
-		ret = tracefs_hist_continue(hist);
+		ret = tracefs_hist_continue(instance, hist);
 		break;
 	case RESET:
-		ret = tracefs_hist_reset(hist);
+		ret = tracefs_hist_reset(instance, hist);
 		break;
 	case DELETE:
-		ret = tracefs_hist_destroy(hist);
+		ret = tracefs_hist_destroy(instance, hist);
 		break;
 	case SHOW: {
 		char *content;
diff --git a/include/tracefs.h b/include/tracefs.h
index 246647f6496d..ff115d6ce01d 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -282,9 +282,9 @@ struct tracefs_hist;
 void tracefs_hist_free
 (struct tracefs_hist *hist);
 struct tracefs_hist *
-tracefs_hist_alloc(struct tracefs_instance * instance,
-		       const char *system, const char *event,
-		       const char *key, enum tracefs_hist_key_type type);
+tracefs_hist_alloc(struct tep_handle *tep,
+		   const char *system, const char *event,
+		   const char *key, enum tracefs_hist_key_type type);
 int tracefs_hist_add_key(struct tracefs_hist *hist, const char *key,
 			 enum tracefs_hist_key_type type);
 int tracefs_hist_add_value(struct tracefs_hist *hist, const char *value);
@@ -294,11 +294,11 @@ int tracefs_hist_sort_key_direction(struct tracefs_hist *hist,
 				    const char *sort_key,
 				    enum tracefs_hist_sort_direction dir);
 int tracefs_hist_add_name(struct tracefs_hist *hist, const char *name);
-int tracefs_hist_start(struct tracefs_hist *hist);
-int tracefs_hist_pause(struct tracefs_hist *hist);
-int tracefs_hist_continue(struct tracefs_hist *hist);
-int tracefs_hist_reset(struct tracefs_hist *hist);
-int tracefs_hist_destroy(struct tracefs_hist *hist);
+int tracefs_hist_start(struct tracefs_instance *instance, struct tracefs_hist *hist);
+int tracefs_hist_pause(struct tracefs_instance *instance,struct tracefs_hist *hist);
+int tracefs_hist_continue(struct tracefs_instance *instance,struct tracefs_hist *hist);
+int tracefs_hist_reset(struct tracefs_instance *instance,struct tracefs_hist *hist);
+int tracefs_hist_destroy(struct tracefs_instance *instance,struct tracefs_hist *hist);
 
 struct tracefs_synth;
 
diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
index 7292f89457c9..b72171af9577 100644
--- a/src/tracefs-hist.c
+++ b/src/tracefs-hist.c
@@ -23,9 +23,10 @@
 #define DESCENDING ".descending"
 
 struct tracefs_hist {
-	struct tracefs_instance *instance;
+	struct tep_handle	*tep;
+	struct tep_event	*event;
 	char			*system;
-	char			*event;
+	char			*event_name;
 	char			*name;
 	char			**keys;
 	char			**values;
@@ -65,15 +66,17 @@ static void add_list(struct trace_seq *seq, const char *start,
  * Returns 0 on succes -1 on error.
  */
 static int
-trace_hist_start(struct tracefs_hist *hist,
+trace_hist_start(struct tracefs_instance *instance, struct tracefs_hist *hist,
 		 enum tracefs_hist_command command)
 {
-	struct tracefs_instance *instance = hist->instance;
 	const char *system = hist->system;
-	const char *event = hist->event;
+	const char *event = hist->event_name;
 	struct trace_seq seq;
 	int ret;
 
+	if (!tracefs_event_file_exists(instance, system, event, HIST_FILE))
+		return -1;
+
 	errno = -EINVAL;
 	if (!hist->keys)
 		return -1;
@@ -129,9 +132,9 @@ void tracefs_hist_free(struct tracefs_hist *hist)
 	if (!hist)
 		return;
 
-	trace_put_instance(hist->instance);
+	tep_unref(hist->tep);
 	free(hist->system);
-	free(hist->event);
+	free(hist->event_name);
 	free(hist->name);
 	free(hist->filter);
 	tracefs_list_free(hist->keys);
@@ -142,9 +145,9 @@ void tracefs_hist_free(struct tracefs_hist *hist)
 
 /**
  * tracefs_hist_alloc - Initialize a histogram
- * @instance: The instance the histogram will be in (NULL for toplevel)
+ * @tep: The tep handle that has the @system and @event.
  * @system: The system the histogram event is in.
- * @event: The event that the histogram will be attached to.
+ * @event_name: The name of the event that the histogram will be attached to.
  * @key: The primary key the histogram will use
  * @type: The format type of the key.
  *
@@ -157,33 +160,31 @@ void tracefs_hist_free(struct tracefs_hist *hist)
  * NULL on failure.
  */
 struct tracefs_hist *
-tracefs_hist_alloc(struct tracefs_instance * instance,
-			const char *system, const char *event,
+tracefs_hist_alloc(struct tep_handle *tep,
+			const char *system, const char *event_name,
 			const char *key, enum tracefs_hist_key_type type)
 {
+	struct tep_event *event;
 	struct tracefs_hist *hist;
 	int ret;
 
-	if (!system || !event || !key)
+	if (!system || !event_name || !key)
 		return NULL;
 
-	if (!tracefs_event_file_exists(instance, system, event, HIST_FILE))
+	event = tep_find_event_by_name(tep, system, event_name);
+	if (!event)
 		return NULL;
 
 	hist = calloc(1, sizeof(*hist));
 	if (!hist)
 		return NULL;
 
-	ret = trace_get_instance(instance);
-	if (ret < 0) {
-		free(hist);
-		return NULL;
-	}
-
-	hist->instance = instance;
+	tep_ref(tep);
+	hist->tep = tep;
 
+	hist->event = event;
 	hist->system = strdup(system);
-	hist->event = strdup(event);
+	hist->event_name = strdup(event_name);
 
 	ret = tracefs_hist_add_key(hist, key, type);
 
@@ -302,58 +303,63 @@ int tracefs_hist_add_name(struct tracefs_hist *hist, const char *name)
 
 /**
  * tracefs_hist_start - enable a histogram
+ * @instance: The instance the histogram will be in (NULL for toplevel)
  * @hist: The histogram to start
  *
  * Starts executing a histogram.
  *
  * Returns 0 on success, -1 on error.
  */
-int tracefs_hist_start(struct tracefs_hist *hist)
+int tracefs_hist_start(struct tracefs_instance *instance, struct tracefs_hist *hist)
 {
-	return trace_hist_start(hist, 0);
+	return trace_hist_start(instance, hist, 0);
 }
 
 /**
  * tracefs_hist_pause - pause a histogram
+ * @instance: The instance the histogram is in (NULL for toplevel)
  * @hist: The histogram to pause
  *
  * Pause a histogram.
  *
  * Returns 0 on success, -1 on error.
  */
-int tracefs_hist_pause(struct tracefs_hist *hist)
+int tracefs_hist_pause(struct tracefs_instance *instance, struct tracefs_hist *hist)
 {
-	return trace_hist_start(hist, HIST_CMD_PAUSE);
+	return trace_hist_start(instance, hist, HIST_CMD_PAUSE);
 }
 
 /**
  * tracefs_hist_continue - continue a paused histogram
+ * @instance: The instance the histogram is in (NULL for toplevel)
  * @hist: The histogram to continue
  *
  * Continue a histogram.
  *
  * Returns 0 on success, -1 on error.
  */
-int tracefs_hist_continue(struct tracefs_hist *hist)
+int tracefs_hist_continue(struct tracefs_instance *instance, struct tracefs_hist *hist)
 {
-	return trace_hist_start(hist, HIST_CMD_CONT);
+	return trace_hist_start(instance, hist, HIST_CMD_CONT);
 }
 
 /**
  * tracefs_hist_reset - clear a histogram
+ * @instance: The instance the histogram is in (NULL for toplevel)
  * @hist: The histogram to reset
  *
  * Resets a histogram.
  *
  * Returns 0 on success, -1 on error.
  */
-int tracefs_hist_reset(struct tracefs_hist *hist)
+int tracefs_hist_reset(struct tracefs_instance *instance, struct tracefs_hist *hist)
 {
-	return trace_hist_start(hist, HIST_CMD_CLEAR);
+	return trace_hist_start(instance, hist, HIST_CMD_CLEAR);
 }
 
 /**
  * tracefs_hist_destroy - deletes a histogram (needs to be enabled again)
+ * @instance: The instance the histogram is in (NULL for toplevel)
  * @hist: The histogram to delete
  *
  * Deletes (removes) a running histogram. This is different than
@@ -363,9 +369,9 @@ int tracefs_hist_reset(struct tracefs_hist *hist)
  *
  * Returns 0 on success, -1 on error.
  */
-int tracefs_hist_destroy(struct tracefs_hist *hist)
+int tracefs_hist_destroy(struct tracefs_instance *instance, struct tracefs_hist *hist)
 {
-	return trace_hist_start(hist, HIST_CMD_DESTROY);
+	return trace_hist_start(instance, hist, HIST_CMD_DESTROY);
 }
 
 static char **
-- 
2.30.2


  reply	other threads:[~2021-08-03 16:48 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-03 16:48 [PATCH v2 0/7] libtracefs: Updates to the histograms for tracefs_sql() Steven Rostedt
2021-08-03 16:48 ` Steven Rostedt [this message]
2021-08-03 16:48 ` [PATCH v2 2/7] libtracefs: Expose tracefs_hist_command() as an API Steven Rostedt
2021-08-03 16:48 ` [PATCH v2 3/7] libtracefs: Add API tracefs_hist_append_filter() Steven Rostedt
2021-08-03 16:48 ` [PATCH v2 4/7] libtracefs: Add API tracefs_hist_show() Steven Rostedt
2021-08-03 16:48 ` [PATCH v2 5/7] libtracefs: Split up libtracefs-synth man page Steven Rostedt
2021-08-03 16:48 ` [PATCH v2 6/7] libtracefs: Add API tracefs_synth_get_start_hist() Steven Rostedt
2021-08-03 16:48 ` [PATCH v2 7/7] libtracefs: Add API tracefs_synth_complete() Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210803164811.693731-2-rostedt@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=linux-trace-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.