linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] libtracefs: Work to add histogram APIs
@ 2021-07-07  3:11 Steven Rostedt
  2021-07-07  3:11 ` [PATCH 1/4] libtracefs: Implement tracefs_list_size() Steven Rostedt
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Steven Rostedt @ 2021-07-07  3:11 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (VMware)

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

The first three patches are helper functions to help with
the final patch, although the second patch can probably stand on
its own, as it simplifies access to the files in the system/event/
directory, instead of having to build it first and then call
tracefs_instance_file_*().

Steven Rostedt (VMware) (4):
  libtracefs: Implement tracefs_list_size()
  libtracefs: Implement functions to work on event directory files
  libtracefs: Have instances have internal ref counting
  libtracefs: Implement API to create / modify and display histograms

 Documentation/libtracefs-hist-cont.txt | 192 +++++++++
 Documentation/libtracefs-hist.txt      | 290 ++++++++++++++
 include/tracefs-local.h                |   4 +
 include/tracefs.h                      |  66 +++
 src/Makefile                           |   1 +
 src/tracefs-events.c                   | 182 +++++++++
 src/tracefs-hist.c                     | 529 +++++++++++++++++++++++++
 src/tracefs-instance.c                 |  56 ++-
 src/tracefs-utils.c                    |  16 +
 9 files changed, 1327 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/libtracefs-hist-cont.txt
 create mode 100644 Documentation/libtracefs-hist.txt
 create mode 100644 src/tracefs-hist.c

-- 
2.30.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] libtracefs: Implement tracefs_list_size()
  2021-07-07  3:11 [PATCH 0/4] libtracefs: Work to add histogram APIs Steven Rostedt
@ 2021-07-07  3:11 ` Steven Rostedt
  2021-07-07  3:11 ` [PATCH 2/4] libtracefs: Implement functions to work on event directory files Steven Rostedt
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Steven Rostedt @ 2021-07-07  3:11 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (VMware)

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

Add tracefs_list_size() that returns a the size of a list created by
tracefs_list_add().

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 include/tracefs.h   |  1 +
 src/tracefs-utils.c | 16 ++++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/include/tracefs.h b/include/tracefs.h
index 266c513e4116..6b9a76c4c40a 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -65,6 +65,7 @@ int tracefs_error_clear(struct tracefs_instance *instance);
 
 void tracefs_list_free(char **list);
 char **tracefs_list_add(char **list, const char *string);
+int tracefs_list_size(char **list);
 
 /**
  * tracefs_trace_on_get_fd - Get a file descriptor of "tracing_on" in given instance
diff --git a/src/tracefs-utils.c b/src/tracefs-utils.c
index ec5ecb08ed2e..6750336c9ef6 100644
--- a/src/tracefs-utils.c
+++ b/src/tracefs-utils.c
@@ -447,3 +447,19 @@ char **tracefs_list_add(char **list, const char *string)
 
 	return list;
 }
+
+/**
+ * tracefs_list_size - Return the number of strings in the list
+ * @list: The list to determine the size.
+ *
+ * Returns the number of elements in the list.
+ * If @list is NULL, then zero is returned.
+ */
+int tracefs_list_size(char **list)
+{
+	if (!list)
+		return 0;
+
+	list--;
+	return (int)*(unsigned long *)list;
+}
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] libtracefs: Implement functions to work on event directory files
  2021-07-07  3:11 [PATCH 0/4] libtracefs: Work to add histogram APIs Steven Rostedt
  2021-07-07  3:11 ` [PATCH 1/4] libtracefs: Implement tracefs_list_size() Steven Rostedt
@ 2021-07-07  3:11 ` Steven Rostedt
  2021-07-07  3:11 ` [PATCH 3/4] libtracefs: Have instances have internal ref counting Steven Rostedt
  2021-07-07  3:11 ` [PATCH 4/4] libtracefs: Implement API to create / modify and display histograms Steven Rostedt
  3 siblings, 0 replies; 9+ messages in thread
From: Steven Rostedt @ 2021-07-07  3:11 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (VMware)

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

Add the following functions:

 tracefs_event_get_file() to easily put together a path to a file in the
            event directory.

 tracefs_event_file_read() to easily read the content from a file in the
            event directory.

 tracefs_event_file_write() to easily write content to a file in the
            event directory.

 tracefs_event_file_append() to easily append content to a file in the
            event directory.

 tracefs_event_file_clear() to easily clear the content of a file in the
            event directory.

 tracefs_event_file_exists() to easily know if a file exists in the
            event directory.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 include/tracefs.h    |  19 +++++
 src/tracefs-events.c | 182 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 201 insertions(+)

diff --git a/include/tracefs.h b/include/tracefs.h
index 6b9a76c4c40a..afbfd4eb01d6 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -102,6 +102,25 @@ int tracefs_iterate_raw_events(struct tep_handle *tep,
 				void *callback_context);
 void tracefs_iterate_stop(struct tracefs_instance *instance);
 
+char *tracefs_event_get_file(struct tracefs_instance *instance,
+			     const char *system, const char *event,
+			     const char *file);
+char *tracefs_event_file_read(struct tracefs_instance *instance,
+			      const char *system, const char *event,
+			      const char *file, int *psize);
+int tracefs_event_file_write(struct tracefs_instance *instance,
+			     const char *system, const char *event,
+			     const char *file, const char *str);
+int tracefs_event_file_append(struct tracefs_instance *instance,
+			      const char *system, const char *event,
+			      const char *file, const char *str);
+int tracefs_event_file_clear(struct tracefs_instance *instance,
+			     const char *system, const char *event,
+			     const char *file);
+bool tracefs_event_file_exists(struct tracefs_instance *instance,
+			       const char *system, const char *event,
+			       const char *file);
+
 char **tracefs_tracers(const char *tracing_dir);
 
 struct tep_handle *tracefs_local_events(const char *tracing_dir);
diff --git a/src/tracefs-events.c b/src/tracefs-events.c
index 4bfd8e451ef5..7febc2a0f24d 100644
--- a/src/tracefs-events.c
+++ b/src/tracefs-events.c
@@ -302,6 +302,188 @@ __hidden char *trace_append_file(const char *dir, const char *name)
 	return ret < 0 ? NULL : file;
 }
 
+static int event_file(char **path, const char *system,
+		      const char *event, const char *file)
+{
+	if (!system || !event || !file)
+		return -1;
+
+	return asprintf(path, "events/%s/%s/%s",
+			system, event, file);
+}
+
+/**
+ * tracefs_event_get_file - return a file in an event directory
+ * @instance: The instance the event is in (NULL for top level)
+ * @system: The system name that the event file is in
+ * @event: The event name of the event
+ * @file: The name of the file in the event directory.
+ *
+ * Returns a path to a file in the event director.
+ * or NULL on error. The path returned must be freed with
+ * tracefs_put_tracing_file().
+ */
+char *tracefs_event_get_file(struct tracefs_instance *instance,
+			     const char *system, const char *event,
+			     const char *file)
+{
+	char *instance_path;
+	char *path;
+	int ret;
+
+	ret = event_file(&path, system, event, file);
+	if (ret < 0)
+		return NULL;
+
+	instance_path = tracefs_instance_get_file(instance, path);
+	free(path);
+
+	return instance_path;
+}
+
+/**
+ * tracefs_event_file_read - read the content from an event file
+ * @instance: The instance the event is in (NULL for top level)
+ * @system: The system name that the event file is in
+ * @event: The event name of the event
+ * @file: The name of the file in the event directory.
+ * @psize: the size of the content read.
+ *
+ * Reads the content of the event file that is passed via the
+ * arguments and returns the content.
+ *
+ * Return a string containing the content of the file or NULL
+ * on error. The string returned must be freed with free().
+ */
+char *tracefs_event_file_read(struct tracefs_instance *instance,
+			      const char *system, const char *event,
+			      const char *file, int *psize)
+{
+	char *content;
+	char *path;
+	int ret;
+
+	ret = event_file(&path, system, event, file);
+	if (ret < 0)
+		return NULL;
+
+	content = tracefs_instance_file_read(instance, path, psize);
+	free(path);
+	return content;
+}
+
+/**
+ * tracefs_event_file_write - write to an event file
+ * @instance: The instance the event is in (NULL for top level)
+ * @system: The system name that the event file is in
+ * @event: The event name of the event
+ * @file: The name of the file in the event directory.
+ * @str: The string to write into the file
+ *
+ * Writes the content of @str to a file in the instance directory.
+ * The content of the file will be overwritten by @str.
+ *
+ * Return 0 on success, and -1 on error.
+ */
+int tracefs_event_file_write(struct tracefs_instance *instance,
+			     const char *system, const char *event,
+			     const char *file, const char *str)
+{
+	char *path;
+	int ret;
+
+	ret = event_file(&path, system, event, file);
+	if (ret < 0)
+		return -1;
+
+	ret = tracefs_instance_file_write(instance, path, str);
+	free(path);
+	return ret;
+}
+
+/**
+ * tracefs_event_file_append - write to an event file
+ * @instance: The instance the event is in (NULL for top level)
+ * @system: The system name that the event file is in
+ * @event: The event name of the event
+ * @file: The name of the file in the event directory.
+ * @str: The string to write into the file
+ *
+ * Writes the content of @str to a file in the instance directory.
+ * The content of @str will be appended to the content of the file.
+ * The current content should not be lost.
+ *
+ * Return 0 on success, and -1 on error.
+ */
+int tracefs_event_file_append(struct tracefs_instance *instance,
+			      const char *system, const char *event,
+			      const char *file, const char *str)
+{
+	char *path;
+	int ret;
+
+	ret = event_file(&path, system, event, file);
+	if (ret < 0)
+		return -1;
+
+	ret = tracefs_instance_file_append(instance, path, str);
+	free(path);
+	return ret;
+}
+
+/**
+ * tracefs_event_file_clear - clear an event file
+ * @instance: The instance the event is in (NULL for top level)
+ * @system: The system name that the event file is in
+ * @event: The event name of the event
+ * @file: The name of the file in the event directory.
+ *
+ * Clears the content of the event file. That is, it is opened
+ * with O_TRUNC and then closed.
+ *
+ * Return 0 on success, and -1 on error.
+ */
+int tracefs_event_file_clear(struct tracefs_instance *instance,
+			     const char *system, const char *event,
+			     const char *file)
+{
+	char *path;
+	int ret;
+
+	ret = event_file(&path, system, event, file);
+	if (ret < 0)
+		return -1;
+
+	ret = tracefs_instance_file_clear(instance, path);
+	free(path);
+	return ret;
+}
+
+/**
+ * tracefs_event_file_exits - test if a file exists
+ * @instance: The instance the event is in (NULL for top level)
+ * @system: The system name that the event file is in
+ * @event: The event name of the event
+ * @file: The name of the file in the event directory.
+ *
+ * Return true if the file exists, false if it odes not or
+ * an error occurred.
+ */
+bool tracefs_event_file_exists(struct tracefs_instance *instance,
+			       const char *system, const char *event,
+			       const char *file)
+{
+	char *path;
+	bool ret;
+
+	if (event_file(&path, system, event, file) < 0)
+		return false;
+
+	ret = tracefs_file_exists(instance, path);
+	free(path);
+	return ret;
+}
+
 /**
  * tracefs_event_systems - return list of systems for tracing
  * @tracing_dir: directory holding the "events" directory
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] libtracefs: Have instances have internal ref counting
  2021-07-07  3:11 [PATCH 0/4] libtracefs: Work to add histogram APIs Steven Rostedt
  2021-07-07  3:11 ` [PATCH 1/4] libtracefs: Implement tracefs_list_size() Steven Rostedt
  2021-07-07  3:11 ` [PATCH 2/4] libtracefs: Implement functions to work on event directory files Steven Rostedt
@ 2021-07-07  3:11 ` Steven Rostedt
  2021-07-07  3:11 ` [PATCH 4/4] libtracefs: Implement API to create / modify and display histograms Steven Rostedt
  3 siblings, 0 replies; 9+ messages in thread
From: Steven Rostedt @ 2021-07-07  3:11 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (VMware)

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

Add a way to keep ref counts on accesses to instances. This way, another
structure can hold a reference to an instance, and if the user frees that
instance, it wont be freed until the other structure frees it.

That is, if we create a structure called tracefs_hist, that takes an
instance on its creation. It can hold a reference to that instance, and
make sure that it does not get freed:

  instance = tracefs_instance_create("my_instance");
  hist = tracefs_hist_alloc(instance, "sched", "sched_wakeup", "pid", 0);

  tracefs_instance_free(instance);

  tracefs_hist_start(hist);
  [..]
  tracefs_hist_free(hist);

Will not crash, as the tracefs_instance_free() will not free the instance
as there's still a reference on it, and tracefs_hist_start() will still
have that reference. But then when the tracefs_hist_free() is called, then
the instance will get freed.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 include/tracefs-local.h |  4 +++
 src/tracefs-instance.c  | 56 ++++++++++++++++++++++++++++++++++-------
 2 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/include/tracefs-local.h b/include/tracefs-local.h
index 512f17a7c812..2324dec9d076 100644
--- a/include/tracefs-local.h
+++ b/include/tracefs-local.h
@@ -25,6 +25,7 @@ struct tracefs_instance {
 	char				*trace_dir;
 	char				*name;
 	pthread_mutex_t			lock;
+	int				ref;
 	int				flags;
 	int				ftrace_filter_fd;
 	int				ftrace_notrace_fd;
@@ -41,6 +42,9 @@ static inline pthread_mutex_t *trace_get_lock(struct tracefs_instance *instance)
 	return instance ? &instance->lock : &toplevel_lock;
 }
 
+void trace_put_instance(struct tracefs_instance *instance);
+int trace_get_instance(struct tracefs_instance *instance);
+
 /* Can be overridden */
 void tracefs_warning(const char *fmt, ...);
 
diff --git a/src/tracefs-instance.c b/src/tracefs-instance.c
index 11fb580456ff..185d968ae319 100644
--- a/src/tracefs-instance.c
+++ b/src/tracefs-instance.c
@@ -20,7 +20,11 @@
 #include "tracefs.h"
 #include "tracefs-local.h"
 
-#define FLAG_INSTANCE_NEWLY_CREATED	(1 << 0)
+enum {
+	FLAG_INSTANCE_NEWLY_CREATED	= (1 << 0),
+	FLAG_INSTANCE_DELETED		= (1 << 1),
+};
+
 
 struct tracefs_options_mask	toplevel_supported_opts;
 struct tracefs_options_mask	toplevel_enabled_opts;
@@ -79,15 +83,30 @@ error:
 	return NULL;
 }
 
-/**
- * tracefs_instance_free - Free an instance, previously allocated by
-			   tracefs_instance_create()
- * @instance: Pointer to the instance to be freed
- *
- */
-void tracefs_instance_free(struct tracefs_instance *instance)
+
+__hidden int trace_get_instance(struct tracefs_instance *instance)
 {
-	if (!instance)
+	int ret;
+
+	pthread_mutex_lock(&instance->lock);
+	if (instance->flags & FLAG_INSTANCE_DELETED) {
+		ret = -1;
+	} else {
+		instance->ref++;
+		ret = 0;
+	}
+	pthread_mutex_unlock(&instance->lock);
+	return ret;
+}
+
+__hidden void trace_put_instance(struct tracefs_instance *instance)
+{
+	pthread_mutex_lock(&instance->lock);
+	if (--instance->ref < 0)
+		instance->flags |= FLAG_INSTANCE_DELETED;
+	pthread_mutex_unlock(&instance->lock);
+
+	if (!(instance->flags & FLAG_INSTANCE_DELETED))
 		return;
 
 	if (instance->ftrace_filter_fd >= 0)
@@ -108,6 +127,20 @@ void tracefs_instance_free(struct tracefs_instance *instance)
 	free(instance);
 }
 
+/**
+ * tracefs_instance_free - Free an instance, previously allocated by
+			   tracefs_instance_create()
+ * @instance: Pointer to the instance to be freed
+ *
+ */
+void tracefs_instance_free(struct tracefs_instance *instance)
+{
+	if (!instance)
+		return;
+
+	trace_put_instance(instance);
+}
+
 static mode_t get_trace_file_permissions(char *name)
 {
 	mode_t rmode = 0;
@@ -248,6 +281,11 @@ int tracefs_instance_destroy(struct tracefs_instance *instance)
 	if (path)
 		ret = rmdir(path);
 	tracefs_put_tracing_file(path);
+	if (ret) {
+		pthread_mutex_lock(&instance->lock);
+		instance->flags |= FLAG_INSTANCE_DELETED;
+		pthread_mutex_unlock(&instance->lock);
+	}
 
 	return ret;
 }
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] libtracefs: Implement API to create / modify and display histograms
  2021-07-07  3:11 [PATCH 0/4] libtracefs: Work to add histogram APIs Steven Rostedt
                   ` (2 preceding siblings ...)
  2021-07-07  3:11 ` [PATCH 3/4] libtracefs: Have instances have internal ref counting Steven Rostedt
@ 2021-07-07  3:11 ` Steven Rostedt
  2021-07-07  8:12   ` Yordan Karadzhov (VMware)
  3 siblings, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2021-07-07  3:11 UTC (permalink / raw)
  To: linux-trace-devel
  Cc: Steven Rostedt (VMware),
	Tom Zanussi, Masami Hiramatsu, Namhyung Kim,
	Daniel Bristot de Oliveira

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

Add an API to facilitate the modification of histograms in ftrace:

  tracefs_hist_alloc()
  tracefs_hist_add_key()
  tracefs_hist_add_value()
  tracefs_hist_add_name()
  tracefs_hist_start()
  tracefs_hist_pause()
  tracefs_hist_continue()
  tracefs_hist_reset()
  tracefs_hist_delete()
  tracefs_hist_add_sort_key()
  tracefs_hist_sort_key_direction()

The above functions can create a tracefs_hist descriptor that can
facilitate the creation and modification of ftrace event histograms.

Cc: Tom Zanussi <zanussi@kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/libtracefs-hist-cont.txt | 192 +++++++++
 Documentation/libtracefs-hist.txt      | 290 ++++++++++++++
 include/tracefs.h                      |  46 +++
 src/Makefile                           |   1 +
 src/tracefs-hist.c                     | 529 +++++++++++++++++++++++++
 5 files changed, 1058 insertions(+)
 create mode 100644 Documentation/libtracefs-hist-cont.txt
 create mode 100644 Documentation/libtracefs-hist.txt
 create mode 100644 src/tracefs-hist.c

diff --git a/Documentation/libtracefs-hist-cont.txt b/Documentation/libtracefs-hist-cont.txt
new file mode 100644
index 000000000000..1b0153c19481
--- /dev/null
+++ b/Documentation/libtracefs-hist-cont.txt
@@ -0,0 +1,192 @@
+libtracefs(3)
+=============
+
+NAME
+----
+tracefs_hist_pause, tracefs_hist_continue, tracefs_hist_reset - Pause, continue, or clear an existing histogram
+
+SYNOPSIS
+--------
+[verse]
+--
+*#include <tracefs.h>*
+
+int tracefs_hist_pause(struct tracefs_hist pass:[*]hist);
+int tracefs_hist_continue(struct tracefs_hist pass:[*]hist);
+int tracefs_hist_reset(struct tracefs_hist pass:[*]hist);
+--
+
+DESCRIPTION
+-----------
+*tracefs_hist_pause* is called to pause the histogram _hist_.
+
+*tracefs_hist_continue* is called to continue a paused histogram _hist_.
+
+*tracefs_hist_reset* is called to clear / reset the histogram _hist_.
+
+RETURN VALUE
+------------
+All the return zero on success or -1 on error.
+
+EXAMPLE
+-------
+[source,c]
+--
+#include <stdlib.h>
+#include <tracefs/tracefs.h>
+
+enum commands {
+	START,
+	PAUSE,
+	CONT,
+	RESET,
+	DELETE,
+	SHOW,
+};
+
+int main (int argc, char **argv, char **env)
+{
+	struct tracefs_instance *instance;
+	struct tracefs_hist *hist;
+	enum commands cmd;
+	char *cmd_str;
+	int ret;
+
+	if (argc < 2) {
+		fprintf(stderr, "usage: %s command\n", argv[0]);
+		exit(-1);
+	}
+
+	cmd_str = argv[1];
+
+	if (!strcmp(cmd_str, "start"))
+		cmd = START;
+	else if (!strcmp(cmd_str, "pause"))
+		cmd = PAUSE;
+	else if (!strcmp(cmd_str, "cont"))
+		cmd = CONT;
+	else if (!strcmp(cmd_str, "reset"))
+		cmd = RESET;
+	else if (!strcmp(cmd_str, "delete"))
+		cmd = DELETE;
+	else if (!strcmp(cmd_str, "show"))
+		cmd = SHOW;
+	else {
+		fprintf(stderr, "Unknown command %s\n", cmd_str);
+		exit(-1);
+	}
+
+	instance = tracefs_instance_create("hist_test");
+	if (!instance) {
+		fprintf(stderr, "Failed instance create\n");
+		exit(-1);
+	}
+
+	hist = tracefs_hist_alloc(instance, "kmem", "kmalloc",
+				  "call_site", TRACEFS_HIST_KEY_SYM);
+	if (!hist) {
+		fprintf(stderr, "Failed hist create\n");
+		exit(-1);
+	}
+
+	ret = tracefs_hist_add_key(hist, "bytes_req", 0);
+	ret |= tracefs_hist_add_value(hist, "bytes_alloc");
+	ret |= tracefs_hist_add_sort_key(hist, "bytes_req", "bytes_alloc", NULL);
+
+	ret |= tracefs_hist_sort_key_direction(hist, "bytes_alloc",
+					       TRACEFS_HIST_SORT_DESCENDING);
+	if (ret) {
+		fprintf(stderr, "Failed modifying histogram\n");
+		exit(-1);
+	}
+
+	tracefs_error_clear(instance);
+
+	switch (cmd) {
+	case START:
+		ret = tracefs_hist_start(hist);
+		if (ret) {
+			char *err = tracefs_error_last(instance);
+			if (err)
+				fprintf(stderr, "\n%s\n", err);
+		}
+		break;
+	case PAUSE:
+		ret = tracefs_hist_pause(hist);
+		break;
+	case CONT:
+		ret = tracefs_hist_continue(hist);
+		break;
+	case RESET:
+		ret = tracefs_hist_reset(hist);
+		break;
+	case DELETE:
+		ret = tracefs_hist_destroy(hist);
+		break;
+	case SHOW: {
+		char *content;
+		content = tracefs_event_file_read(instance, "kmem", "kmalloc",
+						  "hist", NULL);
+		ret = content ? 0 : -1;
+		if (content) {
+			printf("%s\n", content);
+			free(content);
+		}
+		break;
+	}
+	}
+	if (ret)
+		fprintf(stderr, "Failed: command\n");
+	exit(ret);
+}
+--
+
+FILES
+-----
+[verse]
+--
+*tracefs.h*
+	Header file to include in order to have access to the library APIs.
+*-ltracefs*
+	Linker switch to add when building a program that uses the library.
+--
+
+SEE ALSO
+--------
+_libtracefs(3)_,
+_libtraceevent(3)_,
+_trace-cmd(1)_,
+_tracefs_hist_alloc(3)_,
+_tracefs_hist_free(3)_,
+_tracefs_hist_add_key(3)_,
+_tracefs_hist_add_value(3)_,
+_tracefs_hist_add_name(3)_,
+_tracefs_hist_start(3)_,
+_tracefs_hist_destory(3)_,
+_tracefs_hist_add_sort_key(3)_,
+_tracefs_hist_sort_key_direction(3)_
+
+AUTHOR
+------
+[verse]
+--
+*Steven Rostedt* <rostedt@goodmis.org>
+*Tzvetomir Stoyanov* <tz.stoyanov@gmail.com>
+*sameeruddin shaik* <sameeruddin.shaik8@gmail.com>
+--
+REPORTING BUGS
+--------------
+Report bugs to  <linux-trace-devel@vger.kernel.org>
+
+LICENSE
+-------
+libtracefs is Free Software licensed under the GNU LGPL 2.1
+
+RESOURCES
+---------
+https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/
+
+COPYING
+-------
+Copyright \(C) 2020 VMware, Inc. Free use of this software is granted under
+the terms of the GNU Public License (GPL).
diff --git a/Documentation/libtracefs-hist.txt b/Documentation/libtracefs-hist.txt
new file mode 100644
index 000000000000..67b2a068ac45
--- /dev/null
+++ b/Documentation/libtracefs-hist.txt
@@ -0,0 +1,290 @@
+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_sort_key_direction - Create and update event histograms
+
+SYNOPSIS
+--------
+[verse]
+--
+*#include <tracefs.h>*
+
+struct tracefs_hist pass:[*]tracefs_hist_alloc(struct tracefs_instance pass:[*] instance,
+			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);
+int tracefs_hist_add_key(struct tracefs_hist pass:[*]hist, const char pass:[*]key,
+			 enum tracefs_hist_key_type type);
+int tracefs_hist_add_value(struct tracefs_hist pass:[*]hist, const char pass:[*]value);
+int tracefs_hist_add_sort_key(struct tracefs_hist pass:[*]hist,
+			      const char pass:[*]sort_key, ...);
+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);
+--
+
+DESCRIPTION
+-----------
+Event histograms are created by the trigger file in the event directory.
+The syntax can be complex and difficult to get correct. This API handles the
+syntax, and facilitates the creation and interaction with the event histograms.
+See https://www.kernel.org/doc/html/latest/trace/histogram.html for more information.
+
+*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.
+
+*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
+to be called to do so.
+
+*tracefs_hist_add_key*() Adds a secondary or tertiary key to the histogram.
+The key passed to *tracefs_hist_alloc*() is the primary key of the histogram.
+The first time this function is called, it will add a secondary key (or two dimensional
+histogram). If this function is called again on the same histogram, it will add
+a _tertiary_ key (or three dimensional histogram). The _hist_ parameter is the
+histrogram descriptor to add the _key_ to. The _type_ is the type of key to add
+(See KEY TYPES below).
+
+*tracefs_hist_add_value*() will add a value to record. By default, the value is
+simply the "hitcount" of the number of times a instance of the histogram's
+key was hit. The _hist_ is the histogram descriptor to add the value to.
+The _value_ is a field in the histogram to add to when an instane of the
+key is hit.
+
+*tracefs_hist_add_sort_key*() will add a key to sort on. Th _hist_ is the
+the histrogram descriptor to add the sort key to. The _sort_key_ is a string
+that must match either an already defined key of the histogram, or an already
+defined value. Multiple sort keys may be added to denote a secondary, sort order
+and so on, but all sort keys must match an existing key or value, or be
+TRACEFS_HIST_HITCOUNT. The last parameter of *tracefs_hist_add_sort_key*() must
+be NULL.
+
+*tracefs_hist_sort_key_direction*() allows to change the direction of an
+existing sort key of _hist_. The _sort_key_ is the sort key to change, and
+_dir_ can be either TRACEFS_HIST_SORT_ASCENDING or TRACEFS_HIST_SORT_DESCENDING,
+to make the direction of the sort key either ascending or descending respectively.
+
+*tracefs_hist_add_name*() adds a name to a histogram. A histogram may be
+named and if the name matches between more than one event, and they have
+compatible keys, the multiple histograms with the same name will be merged
+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_.
+
+*tracefs_hist_pause* is called to pause the histogram _hist_.
+
+*tracefs_hist_continue* is called to continue a paused histogram _hist_.
+
+*tracefs_hist_reset* is called to clear / reset the histogram _hist_.
+
+*tracefs_hist_destory* is called to delete the histogram where it will no longer
+exist.
+
+
+KEY TYPES
+---------
+
+*tracefs_hist_alloc*() and *tracefs_hist_add_key*() both add a key and requires
+that key to have a type. The types may be:
+
+ *TRACEFS_HIST_KEY_NORMAL* or zero (0) which is to not modify the type.
+
+ *TRACEFS_HIST_KEY_HEX* to display the key in hex.
+
+ *TRACEFS_HIST_KEY_SYM* to display the key as a kernel symbol (if found). If
+the key is an address, this is useful as it will display the function names instead
+of just a number.
+
+ *TRACEFS_HIST_KEY_SYM_OFFSET* similar to *TRACEFS_HIST_KEY_SYM* but will also include
+the offset of the function to match the exact address.
+
+*TRACEFS_HIST_KEY_SYSCALL* If the key is a system call "id" (the number passed from user
+space to the kernel to tell it what system call it is calling), then the name of
+the system call is displayed.
+
+*TRACEFS_HIST_KEY_EXECNAME* If "common_pid" is the key (the pid of the executing task), 
+instead of showing the number, show the name of the running task.
+
+*TRACEFS_HIST_KEY_LOG* will display the key in a binary logarithmic scale.
+
+*TRACEFS_HIST_KEY_USECS* for use with "common_timestamp" or TRACEFS_HIST_TIMESTAMP,
+in which case it will show the timestamp in microseconds instead of nanoseconds.
+
+
+RETURN VALUE
+------------
+*tracefs_hist_alloc*() returns an allocated histogram descriptor which must
+be freed by *tracefs_hist_free*() or NULL on error.
+
+All the other functions return zero on success or -1 on error.
+
+If *tracefs_hist_start*() returns an error, a message may be displayed
+in the kernel that can be retrieved by *tracefs_error_last()*.
+
+EXAMPLE
+-------
+[source,c]
+--
+#include <stdlib.h>
+#include <tracefs/tracefs.h>
+
+enum commands {
+	START,
+	PAUSE,
+	CONT,
+	RESET,
+	DELETE,
+	SHOW,
+};
+
+int main (int argc, char **argv, char **env)
+{
+	struct tracefs_instance *instance;
+	struct tracefs_hist *hist;
+	enum commands cmd;
+	char *cmd_str;
+	int ret;
+
+	if (argc < 2) {
+		fprintf(stderr, "usage: %s command\n", argv[0]);
+		exit(-1);
+	}
+
+	cmd_str = argv[1];
+
+	if (!strcmp(cmd_str, "start"))
+		cmd = START;
+	else if (!strcmp(cmd_str, "pause"))
+		cmd = PAUSE;
+	else if (!strcmp(cmd_str, "cont"))
+		cmd = CONT;
+	else if (!strcmp(cmd_str, "reset"))
+		cmd = RESET;
+	else if (!strcmp(cmd_str, "delete"))
+		cmd = DELETE;
+	else if (!strcmp(cmd_str, "show"))
+		cmd = SHOW;
+	else {
+		fprintf(stderr, "Unknown command %s\n", cmd_str);
+		exit(-1);
+	}
+
+	instance = tracefs_instance_create("hist_test");
+	if (!instance) {
+		fprintf(stderr, "Failed instance create\n");
+		exit(-1);
+	}
+
+	hist = tracefs_hist_alloc(instance, "kmem", "kmalloc",
+				  "call_site", TRACEFS_HIST_KEY_SYM);
+	if (!hist) {
+		fprintf(stderr, "Failed hist create\n");
+		exit(-1);
+	}
+
+	ret = tracefs_hist_add_key(hist, "bytes_req", 0);
+	ret |= tracefs_hist_add_value(hist, "bytes_alloc");
+	ret |= tracefs_hist_add_sort_key(hist, "bytes_req", "bytes_alloc", NULL);
+
+	ret |= tracefs_hist_sort_key_direction(hist, "bytes_alloc",
+					       TRACEFS_HIST_SORT_DESCENDING);
+	if (ret) {
+		fprintf(stderr, "Failed modifying histogram\n");
+		exit(-1);
+	}
+
+	tracefs_error_clear(instance);
+
+	switch (cmd) {
+	case START:
+		ret = tracefs_hist_start(hist);
+		if (ret) {
+			char *err = tracefs_error_last(instance);
+			if (err)
+				fprintf(stderr, "\n%s\n", err);
+		}
+		break;
+	case PAUSE:
+		ret = tracefs_hist_pause(hist);
+		break;
+	case CONT:
+		ret = tracefs_hist_continue(hist);
+		break;
+	case RESET:
+		ret = tracefs_hist_reset(hist);
+		break;
+	case DELETE:
+		ret = tracefs_hist_destroy(hist);
+		break;
+	case SHOW: {
+		char *content;
+		content = tracefs_event_file_read(instance, "kmem", "kmalloc",
+						  "hist", NULL);
+		ret = content ? 0 : -1;
+		if (content) {
+			printf("%s\n", content);
+			free(content);
+		}
+		break;
+	}
+	}
+	if (ret)
+		fprintf(stderr, "Failed: command\n");
+	exit(ret);
+}
+--
+
+FILES
+-----
+[verse]
+--
+*tracefs.h*
+	Header file to include in order to have access to the library APIs.
+*-ltracefs*
+	Linker switch to add when building a program that uses the library.
+--
+
+SEE ALSO
+--------
+_libtracefs(3)_,
+_libtraceevent(3)_,
+_trace-cmd(1)_,
+_tracefs_hist_pause(3)_,
+_tracefs_hist_continue(3)_,
+_tracefs_hist_reset(3)_
+
+AUTHOR
+------
+[verse]
+--
+*Steven Rostedt* <rostedt@goodmis.org>
+*Tzvetomir Stoyanov* <tz.stoyanov@gmail.com>
+*sameeruddin shaik* <sameeruddin.shaik8@gmail.com>
+--
+REPORTING BUGS
+--------------
+Report bugs to  <linux-trace-devel@vger.kernel.org>
+
+LICENSE
+-------
+libtracefs is Free Software licensed under the GNU LGPL 2.1
+
+RESOURCES
+---------
+https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/
+
+COPYING
+-------
+Copyright \(C) 2020 VMware, Inc. Free use of this software is granted under
+the terms of the GNU Public License (GPL).
diff --git a/include/tracefs.h b/include/tracefs.h
index afbfd4eb01d6..7e1927b078d3 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -253,4 +253,50 @@ enum tracefs_kprobe_type tracefs_kprobe_info(const char *group, const char *even
 					     char **type, char **addr, char **format);
 int tracefs_kprobe_clear_all(bool force);
 int tracefs_kprobe_clear_probe(const char *system, const char *event, bool force);
+
+enum tracefs_hist_key_type {
+	TRACEFS_HIST_KEY_NORMAL = 0,
+	TRACEFS_HIST_KEY_HEX,
+	TRACEFS_HIST_KEY_SYM,
+	TRACEFS_HIST_KEY_SYM_OFFSET,
+	TRACEFS_HIST_KEY_SYSCALL,
+	TRACEFS_HIST_KEY_EXECNAME,
+	TRACEFS_HIST_KEY_LOG,
+	TRACEFS_HIST_KEY_USECS,
+};
+
+enum tracefs_hist_sort_direction {
+	TRACEFS_HIST_SORT_ASCENDING,
+	TRACEFS_HIST_SORT_DESCENDING,
+};
+
+#define TRACEFS_HIST_TIMESTAMP		"common_timestamp"
+#define TRACEFS_HIST_TIMESTAMP_USECS	"common_timestamp.usecs"
+#define TRACEFS_HIST_CPU		"cpu"
+
+#define TRACEFS_HIST_HITCOUNT		"hitcount"
+
+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);
+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);
+int tracefs_hist_add_sort_key(struct tracefs_hist *hist,
+			      const char *sort_key, ...);
+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);
+
 #endif /* _TRACE_FS_H */
diff --git a/src/Makefile b/src/Makefile
index 0697a047f4bc..c7f7c1cc1680 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -9,6 +9,7 @@ OBJS += tracefs-events.o
 OBJS += tracefs-tools.o
 OBJS += tracefs-marker.o
 OBJS += tracefs-kprobes.o
+OBJS += tracefs-hist.o
 
 OBJS := $(OBJS:%.o=$(bdir)/%.o)
 DEPS := $(OBJS:$(bdir)/%.o=$(bdir)/.%.d)
diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
new file mode 100644
index 000000000000..9031a77eba4b
--- /dev/null
+++ b/src/tracefs-hist.c
@@ -0,0 +1,529 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Copyright (C) 2021 VMware Inc, Steven Rostedt <rostedt@goodmis.org>
+ *
+ * Updates:
+ * Copyright (C) 2021, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
+ *
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <dirent.h>
+#include <unistd.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <limits.h>
+
+#include "tracefs.h"
+#include "tracefs-local.h"
+
+#define HIST_FILE "hist"
+
+#define ASCENDING ".ascending"
+#define DESCENDING ".descending"
+
+struct tracefs_hist {
+	struct tracefs_instance *instance;
+	char			*system;
+	char			*event;
+	char			*name;
+	char			**keys;
+	char			**values;
+	char			**sort;
+	char			*filter;
+	int			size;
+};
+
+enum tracefs_hist_command {
+	HIST_CMD_NONE = 0,
+	HIST_CMD_PAUSE,
+	HIST_CMD_CONT,
+	HIST_CMD_CLEAR,
+	HIST_CMD_DESTROY,
+};
+
+static void add_list(struct trace_seq *seq, const char *start,
+		     char **list)
+{
+	int i;
+
+	trace_seq_puts(seq, start);
+	for (i = 0; list[i]; i++) {
+		if (i)
+			trace_seq_putc(seq, ',');
+		trace_seq_puts(seq, list[i]);
+	}
+}
+
+/*
+ * trace_hist_start - Create and start a histogram for an event
+ * @hist: The histogram to write into the trigger file
+ * @command: If not zero, can pause, continue or clear the histogram
+ *
+ * This creates a histogram for an event with the given fields.
+ *
+ * Returns 0 on succes -1 on error.
+ */
+static int
+trace_hist_start(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;
+	struct trace_seq seq;
+	int ret;
+
+	errno = -EINVAL;
+	if (!hist->keys)
+		return -1;
+
+	trace_seq_init(&seq);
+
+	if (command == HIST_CMD_DESTROY)
+		trace_seq_putc(&seq, '!');
+
+	add_list(&seq, "hist:keys=", hist->keys);
+
+	if (hist->values)
+		add_list(&seq, ":vals=", hist->values);
+
+	if (hist->sort)
+		add_list(&seq, ":sort=", hist->sort);
+
+	if (hist->size)
+		trace_seq_printf(&seq, ":size=%d", hist->size);
+
+	switch(command) {
+	case HIST_CMD_NONE: break;
+	case HIST_CMD_PAUSE: trace_seq_puts(&seq, ":pause"); break;
+	case HIST_CMD_CONT: trace_seq_puts(&seq, ":cont"); break;
+	case HIST_CMD_CLEAR: trace_seq_puts(&seq, ":clear"); break;
+	default: break;
+	}
+
+	if (hist->name)
+		trace_seq_printf(&seq, ":name=%s", hist->name);
+
+	if (hist->filter)
+		trace_seq_printf(&seq, " if %s\n", hist->filter);
+
+	trace_seq_terminate(&seq);
+
+	ret = -1;
+	if (seq.state == TRACE_SEQ__GOOD)
+		ret = tracefs_event_file_append(instance, system, event,
+						"trigger", seq.buffer);
+
+	trace_seq_destroy(&seq);
+
+	return ret < 0 ? -1 : 0;
+}
+
+/**
+ * tracefs_hist_free - free a tracefs_hist element
+ * @hist: The histogram to free
+ */
+void tracefs_hist_free(struct tracefs_hist *hist)
+{
+	if (!hist)
+		return;
+
+	trace_put_instance(hist->instance);
+	free(hist->system);
+	free(hist->event);
+	free(hist->name);
+	free(hist->filter);
+	tracefs_list_free(hist->keys);
+	tracefs_list_free(hist->values);
+	tracefs_list_free(hist->sort);
+	free(hist);
+}
+
+/**
+ * tracefs_hist_alloc - Initialize a histogram
+ * @instance: The instance the histogram will be in (NULL for toplevel)
+ * @system: The system the histogram event is in.
+ * @event: The event that the histogram will be attached to.
+ * @key: The primary key the histogram will use
+ * @type: The format type of the key.
+ *
+ * Will initialize a histogram descriptor that will be attached to
+ * the @system/@event with the given @key as the primary. This only
+ * initializes the descriptor, it does not start the histogram
+ * in the kernel.
+ *
+ * Returns an initialized histogram on success.
+ * NULL on failure.
+ */
+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)
+{
+	struct tracefs_hist *hist;
+	int ret;
+
+	if (!system || !event || !key)
+		return NULL;
+
+	if (!tracefs_event_file_exists(instance, system, event, HIST_FILE))
+		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;
+
+	hist->system = strdup(system);
+	hist->event = strdup(event);
+
+	ret = tracefs_hist_add_key(hist, key, type);
+
+	if (!hist->system || !hist->event || ret < 0) {
+		tracefs_hist_free(hist);
+		return NULL;
+	}
+
+
+	return hist;
+}
+
+/**
+ * tracefs_hist_add_key - add to a key to a histogram
+ * @hist: The histogram to add the key to.
+ * @key: The name of the key field.
+ * @type: The type of the key format.
+ *
+ * This adds a secondary or tertiary key to the histogram.
+ *
+ * Returns 0 on success, -1 on error.
+ */
+int tracefs_hist_add_key(struct tracefs_hist *hist, const char *key,
+			 enum tracefs_hist_key_type type)
+{
+	bool use_key = false;
+	char *key_type = NULL;
+	char **new_list;
+	int ret;
+
+	switch (type) {
+	case TRACEFS_HIST_KEY_NORMAL:
+		use_key = true;
+		ret = 0;
+		break;
+	case TRACEFS_HIST_KEY_HEX:
+		ret = asprintf(&key_type, "%s.hex", key);
+		break;
+	case TRACEFS_HIST_KEY_SYM:
+		ret = asprintf(&key_type, "%s.sym", key);
+		break;
+	case TRACEFS_HIST_KEY_SYM_OFFSET:
+		ret = asprintf(&key_type, "%s.sym-offset", key);
+		break;
+	case TRACEFS_HIST_KEY_SYSCALL:
+		ret = asprintf(&key_type, "%s.syscall", key);
+		break;
+	case TRACEFS_HIST_KEY_EXECNAME:
+		ret = asprintf(&key_type, "%s.execname", key);
+		break;
+	case TRACEFS_HIST_KEY_LOG:
+		ret = asprintf(&key_type, "%s.log2", key);
+		break;
+	case TRACEFS_HIST_KEY_USECS:
+		ret = asprintf(&key_type, "%s.usecs", key);
+		break;
+	}
+
+	if (ret < 0)
+		return -1;
+
+	new_list = tracefs_list_add(hist->keys, use_key ? key : key_type);
+	free(key_type);
+	if (!new_list)
+		return -1;
+
+	hist->keys = new_list;
+
+	return 0;
+}
+
+/**
+ * tracefs_hist_add_value - add to a value to a histogram
+ * @hist: The histogram to add the value to.
+ * @key: The name of the value field.
+ *
+ * This adds a value field to the histogram.
+ *
+ * Returns 0 on success, -1 on error.
+ */
+int tracefs_hist_add_value(struct tracefs_hist *hist, const char *value)
+{
+	char **new_list;
+
+	new_list = tracefs_list_add(hist->values, value);
+	if (!new_list)
+		return -1;
+
+	hist->values = new_list;
+
+	return 0;
+}
+
+/**
+ * tracefs_hist_add_name - name a histogram
+ * @hist: The histogram to name.
+ * @name: The name of the histogram.
+ *
+ * Adds a name to the histogram. Named histograms will share their
+ * data with other events that have the same name as if it was
+ * a single histogram.
+ *
+ * If the histogram already has a name, this will fail.
+ *
+ * Returns 0 on success, -1 on error.
+ */
+int tracefs_hist_add_name(struct tracefs_hist *hist, const char *name)
+{
+	if (hist->name)
+		return -1;
+
+	hist->name = strdup(name);
+
+	return hist->name ? 0 : -1;
+}
+
+/**
+ * tracefs_hist_start - enable a histogram
+ * @hist: The histogram to start
+ *
+ * Starts executing a histogram.
+ *
+ * Returns 0 on success, -1 on error.
+ */
+int tracefs_hist_start(struct tracefs_hist *hist)
+{
+	return trace_hist_start(hist, 0);
+}
+
+/**
+ * tracefs_hist_pause - pause a histogram
+ * @hist: The histogram to pause
+ *
+ * Pause a histogram.
+ *
+ * Returns 0 on success, -1 on error.
+ */
+int tracefs_hist_pause(struct tracefs_hist *hist)
+{
+	return trace_hist_start(hist, HIST_CMD_PAUSE);
+}
+
+/**
+ * tracefs_hist_continue - continue a paused histogram
+ * @hist: The histogram to continue
+ *
+ * Continue a histogram.
+ *
+ * Returns 0 on success, -1 on error.
+ */
+int tracefs_hist_continue(struct tracefs_hist *hist)
+{
+	return trace_hist_start(hist, HIST_CMD_CONT);
+}
+
+/**
+ * tracefs_hist_reset - clear a histogram
+ * @hist: The histogram to reset
+ *
+ * Resets a histogram.
+ *
+ * Returns 0 on success, -1 on error.
+ */
+int tracefs_hist_reset(struct tracefs_hist *hist)
+{
+	return trace_hist_start(hist, HIST_CMD_CLEAR);
+}
+
+/**
+ * tracefs_hist_destroy - deletes a histogram (needs to be enabled again)
+ * @hist: The histogram to delete
+ *
+ * Deletes (removes) a running histogram. This is different than
+ * clear, as clear only clears the data but the histogram still exists.
+ * This deletes the histogram and should be called before
+ * tracefs_hist_free() to clean up properly.
+ *
+ * Returns 0 on success, -1 on error.
+ */
+int tracefs_hist_destroy(struct tracefs_hist *hist)
+{
+	return trace_hist_start(hist, HIST_CMD_DESTROY);
+}
+
+static char **
+add_sort_key(struct tracefs_hist *hist, const char *sort_key, char **list)
+{
+	char **key_list = hist->keys;
+	char **val_list = hist->values;
+	int i;
+
+	if (strcmp(sort_key, TRACEFS_HIST_HITCOUNT) == 0)
+		goto out;
+
+	for (i = 0; key_list[i]; i++) {
+		if (strcmp(key_list[i], sort_key) == 0)
+			break;
+	}
+
+	if (!key_list[i]) {
+		for (i = 0; val_list[i]; i++) {
+		if (strcmp(val_list[i], sort_key) == 0)
+			break;
+		if (!val_list[i])
+			return NULL;
+		}
+	}
+
+
+ out:
+	return tracefs_list_add(list, sort_key);
+}
+
+/**
+ * tracefs_hist_add_sort_key - add a key for sorting the histogram
+ * @hist: The histogram to add the sort key to
+ * @sort_key: The key to sort (and the strings after it)
+ *  Last one must be NULL.
+ *
+ * Add a list of sort keys in the order of priority that the
+ * keys would be sorted on output. Keys must be added first.
+ *
+ * Returns 0 on success, -1 on error.
+ */
+int tracefs_hist_add_sort_key(struct tracefs_hist *hist,
+			      const char *sort_key, ...)
+{
+	char **list = NULL;
+	char **tmp;
+	va_list ap;
+
+	if (!hist || !sort_key)
+		return -1;
+
+	tmp = add_sort_key(hist, sort_key, list);
+	if (!tmp)
+		goto fail;
+	list = tmp;
+
+	va_start(ap, sort_key);
+	for (;;) {
+		sort_key = va_arg(ap, const char *);
+		if (!sort_key)
+			break;
+		tmp = add_sort_key(hist, sort_key, list);
+		if (!tmp)
+			goto fail;
+		list = tmp;
+	}
+	va_end(ap);
+
+	tracefs_list_free(hist->sort);
+	hist->sort = list;
+
+	return 0;
+ fail:
+	tracefs_list_free(list);
+	return -1;
+}
+
+static int end_match(const char *sort_key, const char *ending)
+{
+	int key_len = strlen(sort_key);
+	int end_len = strlen(ending);
+
+	if (key_len <= end_len)
+		return 0;
+
+	sort_key += key_len - end_len;
+
+	return strcmp(sort_key, ending) == 0 ? key_len - end_len : 0;
+}
+
+/**
+ * tracefs_hist_sort_key_direction - set direction of a sort key
+ * @hist: The histogram to modify.
+ * @sort_str: The sort key to set the direction for
+ * @dir: The direction to set the sort key to.
+ *
+ * Returns 0 on success, and -1 on error;
+ */
+int tracefs_hist_sort_key_direction(struct tracefs_hist *hist,
+				    const char *sort_str,
+				    enum tracefs_hist_sort_direction dir)
+{
+	char **sort = hist->sort;
+	char *sort_key;
+	char *direct;
+	int match;
+	int i;
+
+	if (!sort)
+		return -1;
+
+	for (i = 0; sort[i]; i++) {
+		if (strcmp(sort[i], sort_str) == 0)
+			break;
+	}
+	if (!sort[i])
+		return -1;
+
+	sort_key = sort[i];
+
+	switch (dir) {
+	case TRACEFS_HIST_SORT_ASCENDING:
+		direct = ASCENDING;
+		break;
+	case TRACEFS_HIST_SORT_DESCENDING:
+		direct = DESCENDING;
+		break;
+	default:
+		return -1;
+	}
+
+	match = end_match(sort_key, ASCENDING);
+	if (match) {
+		/* Already match? */
+		if (dir == TRACEFS_HIST_SORT_ASCENDING)
+			return 0;
+	} else {
+		match = end_match(sort_key, DESCENDING);
+		/* Already match? */
+		if (match && dir == TRACEFS_HIST_SORT_DESCENDING)
+			return 0;
+	}
+
+	if (match)
+		/* Clear the original text */
+		sort_key[match] = '\0';
+
+	sort_key = realloc(sort_key, strlen(sort_key) + strlen(direct) + 1);
+	if (!sort_key) {
+		/* Failed to alloc, may need to put back the match */
+		sort_key = sort[i];
+		if (match)
+			sort_key[match] = '.';
+		return -1;
+	}
+
+	strcat(sort_key, direct);
+	sort[i] = sort_key;
+	return 0;
+}
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] libtracefs: Implement API to create / modify and display histograms
  2021-07-07  3:11 ` [PATCH 4/4] libtracefs: Implement API to create / modify and display histograms Steven Rostedt
@ 2021-07-07  8:12   ` Yordan Karadzhov (VMware)
  2021-07-07 13:13     ` Steven Rostedt
  0 siblings, 1 reply; 9+ messages in thread
From: Yordan Karadzhov (VMware) @ 2021-07-07  8:12 UTC (permalink / raw)
  To: Steven Rostedt, linux-trace-devel
  Cc: Tom Zanussi, Masami Hiramatsu, Namhyung Kim, Daniel Bristot de Oliveira

Hi Steven,

On 7.07.21 г. 6:11, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> 
> Add an API to facilitate the modification of histograms in ftrace:
> 
>    tracefs_hist_alloc()
>    tracefs_hist_add_key()
>    tracefs_hist_add_value()
>    tracefs_hist_add_name()
>    tracefs_hist_start()
>    tracefs_hist_pause()
>    tracefs_hist_continue()
>    tracefs_hist_reset()
>    tracefs_hist_delete()
>    tracefs_hist_add_sort_key()
>    tracefs_hist_sort_key_direction()
> 
> The above functions can create a tracefs_hist descriptor that can
> facilitate the creation and modification of ftrace event histograms.
> 
> Cc: Tom Zanussi <zanussi@kernel.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>   Documentation/libtracefs-hist-cont.txt | 192 +++++++++
>   Documentation/libtracefs-hist.txt      | 290 ++++++++++++++
>   include/tracefs.h                      |  46 +++
>   src/Makefile                           |   1 +
>   src/tracefs-hist.c                     | 529 +++++++++++++++++++++++++
>   5 files changed, 1058 insertions(+)
>   create mode 100644 Documentation/libtracefs-hist-cont.txt
>   create mode 100644 Documentation/libtracefs-hist.txt
>   create mode 100644 src/tracefs-hist.c
> 
> diff --git a/Documentation/libtracefs-hist-cont.txt b/Documentation/libtracefs-hist-cont.txt
> new file mode 100644
> index 000000000000..1b0153c19481
> --- /dev/null
> +++ b/Documentation/libtracefs-hist-cont.txt
> @@ -0,0 +1,192 @@
> +libtracefs(3)
> +=============
> +
> +NAME
> +----
> +tracefs_hist_pause, tracefs_hist_continue, tracefs_hist_reset - Pause, continue, or clear an existing histogram
> +
> +SYNOPSIS
> +--------
> +[verse]
> +--
> +*#include <tracefs.h>*
> +
> +int tracefs_hist_pause(struct tracefs_hist pass:[*]hist);
> +int tracefs_hist_continue(struct tracefs_hist pass:[*]hist);
> +int tracefs_hist_reset(struct tracefs_hist pass:[*]hist);
> +--
> +
> +DESCRIPTION
> +-----------
> +*tracefs_hist_pause* is called to pause the histogram _hist_.
> +
> +*tracefs_hist_continue* is called to continue a paused histogram _hist_.
> +
> +*tracefs_hist_reset* is called to clear / reset the histogram _hist_.
> +
> +RETURN VALUE
> +------------
> +All the return zero on success or -1 on error.
> +
> +EXAMPLE
> +-------
> +[source,c]
> +--
> +#include <stdlib.h>
> +#include <tracefs/tracefs.h>
> +
> +enum commands {
> +	START,
> +	PAUSE,
> +	CONT,
> +	RESET,
> +	DELETE,
> +	SHOW,
> +};
> +
> +int main (int argc, char **argv, char **env)
> +{
> +	struct tracefs_instance *instance;
> +	struct tracefs_hist *hist;
> +	enum commands cmd;
> +	char *cmd_str;
> +	int ret;
> +
> +	if (argc < 2) {
> +		fprintf(stderr, "usage: %s command\n", argv[0]);
> +		exit(-1);
> +	}
> +
> +	cmd_str = argv[1];
> +
> +	if (!strcmp(cmd_str, "start"))
> +		cmd = START;
> +	else if (!strcmp(cmd_str, "pause"))
> +		cmd = PAUSE;
> +	else if (!strcmp(cmd_str, "cont"))
> +		cmd = CONT;
> +	else if (!strcmp(cmd_str, "reset"))
> +		cmd = RESET;
> +	else if (!strcmp(cmd_str, "delete"))
> +		cmd = DELETE;
> +	else if (!strcmp(cmd_str, "show"))
> +		cmd = SHOW;
> +	else {
> +		fprintf(stderr, "Unknown command %s\n", cmd_str);
> +		exit(-1);
> +	}
> +

I see no reason to ask the user to implement this logic.
This can be part of the library.


> +	instance = tracefs_instance_create("hist_test");
> +	if (!instance) {
> +		fprintf(stderr, "Failed instance create\n");
> +		exit(-1);
> +	}
> +
> +	hist = tracefs_hist_alloc(instance, "kmem", "kmalloc",
> +				  "call_site", TRACEFS_HIST_KEY_SYM);
> +	if (!hist) {
> +		fprintf(stderr, "Failed hist create\n");
> +		exit(-1);
> +	}
> +
> +	ret = tracefs_hist_add_key(hist, "bytes_req", 0);
> +	ret |= tracefs_hist_add_value(hist, "bytes_alloc");
> +	ret |= tracefs_hist_add_sort_key(hist, "bytes_req", "bytes_alloc", NULL);
> +
> +	ret |= tracefs_hist_sort_key_direction(hist, "bytes_alloc",
> +					       TRACEFS_HIST_SORT_DESCENDING);
> +	if (ret) {
> +		fprintf(stderr, "Failed modifying histogram\n");
> +		exit(-1);
> +	}
> +
> +	tracefs_error_clear(instance);
> +
> +	switch (cmd) {
> +	case START:
> +		ret = tracefs_hist_start(hist);
> +		if (ret) {
> +			char *err = tracefs_error_last(instance);
> +			if (err)
> +				fprintf(stderr, "\n%s\n", err);
> +		}
> +		break;
> +	case PAUSE:
> +		ret = tracefs_hist_pause(hist);
> +		break;
> +	case CONT:
> +		ret = tracefs_hist_continue(hist);
> +		break;
> +	case RESET:
> +		ret = tracefs_hist_reset(hist);
> +		break;
> +	case DELETE:
> +		ret = tracefs_hist_destroy(hist);
> +		break;
> +	case SHOW: {
> +		char *content;
> +		content = tracefs_event_file_read(instance, "kmem", "kmalloc",
> +						  "hist", NULL);

It looks more intuitive to have

char *tracefs_hist_read(struct tracefs_hist *hist)

> +		ret = content ? 0 : -1;
> +		if (content) {
> +			printf("%s\n", content);
> +			free(content);
> +		}
> +		break;
> +	}
> +	}

This "switch" can move to the library as well.
We can have a method

int tracefs_hist_ctrl(struct tracefs_hist *hist,
		      const char *cmd,
		      void *output);

Thanks!
Yordan



> +	if (ret)
> +		fprintf(stderr, "Failed: command\n");
> +	exit(ret);
> +}
> +--
> +
> +FILES
> +-----
> +[verse]
> +--
> +*tracefs.h*
> +	Header file to include in order to have access to the library APIs.
> +*-ltracefs*
> +	Linker switch to add when building a program that uses the library.
> +--
> +
> +SEE ALSO
> +--------
> +_libtracefs(3)_,
> +_libtraceevent(3)_,
> +_trace-cmd(1)_,
> +_tracefs_hist_alloc(3)_,
> +_tracefs_hist_free(3)_,
> +_tracefs_hist_add_key(3)_,
> +_tracefs_hist_add_value(3)_,
> +_tracefs_hist_add_name(3)_,
> +_tracefs_hist_start(3)_,
> +_tracefs_hist_destory(3)_,
> +_tracefs_hist_add_sort_key(3)_,
> +_tracefs_hist_sort_key_direction(3)_
> +
> +AUTHOR
> +------
> +[verse]
> +--
> +*Steven Rostedt* <rostedt@goodmis.org>
> +*Tzvetomir Stoyanov* <tz.stoyanov@gmail.com>
> +*sameeruddin shaik* <sameeruddin.shaik8@gmail.com>
> +--
> +REPORTING BUGS
> +--------------
> +Report bugs to  <linux-trace-devel@vger.kernel.org>
> +
> +LICENSE
> +-------
> +libtracefs is Free Software licensed under the GNU LGPL 2.1
> +
> +RESOURCES
> +---------
> +https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/
> +
> +COPYING
> +-------
> +Copyright \(C) 2020 VMware, Inc. Free use of this software is granted under
> +the terms of the GNU Public License (GPL).
> diff --git a/Documentation/libtracefs-hist.txt b/Documentation/libtracefs-hist.txt
> new file mode 100644
> index 000000000000..67b2a068ac45
> --- /dev/null
> +++ b/Documentation/libtracefs-hist.txt
> @@ -0,0 +1,290 @@
> +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_sort_key_direction - Create and update event histograms
> +
> +SYNOPSIS
> +--------
> +[verse]
> +--
> +*#include <tracefs.h>*
> +
> +struct tracefs_hist pass:[*]tracefs_hist_alloc(struct tracefs_instance pass:[*] instance,
> +			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);
> +int tracefs_hist_add_key(struct tracefs_hist pass:[*]hist, const char pass:[*]key,
> +			 enum tracefs_hist_key_type type);
> +int tracefs_hist_add_value(struct tracefs_hist pass:[*]hist, const char pass:[*]value);
> +int tracefs_hist_add_sort_key(struct tracefs_hist pass:[*]hist,
> +			      const char pass:[*]sort_key, ...);
> +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);
> +--
> +
> +DESCRIPTION
> +-----------
> +Event histograms are created by the trigger file in the event directory.
> +The syntax can be complex and difficult to get correct. This API handles the
> +syntax, and facilitates the creation and interaction with the event histograms.
> +See https://www.kernel.org/doc/html/latest/trace/histogram.html for more information.
> +
> +*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.
> +
> +*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
> +to be called to do so.
> +
> +*tracefs_hist_add_key*() Adds a secondary or tertiary key to the histogram.
> +The key passed to *tracefs_hist_alloc*() is the primary key of the histogram.
> +The first time this function is called, it will add a secondary key (or two dimensional
> +histogram). If this function is called again on the same histogram, it will add
> +a _tertiary_ key (or three dimensional histogram). The _hist_ parameter is the
> +histrogram descriptor to add the _key_ to. The _type_ is the type of key to add
> +(See KEY TYPES below).
> +
> +*tracefs_hist_add_value*() will add a value to record. By default, the value is
> +simply the "hitcount" of the number of times a instance of the histogram's
> +key was hit. The _hist_ is the histogram descriptor to add the value to.
> +The _value_ is a field in the histogram to add to when an instane of the
> +key is hit.
> +
> +*tracefs_hist_add_sort_key*() will add a key to sort on. Th _hist_ is the
> +the histrogram descriptor to add the sort key to. The _sort_key_ is a string
> +that must match either an already defined key of the histogram, or an already
> +defined value. Multiple sort keys may be added to denote a secondary, sort order
> +and so on, but all sort keys must match an existing key or value, or be
> +TRACEFS_HIST_HITCOUNT. The last parameter of *tracefs_hist_add_sort_key*() must
> +be NULL.
> +
> +*tracefs_hist_sort_key_direction*() allows to change the direction of an
> +existing sort key of _hist_. The _sort_key_ is the sort key to change, and
> +_dir_ can be either TRACEFS_HIST_SORT_ASCENDING or TRACEFS_HIST_SORT_DESCENDING,
> +to make the direction of the sort key either ascending or descending respectively.
> +
> +*tracefs_hist_add_name*() adds a name to a histogram. A histogram may be
> +named and if the name matches between more than one event, and they have
> +compatible keys, the multiple histograms with the same name will be merged
> +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_.
> +
> +*tracefs_hist_pause* is called to pause the histogram _hist_.
> +
> +*tracefs_hist_continue* is called to continue a paused histogram _hist_.
> +
> +*tracefs_hist_reset* is called to clear / reset the histogram _hist_.
> +
> +*tracefs_hist_destory* is called to delete the histogram where it will no longer
> +exist.
> +
> +
> +KEY TYPES
> +---------
> +
> +*tracefs_hist_alloc*() and *tracefs_hist_add_key*() both add a key and requires
> +that key to have a type. The types may be:
> +
> + *TRACEFS_HIST_KEY_NORMAL* or zero (0) which is to not modify the type.
> +
> + *TRACEFS_HIST_KEY_HEX* to display the key in hex.
> +
> + *TRACEFS_HIST_KEY_SYM* to display the key as a kernel symbol (if found). If
> +the key is an address, this is useful as it will display the function names instead
> +of just a number.
> +
> + *TRACEFS_HIST_KEY_SYM_OFFSET* similar to *TRACEFS_HIST_KEY_SYM* but will also include
> +the offset of the function to match the exact address.
> +
> +*TRACEFS_HIST_KEY_SYSCALL* If the key is a system call "id" (the number passed from user
> +space to the kernel to tell it what system call it is calling), then the name of
> +the system call is displayed.
> +
> +*TRACEFS_HIST_KEY_EXECNAME* If "common_pid" is the key (the pid of the executing task),
> +instead of showing the number, show the name of the running task.
> +
> +*TRACEFS_HIST_KEY_LOG* will display the key in a binary logarithmic scale.
> +
> +*TRACEFS_HIST_KEY_USECS* for use with "common_timestamp" or TRACEFS_HIST_TIMESTAMP,
> +in which case it will show the timestamp in microseconds instead of nanoseconds.
> +
> +
> +RETURN VALUE
> +------------
> +*tracefs_hist_alloc*() returns an allocated histogram descriptor which must
> +be freed by *tracefs_hist_free*() or NULL on error.
> +
> +All the other functions return zero on success or -1 on error.
> +
> +If *tracefs_hist_start*() returns an error, a message may be displayed
> +in the kernel that can be retrieved by *tracefs_error_last()*.
> +
> +EXAMPLE
> +-------
> +[source,c]
> +--
> +#include <stdlib.h>
> +#include <tracefs/tracefs.h>
> +
> +enum commands {
> +	START,
> +	PAUSE,
> +	CONT,
> +	RESET,
> +	DELETE,
> +	SHOW,
> +};
> +
> +int main (int argc, char **argv, char **env)
> +{
> +	struct tracefs_instance *instance;
> +	struct tracefs_hist *hist;
> +	enum commands cmd;
> +	char *cmd_str;
> +	int ret;
> +
> +	if (argc < 2) {
> +		fprintf(stderr, "usage: %s command\n", argv[0]);
> +		exit(-1);
> +	}
> +
> +	cmd_str = argv[1];
> +
> +	if (!strcmp(cmd_str, "start"))
> +		cmd = START;
> +	else if (!strcmp(cmd_str, "pause"))
> +		cmd = PAUSE;
> +	else if (!strcmp(cmd_str, "cont"))
> +		cmd = CONT;
> +	else if (!strcmp(cmd_str, "reset"))
> +		cmd = RESET;
> +	else if (!strcmp(cmd_str, "delete"))
> +		cmd = DELETE;
> +	else if (!strcmp(cmd_str, "show"))
> +		cmd = SHOW;
> +	else {
> +		fprintf(stderr, "Unknown command %s\n", cmd_str);
> +		exit(-1);
> +	}
> +
> +	instance = tracefs_instance_create("hist_test");
> +	if (!instance) {
> +		fprintf(stderr, "Failed instance create\n");
> +		exit(-1);
> +	}
> +
> +	hist = tracefs_hist_alloc(instance, "kmem", "kmalloc",
> +				  "call_site", TRACEFS_HIST_KEY_SYM);
> +	if (!hist) {
> +		fprintf(stderr, "Failed hist create\n");
> +		exit(-1);
> +	}
> +
> +	ret = tracefs_hist_add_key(hist, "bytes_req", 0);
> +	ret |= tracefs_hist_add_value(hist, "bytes_alloc");
> +	ret |= tracefs_hist_add_sort_key(hist, "bytes_req", "bytes_alloc", NULL);
> +
> +	ret |= tracefs_hist_sort_key_direction(hist, "bytes_alloc",
> +					       TRACEFS_HIST_SORT_DESCENDING);
> +	if (ret) {
> +		fprintf(stderr, "Failed modifying histogram\n");
> +		exit(-1);
> +	}
> +
> +	tracefs_error_clear(instance);
> +
> +	switch (cmd) {
> +	case START:
> +		ret = tracefs_hist_start(hist);
> +		if (ret) {
> +			char *err = tracefs_error_last(instance);
> +			if (err)
> +				fprintf(stderr, "\n%s\n", err);
> +		}
> +		break;
> +	case PAUSE:
> +		ret = tracefs_hist_pause(hist);
> +		break;
> +	case CONT:
> +		ret = tracefs_hist_continue(hist);
> +		break;
> +	case RESET:
> +		ret = tracefs_hist_reset(hist);
> +		break;
> +	case DELETE:
> +		ret = tracefs_hist_destroy(hist);
> +		break;
> +	case SHOW: {
> +		char *content;
> +		content = tracefs_event_file_read(instance, "kmem", "kmalloc",
> +						  "hist", NULL);
> +		ret = content ? 0 : -1;
> +		if (content) {
> +			printf("%s\n", content);
> +			free(content);
> +		}
> +		break;
> +	}
> +	}
> +	if (ret)
> +		fprintf(stderr, "Failed: command\n");
> +	exit(ret);
> +}
> +--
> +
> +FILES
> +-----
> +[verse]
> +--
> +*tracefs.h*
> +	Header file to include in order to have access to the library APIs.
> +*-ltracefs*
> +	Linker switch to add when building a program that uses the library.
> +--
> +
> +SEE ALSO
> +--------
> +_libtracefs(3)_,
> +_libtraceevent(3)_,
> +_trace-cmd(1)_,
> +_tracefs_hist_pause(3)_,
> +_tracefs_hist_continue(3)_,
> +_tracefs_hist_reset(3)_
> +
> +AUTHOR
> +------
> +[verse]
> +--
> +*Steven Rostedt* <rostedt@goodmis.org>
> +*Tzvetomir Stoyanov* <tz.stoyanov@gmail.com>
> +*sameeruddin shaik* <sameeruddin.shaik8@gmail.com>
> +--
> +REPORTING BUGS
> +--------------
> +Report bugs to  <linux-trace-devel@vger.kernel.org>
> +
> +LICENSE
> +-------
> +libtracefs is Free Software licensed under the GNU LGPL 2.1
> +
> +RESOURCES
> +---------
> +https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/
> +
> +COPYING
> +-------
> +Copyright \(C) 2020 VMware, Inc. Free use of this software is granted under
> +the terms of the GNU Public License (GPL).
> diff --git a/include/tracefs.h b/include/tracefs.h
> index afbfd4eb01d6..7e1927b078d3 100644
> --- a/include/tracefs.h
> +++ b/include/tracefs.h
> @@ -253,4 +253,50 @@ enum tracefs_kprobe_type tracefs_kprobe_info(const char *group, const char *even
>   					     char **type, char **addr, char **format);
>   int tracefs_kprobe_clear_all(bool force);
>   int tracefs_kprobe_clear_probe(const char *system, const char *event, bool force);
> +
> +enum tracefs_hist_key_type {
> +	TRACEFS_HIST_KEY_NORMAL = 0,
> +	TRACEFS_HIST_KEY_HEX,
> +	TRACEFS_HIST_KEY_SYM,
> +	TRACEFS_HIST_KEY_SYM_OFFSET,
> +	TRACEFS_HIST_KEY_SYSCALL,
> +	TRACEFS_HIST_KEY_EXECNAME,
> +	TRACEFS_HIST_KEY_LOG,
> +	TRACEFS_HIST_KEY_USECS,
> +};
> +
> +enum tracefs_hist_sort_direction {
> +	TRACEFS_HIST_SORT_ASCENDING,
> +	TRACEFS_HIST_SORT_DESCENDING,
> +};
> +
> +#define TRACEFS_HIST_TIMESTAMP		"common_timestamp"
> +#define TRACEFS_HIST_TIMESTAMP_USECS	"common_timestamp.usecs"
> +#define TRACEFS_HIST_CPU		"cpu"
> +
> +#define TRACEFS_HIST_HITCOUNT		"hitcount"
> +
> +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);
> +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);
> +int tracefs_hist_add_sort_key(struct tracefs_hist *hist,
> +			      const char *sort_key, ...);
> +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);
> +
>   #endif /* _TRACE_FS_H */
> diff --git a/src/Makefile b/src/Makefile
> index 0697a047f4bc..c7f7c1cc1680 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -9,6 +9,7 @@ OBJS += tracefs-events.o
>   OBJS += tracefs-tools.o
>   OBJS += tracefs-marker.o
>   OBJS += tracefs-kprobes.o
> +OBJS += tracefs-hist.o
>   
>   OBJS := $(OBJS:%.o=$(bdir)/%.o)
>   DEPS := $(OBJS:$(bdir)/%.o=$(bdir)/.%.d)
> diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
> new file mode 100644
> index 000000000000..9031a77eba4b
> --- /dev/null
> +++ b/src/tracefs-hist.c
> @@ -0,0 +1,529 @@
> +// SPDX-License-Identifier: LGPL-2.1
> +/*
> + * Copyright (C) 2021 VMware Inc, Steven Rostedt <rostedt@goodmis.org>
> + *
> + * Updates:
> + * Copyright (C) 2021, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
> + *
> + */
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <dirent.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <limits.h>
> +
> +#include "tracefs.h"
> +#include "tracefs-local.h"
> +
> +#define HIST_FILE "hist"
> +
> +#define ASCENDING ".ascending"
> +#define DESCENDING ".descending"
> +
> +struct tracefs_hist {
> +	struct tracefs_instance *instance;
> +	char			*system;
> +	char			*event;
> +	char			*name;
> +	char			**keys;
> +	char			**values;
> +	char			**sort;
> +	char			*filter;
> +	int			size;
> +};
> +
> +enum tracefs_hist_command {
> +	HIST_CMD_NONE = 0,
> +	HIST_CMD_PAUSE,
> +	HIST_CMD_CONT,
> +	HIST_CMD_CLEAR,
> +	HIST_CMD_DESTROY,
> +};
> +
> +static void add_list(struct trace_seq *seq, const char *start,
> +		     char **list)
> +{
> +	int i;
> +
> +	trace_seq_puts(seq, start);
> +	for (i = 0; list[i]; i++) {
> +		if (i)
> +			trace_seq_putc(seq, ',');
> +		trace_seq_puts(seq, list[i]);
> +	}
> +}
> +
> +/*
> + * trace_hist_start - Create and start a histogram for an event
> + * @hist: The histogram to write into the trigger file
> + * @command: If not zero, can pause, continue or clear the histogram
> + *
> + * This creates a histogram for an event with the given fields.
> + *
> + * Returns 0 on succes -1 on error.
> + */
> +static int
> +trace_hist_start(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;
> +	struct trace_seq seq;
> +	int ret;
> +
> +	errno = -EINVAL;
> +	if (!hist->keys)
> +		return -1;
> +
> +	trace_seq_init(&seq);
> +
> +	if (command == HIST_CMD_DESTROY)
> +		trace_seq_putc(&seq, '!');
> +
> +	add_list(&seq, "hist:keys=", hist->keys);
> +
> +	if (hist->values)
> +		add_list(&seq, ":vals=", hist->values);
> +
> +	if (hist->sort)
> +		add_list(&seq, ":sort=", hist->sort);
> +
> +	if (hist->size)
> +		trace_seq_printf(&seq, ":size=%d", hist->size);
> +
> +	switch(command) {
> +	case HIST_CMD_NONE: break;
> +	case HIST_CMD_PAUSE: trace_seq_puts(&seq, ":pause"); break;
> +	case HIST_CMD_CONT: trace_seq_puts(&seq, ":cont"); break;
> +	case HIST_CMD_CLEAR: trace_seq_puts(&seq, ":clear"); break;
> +	default: break;
> +	}
> +
> +	if (hist->name)
> +		trace_seq_printf(&seq, ":name=%s", hist->name);
> +
> +	if (hist->filter)
> +		trace_seq_printf(&seq, " if %s\n", hist->filter);
> +
> +	trace_seq_terminate(&seq);
> +
> +	ret = -1;
> +	if (seq.state == TRACE_SEQ__GOOD)
> +		ret = tracefs_event_file_append(instance, system, event,
> +						"trigger", seq.buffer);
> +
> +	trace_seq_destroy(&seq);
> +
> +	return ret < 0 ? -1 : 0;
> +}
> +
> +/**
> + * tracefs_hist_free - free a tracefs_hist element
> + * @hist: The histogram to free
> + */
> +void tracefs_hist_free(struct tracefs_hist *hist)
> +{
> +	if (!hist)
> +		return;
> +
> +	trace_put_instance(hist->instance);
> +	free(hist->system);
> +	free(hist->event);
> +	free(hist->name);
> +	free(hist->filter);
> +	tracefs_list_free(hist->keys);
> +	tracefs_list_free(hist->values);
> +	tracefs_list_free(hist->sort);
> +	free(hist);
> +}
> +
> +/**
> + * tracefs_hist_alloc - Initialize a histogram
> + * @instance: The instance the histogram will be in (NULL for toplevel)
> + * @system: The system the histogram event is in.
> + * @event: The event that the histogram will be attached to.
> + * @key: The primary key the histogram will use
> + * @type: The format type of the key.
> + *
> + * Will initialize a histogram descriptor that will be attached to
> + * the @system/@event with the given @key as the primary. This only
> + * initializes the descriptor, it does not start the histogram
> + * in the kernel.
> + *
> + * Returns an initialized histogram on success.
> + * NULL on failure.
> + */
> +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)
> +{
> +	struct tracefs_hist *hist;
> +	int ret;
> +
> +	if (!system || !event || !key)
> +		return NULL;
> +
> +	if (!tracefs_event_file_exists(instance, system, event, HIST_FILE))
> +		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;
> +
> +	hist->system = strdup(system);
> +	hist->event = strdup(event);
> +
> +	ret = tracefs_hist_add_key(hist, key, type);
> +
> +	if (!hist->system || !hist->event || ret < 0) {
> +		tracefs_hist_free(hist);
> +		return NULL;
> +	}
> +
> +
> +	return hist;
> +}
> +
> +/**
> + * tracefs_hist_add_key - add to a key to a histogram
> + * @hist: The histogram to add the key to.
> + * @key: The name of the key field.
> + * @type: The type of the key format.
> + *
> + * This adds a secondary or tertiary key to the histogram.
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +int tracefs_hist_add_key(struct tracefs_hist *hist, const char *key,
> +			 enum tracefs_hist_key_type type)
> +{
> +	bool use_key = false;
> +	char *key_type = NULL;
> +	char **new_list;
> +	int ret;
> +
> +	switch (type) {
> +	case TRACEFS_HIST_KEY_NORMAL:
> +		use_key = true;
> +		ret = 0;
> +		break;
> +	case TRACEFS_HIST_KEY_HEX:
> +		ret = asprintf(&key_type, "%s.hex", key);
> +		break;
> +	case TRACEFS_HIST_KEY_SYM:
> +		ret = asprintf(&key_type, "%s.sym", key);
> +		break;
> +	case TRACEFS_HIST_KEY_SYM_OFFSET:
> +		ret = asprintf(&key_type, "%s.sym-offset", key);
> +		break;
> +	case TRACEFS_HIST_KEY_SYSCALL:
> +		ret = asprintf(&key_type, "%s.syscall", key);
> +		break;
> +	case TRACEFS_HIST_KEY_EXECNAME:
> +		ret = asprintf(&key_type, "%s.execname", key);
> +		break;
> +	case TRACEFS_HIST_KEY_LOG:
> +		ret = asprintf(&key_type, "%s.log2", key);
> +		break;
> +	case TRACEFS_HIST_KEY_USECS:
> +		ret = asprintf(&key_type, "%s.usecs", key);
> +		break;
> +	}
> +
> +	if (ret < 0)
> +		return -1;
> +
> +	new_list = tracefs_list_add(hist->keys, use_key ? key : key_type);
> +	free(key_type);
> +	if (!new_list)
> +		return -1;
> +
> +	hist->keys = new_list;
> +
> +	return 0;
> +}
> +
> +/**
> + * tracefs_hist_add_value - add to a value to a histogram
> + * @hist: The histogram to add the value to.
> + * @key: The name of the value field.
> + *
> + * This adds a value field to the histogram.
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +int tracefs_hist_add_value(struct tracefs_hist *hist, const char *value)
> +{
> +	char **new_list;
> +
> +	new_list = tracefs_list_add(hist->values, value);
> +	if (!new_list)
> +		return -1;
> +
> +	hist->values = new_list;
> +
> +	return 0;
> +}
> +
> +/**
> + * tracefs_hist_add_name - name a histogram
> + * @hist: The histogram to name.
> + * @name: The name of the histogram.
> + *
> + * Adds a name to the histogram. Named histograms will share their
> + * data with other events that have the same name as if it was
> + * a single histogram.
> + *
> + * If the histogram already has a name, this will fail.
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +int tracefs_hist_add_name(struct tracefs_hist *hist, const char *name)
> +{
> +	if (hist->name)
> +		return -1;
> +
> +	hist->name = strdup(name);
> +
> +	return hist->name ? 0 : -1;
> +}
> +
> +/**
> + * tracefs_hist_start - enable a histogram
> + * @hist: The histogram to start
> + *
> + * Starts executing a histogram.
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +int tracefs_hist_start(struct tracefs_hist *hist)
> +{
> +	return trace_hist_start(hist, 0);
> +}
> +
> +/**
> + * tracefs_hist_pause - pause a histogram
> + * @hist: The histogram to pause
> + *
> + * Pause a histogram.
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +int tracefs_hist_pause(struct tracefs_hist *hist)
> +{
> +	return trace_hist_start(hist, HIST_CMD_PAUSE);
> +}
> +
> +/**
> + * tracefs_hist_continue - continue a paused histogram
> + * @hist: The histogram to continue
> + *
> + * Continue a histogram.
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +int tracefs_hist_continue(struct tracefs_hist *hist)
> +{
> +	return trace_hist_start(hist, HIST_CMD_CONT);
> +}
> +
> +/**
> + * tracefs_hist_reset - clear a histogram
> + * @hist: The histogram to reset
> + *
> + * Resets a histogram.
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +int tracefs_hist_reset(struct tracefs_hist *hist)
> +{
> +	return trace_hist_start(hist, HIST_CMD_CLEAR);
> +}
> +
> +/**
> + * tracefs_hist_destroy - deletes a histogram (needs to be enabled again)
> + * @hist: The histogram to delete
> + *
> + * Deletes (removes) a running histogram. This is different than
> + * clear, as clear only clears the data but the histogram still exists.
> + * This deletes the histogram and should be called before
> + * tracefs_hist_free() to clean up properly.
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +int tracefs_hist_destroy(struct tracefs_hist *hist)
> +{
> +	return trace_hist_start(hist, HIST_CMD_DESTROY);
> +}
> +
> +static char **
> +add_sort_key(struct tracefs_hist *hist, const char *sort_key, char **list)
> +{
> +	char **key_list = hist->keys;
> +	char **val_list = hist->values;
> +	int i;
> +
> +	if (strcmp(sort_key, TRACEFS_HIST_HITCOUNT) == 0)
> +		goto out;
> +
> +	for (i = 0; key_list[i]; i++) {
> +		if (strcmp(key_list[i], sort_key) == 0)
> +			break;
> +	}
> +
> +	if (!key_list[i]) {
> +		for (i = 0; val_list[i]; i++) {
> +		if (strcmp(val_list[i], sort_key) == 0)
> +			break;
> +		if (!val_list[i])
> +			return NULL;
> +		}
> +	}
> +
> +
> + out:
> +	return tracefs_list_add(list, sort_key);
> +}
> +
> +/**
> + * tracefs_hist_add_sort_key - add a key for sorting the histogram
> + * @hist: The histogram to add the sort key to
> + * @sort_key: The key to sort (and the strings after it)
> + *  Last one must be NULL.
> + *
> + * Add a list of sort keys in the order of priority that the
> + * keys would be sorted on output. Keys must be added first.
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +int tracefs_hist_add_sort_key(struct tracefs_hist *hist,
> +			      const char *sort_key, ...)
> +{
> +	char **list = NULL;
> +	char **tmp;
> +	va_list ap;
> +
> +	if (!hist || !sort_key)
> +		return -1;
> +
> +	tmp = add_sort_key(hist, sort_key, list);
> +	if (!tmp)
> +		goto fail;
> +	list = tmp;
> +
> +	va_start(ap, sort_key);
> +	for (;;) {
> +		sort_key = va_arg(ap, const char *);
> +		if (!sort_key)
> +			break;
> +		tmp = add_sort_key(hist, sort_key, list);
> +		if (!tmp)
> +			goto fail;
> +		list = tmp;
> +	}
> +	va_end(ap);
> +
> +	tracefs_list_free(hist->sort);
> +	hist->sort = list;
> +
> +	return 0;
> + fail:
> +	tracefs_list_free(list);
> +	return -1;
> +}
> +
> +static int end_match(const char *sort_key, const char *ending)
> +{
> +	int key_len = strlen(sort_key);
> +	int end_len = strlen(ending);
> +
> +	if (key_len <= end_len)
> +		return 0;
> +
> +	sort_key += key_len - end_len;
> +
> +	return strcmp(sort_key, ending) == 0 ? key_len - end_len : 0;
> +}
> +
> +/**
> + * tracefs_hist_sort_key_direction - set direction of a sort key
> + * @hist: The histogram to modify.
> + * @sort_str: The sort key to set the direction for
> + * @dir: The direction to set the sort key to.
> + *
> + * Returns 0 on success, and -1 on error;
> + */
> +int tracefs_hist_sort_key_direction(struct tracefs_hist *hist,
> +				    const char *sort_str,
> +				    enum tracefs_hist_sort_direction dir)
> +{
> +	char **sort = hist->sort;
> +	char *sort_key;
> +	char *direct;
> +	int match;
> +	int i;
> +
> +	if (!sort)
> +		return -1;
> +
> +	for (i = 0; sort[i]; i++) {
> +		if (strcmp(sort[i], sort_str) == 0)
> +			break;
> +	}
> +	if (!sort[i])
> +		return -1;
> +
> +	sort_key = sort[i];
> +
> +	switch (dir) {
> +	case TRACEFS_HIST_SORT_ASCENDING:
> +		direct = ASCENDING;
> +		break;
> +	case TRACEFS_HIST_SORT_DESCENDING:
> +		direct = DESCENDING;
> +		break;
> +	default:
> +		return -1;
> +	}
> +
> +	match = end_match(sort_key, ASCENDING);
> +	if (match) {
> +		/* Already match? */
> +		if (dir == TRACEFS_HIST_SORT_ASCENDING)
> +			return 0;
> +	} else {
> +		match = end_match(sort_key, DESCENDING);
> +		/* Already match? */
> +		if (match && dir == TRACEFS_HIST_SORT_DESCENDING)
> +			return 0;
> +	}
> +
> +	if (match)
> +		/* Clear the original text */
> +		sort_key[match] = '\0';
> +
> +	sort_key = realloc(sort_key, strlen(sort_key) + strlen(direct) + 1);
> +	if (!sort_key) {
> +		/* Failed to alloc, may need to put back the match */
> +		sort_key = sort[i];
> +		if (match)
> +			sort_key[match] = '.';
> +		return -1;
> +	}
> +
> +	strcat(sort_key, direct);
> +	sort[i] = sort_key;
> +	return 0;
> +}
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] libtracefs: Implement API to create / modify and display histograms
  2021-07-07  8:12   ` Yordan Karadzhov (VMware)
@ 2021-07-07 13:13     ` Steven Rostedt
  2021-07-07 13:21       ` Yordan Karadzhov (VMware)
  0 siblings, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2021-07-07 13:13 UTC (permalink / raw)
  To: Yordan Karadzhov (VMware)
  Cc: linux-trace-devel, Tom Zanussi, Masami Hiramatsu, Namhyung Kim,
	Daniel Bristot de Oliveira

On Wed, 7 Jul 2021 11:12:03 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> Hi Steven,

> > +
> > +	switch (cmd) {
> > +	case START:
> > +		ret = tracefs_hist_start(hist);
> > +		if (ret) {
> > +			char *err = tracefs_error_last(instance);
> > +			if (err)
> > +				fprintf(stderr, "\n%s\n", err);
> > +		}
> > +		break;
> > +	case PAUSE:
> > +		ret = tracefs_hist_pause(hist);
> > +		break;
> > +	case CONT:
> > +		ret = tracefs_hist_continue(hist);
> > +		break;
> > +	case RESET:
> > +		ret = tracefs_hist_reset(hist);
> > +		break;
> > +	case DELETE:
> > +		ret = tracefs_hist_destroy(hist);
> > +		break;
> > +	case SHOW: {
> > +		char *content;
> > +		content = tracefs_event_file_read(instance, "kmem", "kmalloc",
> > +						  "hist", NULL);  
> 
> It looks more intuitive to have
> 
> char *tracefs_hist_read(struct tracefs_hist *hist)

I thought a little about this, and rather have parsing of the file. We
could always add a helper function to do this later if it is helpful,

> 
> > +		ret = content ? 0 : -1;
> > +		if (content) {
> > +			printf("%s\n", content);
> > +			free(content);
> > +		}
> > +		break;
> > +	}
> > +	}  
> 
> This "switch" can move to the library as well.
> We can have a method
> 
> int tracefs_hist_ctrl(struct tracefs_hist *hist,
> 		      const char *cmd,
> 		      void *output);
> 

Both this and the read file above could be added as enhancements, but I
don't think we need to add them to this current patch.

Thanks,

-- Steve

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] libtracefs: Implement API to create / modify and display histograms
  2021-07-07 13:13     ` Steven Rostedt
@ 2021-07-07 13:21       ` Yordan Karadzhov (VMware)
  2021-07-07 13:48         ` Steven Rostedt
  0 siblings, 1 reply; 9+ messages in thread
From: Yordan Karadzhov (VMware) @ 2021-07-07 13:21 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-trace-devel, Tom Zanussi, Masami Hiramatsu, Namhyung Kim,
	Daniel Bristot de Oliveira



On 7.07.21 г. 16:13, Steven Rostedt wrote:
> On Wed, 7 Jul 2021 11:12:03 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
>> Hi Steven,
> 
>>> +
>>> +	switch (cmd) {
>>> +	case START:
>>> +		ret = tracefs_hist_start(hist);
>>> +		if (ret) {
>>> +			char *err = tracefs_error_last(instance);
>>> +			if (err)
>>> +				fprintf(stderr, "\n%s\n", err);
>>> +		}
>>> +		break;
>>> +	case PAUSE:
>>> +		ret = tracefs_hist_pause(hist);
>>> +		break;
>>> +	case CONT:
>>> +		ret = tracefs_hist_continue(hist);
>>> +		break;
>>> +	case RESET:
>>> +		ret = tracefs_hist_reset(hist);
>>> +		break;
>>> +	case DELETE:
>>> +		ret = tracefs_hist_destroy(hist);
>>> +		break;
>>> +	case SHOW: {
>>> +		char *content;
>>> +		content = tracefs_event_file_read(instance, "kmem", "kmalloc",
>>> +						  "hist", NULL);
>>
>> It looks more intuitive to have
>>
>> char *tracefs_hist_read(struct tracefs_hist *hist)
> 
> I thought a little about this, and rather have parsing of the file. We
> could always add a helper function to do this later if it is helpful,
> 
>>
>>> +		ret = content ? 0 : -1;
>>> +		if (content) {
>>> +			printf("%s\n", content);
>>> +			free(content);
>>> +		}
>>> +		break;
>>> +	}
>>> +	}
>>
>> This "switch" can move to the library as well.
>> We can have a method
>>
>> int tracefs_hist_ctrl(struct tracefs_hist *hist,
>> 		      const char *cmd,
>> 		      void *output);
>>
> 
> Both this and the read file above could be added as enhancements, but I
> don't think we need to add them to this current patch.
> 

Yes, sure.

But maybe in this case you can postpone the addition of the examples for 
another patch that is after having those enhancements.

Thanks!
Yordan


> Thanks,
> 
> -- Steve
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] libtracefs: Implement API to create / modify and display histograms
  2021-07-07 13:21       ` Yordan Karadzhov (VMware)
@ 2021-07-07 13:48         ` Steven Rostedt
  0 siblings, 0 replies; 9+ messages in thread
From: Steven Rostedt @ 2021-07-07 13:48 UTC (permalink / raw)
  To: Yordan Karadzhov (VMware)
  Cc: linux-trace-devel, Tom Zanussi, Masami Hiramatsu, Namhyung Kim,
	Daniel Bristot de Oliveira

On Wed, 7 Jul 2021 16:21:30 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> Yes, sure.
> 
> But maybe in this case you can postpone the addition of the examples for 
> another patch that is after having those enhancements.

Well, the examples show how to use what is being described.

The new API patch could (and should) update the samples.

-- Steve

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-07-07 13:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-07  3:11 [PATCH 0/4] libtracefs: Work to add histogram APIs Steven Rostedt
2021-07-07  3:11 ` [PATCH 1/4] libtracefs: Implement tracefs_list_size() Steven Rostedt
2021-07-07  3:11 ` [PATCH 2/4] libtracefs: Implement functions to work on event directory files Steven Rostedt
2021-07-07  3:11 ` [PATCH 3/4] libtracefs: Have instances have internal ref counting Steven Rostedt
2021-07-07  3:11 ` [PATCH 4/4] libtracefs: Implement API to create / modify and display histograms Steven Rostedt
2021-07-07  8:12   ` Yordan Karadzhov (VMware)
2021-07-07 13:13     ` Steven Rostedt
2021-07-07 13:21       ` Yordan Karadzhov (VMware)
2021-07-07 13:48         ` 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).