All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] tracefs library
@ 2020-01-06 15:40 Tzvetomir Stoyanov (VMware)
  2020-01-06 15:40 ` [PATCH v2 1/5] trace-cmd: Introduce libtracefs library Tzvetomir Stoyanov (VMware)
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-01-06 15:40 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Add a skeleton for new library: libtracefs, providing APIs
for accessing files from tracefs. The library files are added in
 lib/tracefs
 include/tracefs
It is built and installed by trace-cmd targets
  make libs 
  make install_libs

[
 v2 changes:
  - Fix copyright of the newly created files
  - Remove tracefs_write_file() API, add it as a local helper function 
]

Tzvetomir Stoyanov (VMware) (5):
  trace-cmd: Introduce libtracefs library
  kernel-shark: Use new tracefs library
  trace-cmd: New libtracefs APIs for ftrace instances
  trace-cmd,kernel-shark: New libtracefs APIs for ftrace events and
    systems
  trace-cmd,kernel-shark: New libtracefs APIs for loading ftrace events

 Makefile                              |  27 +-
 include/trace-cmd/trace-cmd.h         |  21 -
 include/tracefs/tracefs.h             |  56 +++
 kernel-shark/CMakeLists.txt           |   3 +-
 kernel-shark/build/FindTraceCmd.cmake |  30 ++
 kernel-shark/src/CMakeLists.txt       |   2 +
 kernel-shark/src/KsCaptureDialog.cpp  |   4 +-
 kernel-shark/src/libkshark.h          |   1 +
 lib/trace-cmd/trace-input.c           |  95 ----
 lib/trace-cmd/trace-output.c          |   3 +-
 lib/trace-cmd/trace-recorder.c        |   7 +-
 lib/trace-cmd/trace-util.c            | 629 ------------------------
 lib/tracefs/Makefile                  |  48 ++
 lib/tracefs/include/tracefs-local.h   |  13 +
 lib/tracefs/tracefs-events.c          | 661 ++++++++++++++++++++++++++
 lib/tracefs/tracefs-instance.c        | 267 +++++++++++
 lib/tracefs/tracefs-utils.c           | 226 +++++++++
 tracecmd/Makefile                     |   2 +-
 tracecmd/include/trace-local.h        |   5 +-
 tracecmd/trace-check-events.c         |   5 +-
 tracecmd/trace-list.c                 |  21 +-
 tracecmd/trace-record.c               | 373 ++++++---------
 tracecmd/trace-show.c                 |   7 +-
 tracecmd/trace-snapshot.c             |   9 +-
 tracecmd/trace-stack.c                |   9 +-
 tracecmd/trace-stat.c                 |  38 +-
 26 files changed, 1531 insertions(+), 1031 deletions(-)
 create mode 100644 include/tracefs/tracefs.h
 create mode 100644 lib/tracefs/Makefile
 create mode 100644 lib/tracefs/include/tracefs-local.h
 create mode 100644 lib/tracefs/tracefs-events.c
 create mode 100644 lib/tracefs/tracefs-instance.c
 create mode 100644 lib/tracefs/tracefs-utils.c

-- 
2.24.1


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

* [PATCH v2 1/5] trace-cmd: Introduce libtracefs library
  2020-01-06 15:40 [PATCH v2 0/5] tracefs library Tzvetomir Stoyanov (VMware)
@ 2020-01-06 15:40 ` Tzvetomir Stoyanov (VMware)
  2020-01-06 15:40 ` [PATCH v2 2/5] kernel-shark: Use new tracefs library Tzvetomir Stoyanov (VMware)
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-01-06 15:40 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Add a skeleton for new library: libtracefs.
It provides APIs for accessing files from tracefs.
 - Added new directories
    lib/tracefs
    include/tracefs
 - Integrated the libtracefs build into the trace-cmd compilation
 - The library is installed by "make install_libs" in:
         libtrasefs.so in $(libdir)/tracefs
         trasefs.h in $(includedir)/tracefs
 - Added implementation of initial APIs:
    char *tracefs_get_tracing_file(const char *name);
    void tracefs_put_tracing_file(char *name);
    const char *tracefs_get_tracing_dir(void);
    char *tracefs_find_tracing_dir(void);

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 Makefile                            |  27 +++-
 include/trace-cmd/trace-cmd.h       |   9 --
 include/tracefs/tracefs.h           |  20 +++
 lib/trace-cmd/trace-output.c        |   3 +-
 lib/trace-cmd/trace-recorder.c      |   7 +-
 lib/trace-cmd/trace-util.c          | 136 ---------------------
 lib/tracefs/Makefile                |  46 +++++++
 lib/tracefs/include/tracefs-local.h |  12 ++
 lib/tracefs/tracefs-utils.c         | 183 ++++++++++++++++++++++++++++
 tracecmd/Makefile                   |   2 +-
 tracecmd/trace-check-events.c       |   3 +-
 tracecmd/trace-list.c               |  19 +--
 tracecmd/trace-record.c             | 127 +++++++++----------
 tracecmd/trace-show.c               |   5 +-
 tracecmd/trace-snapshot.c           |   9 +-
 tracecmd/trace-stack.c              |   9 +-
 tracecmd/trace-stat.c               |  17 +--
 17 files changed, 390 insertions(+), 244 deletions(-)
 create mode 100644 include/tracefs/tracefs.h
 create mode 100644 lib/tracefs/Makefile
 create mode 100644 lib/tracefs/include/tracefs-local.h
 create mode 100644 lib/tracefs/tracefs-utils.c

diff --git a/Makefile b/Makefile
index 48d88e7..aa803ba 100644
--- a/Makefile
+++ b/Makefile
@@ -183,12 +183,19 @@ LIBTRACECMD_DIR = $(obj)/lib/trace-cmd
 LIBTRACECMD_STATIC = $(LIBTRACECMD_DIR)/libtracecmd.a
 LIBTRACECMD_SHARED = $(LIBTRACECMD_DIR)/libtracecmd.so
 
-TRACE_LIBS = -L$(LIBTRACECMD_DIR) -ltracecmd -L$(LIBTRACEEVENT_DIR) -ltraceevent
+LIBTRACEFS_DIR = $(obj)/lib/tracefs
+LIBTRACEFS_STATIC = $(LIBTRACEFS_DIR)/libtracefs.a
+LIBTRACEFS_SHARED = $(LIBTRACEFS_DIR)/libtracefs.so
+
+TRACE_LIBS = -L$(LIBTRACECMD_DIR) -ltracecmd		\
+	     -L$(LIBTRACEEVENT_DIR) -ltraceevent	\
+	     -L$(LIBTRACEFS_DIR) -ltracefs
 
 export LIBS TRACE_LIBS
 export LIBTRACEEVENT_DIR LIBTRACECMD_DIR
 export LIBTRACECMD_STATIC LIBTRACECMD_SHARED
 export LIBTRACEEVENT_STATIC LIBTRACEEVENT_SHARED
+export LIBTRACEFS_STATIC LIBTRACEFS_SHARED
 
 export Q SILENT VERBOSE EXT
 
@@ -198,8 +205,10 @@ include scripts/utils.mk
 INCLUDES = -I$(src)/include -I$(src)/../../include
 INCLUDES += -I$(src)/include/traceevent
 INCLUDES += -I$(src)/include/trace-cmd
+INCLUDES += -I$(src)/include/tracefs
 INCLUDES += -I$(src)/lib/traceevent/include
 INCLUDES += -I$(src)/lib/trace-cmd/include
+INCLUDES += -I$(src)/lib/tracefs/include
 INCLUDES += -I$(src)/tracecmd/include
 INCLUDES += -I$(obj)/tracecmd/include
 
@@ -277,7 +286,7 @@ gui: force $(CMD_TARGETS) $(kshark-dir)/build/Makefile
 	@echo "gui build complete"
 	@echo "  kernelshark located at $(kshark-dir)/bin"
 
-trace-cmd: force $(LIBTRACEEVENT_STATIC) $(LIBTRACECMD_STATIC)
+trace-cmd: force $(LIBTRACEEVENT_STATIC) $(LIBTRACECMD_STATIC) $(LIBTRACEFS_STATIC)
 	$(Q)$(MAKE) -C $(src)/tracecmd $(obj)/tracecmd/$@
 
 $(LIBTRACEEVENT_SHARED): force $(obj)/lib/traceevent/plugins/traceevent_plugin_dir
@@ -292,12 +301,21 @@ $(LIBTRACECMD_STATIC): force
 $(LIBTRACECMD_SHARED): force
 	$(Q)$(MAKE) -C $(src)/lib/trace-cmd $@
 
+$(LIBTRACEFS_STATIC): force
+	$(Q)$(MAKE) -C $(src)/lib/tracefs $@
+
+$(LIBTRACEFS_SHARED): force
+	$(Q)$(MAKE) -C $(src)/lib/tracefs $@
+
+
 libtraceevent.so: $(LIBTRACEEVENT_SHARED)
 libtraceevent.a: $(LIBTRACEEVENT_STATIC)
 libtracecmd.a: $(LIBTRACECMD_STATIC)
 libtracecmd.so: $(LIBTRACECMD_SHARED)
+libtracefs.a: $(LIBTRACEFS_STATIC)
+libtracefs.so: $(LIBTRACEFS_SHARED)
 
-libs: $(LIBTRACECMD_SHARED) $(LIBTRACEEVENT_SHARED)
+libs: $(LIBTRACECMD_SHARED) $(LIBTRACEEVENT_SHARED) $(LIBTRACEFS_SHARED)
 
 plugins: force $(obj)/lib/traceevent/plugins/traceevent_plugin_dir $(obj)/lib/traceevent/plugins/trace_python_dir
 	$(Q)$(MAKE) -C $(src)/lib/traceevent/plugins
@@ -353,10 +371,12 @@ install_gui: install_cmd gui
 install_libs: libs
 	$(Q)$(call do_install,$(LIBTRACECMD_SHARED),$(libdir_SQ)/trace-cmd)
 	$(Q)$(call do_install,$(LIBTRACEEVENT_SHARED),$(libdir_SQ)/traceevent)
+	$(Q)$(call do_install,$(LIBTRACEFS_SHARED),$(libdir_SQ)/tracefs)
 	$(Q)$(call do_install,$(src)/include/traceevent/event-parse.h,$(includedir_SQ)/traceevent)
 	$(Q)$(call do_install,$(src)/include/traceevent/trace-seq.h,$(includedir_SQ)/traceevent)
 	$(Q)$(call do_install,$(src)/include/trace-cmd/trace-cmd.h,$(includedir_SQ)/trace-cmd)
 	$(Q)$(call do_install,$(src)/include/trace-cmd/trace-filter-hash.h,$(includedir_SQ)/trace-cmd)
+	$(Q)$(call do_install,$(src)/include/tracefs/tracefs.h,$(includedir_SQ)/tracefs)
 
 doc:
 	$(MAKE) -C $(src)/Documentation all
@@ -379,6 +399,7 @@ clean:
 	$(RM) tags TAGS cscope*
 	$(MAKE) -C $(src)/lib/traceevent clean
 	$(MAKE) -C $(src)/lib/trace-cmd clean
+	$(MAKE) -C $(src)/lib/tracefs clean
 	$(MAKE) -C $(src)/lib/traceevent/plugins clean
 	$(MAKE) -C $(src)/python clean
 	$(MAKE) -C $(src)/tracecmd clean
diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 13afce7..66736ae 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -64,12 +64,6 @@ static inline int tracecmd_host_bigendian(void)
 	return *ptr == 0x01020304;
 }
 
-/* tracecmd_get_tracing_dir must *not* be freed */
-const char *tracecmd_get_tracing_dir(void);
-
-/* tracecmd_find_tracing_dir must be freed */
-char *tracecmd_find_tracing_dir(void);
-
 /* --- Opening and Reading the trace.dat file --- */
 
 enum {
@@ -208,9 +202,6 @@ tracecmd_get_show_data_func(struct tracecmd_input *handle);
 void tracecmd_set_show_data_func(struct tracecmd_input *handle,
 				 tracecmd_show_data_func func);
 
-char *tracecmd_get_tracing_file(const char *name);
-void tracecmd_put_tracing_file(char *name);
-
 int tracecmd_record_at_buffer_start(struct tracecmd_input *handle, struct tep_record *record);
 unsigned long long tracecmd_page_ts(struct tracecmd_input *handle,
 				    struct tep_record *record);
diff --git a/include/tracefs/tracefs.h b/include/tracefs/tracefs.h
new file mode 100644
index 0000000..e844c75
--- /dev/null
+++ b/include/tracefs/tracefs.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: LGPL-2.1 */
+/*
+ * Copyright (C) 2019, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
+ *
+ */
+#ifndef _TRACE_FS_H
+#define _TRACE_FS_H
+
+#include "traceevent/event-parse.h"
+
+char *tracefs_get_tracing_file(const char *name);
+void tracefs_put_tracing_file(char *name);
+
+/* tracefs_get_tracing_dir must *not* be freed */
+const char *tracefs_get_tracing_dir(void);
+
+/* tracefs_find_tracing_dir must be freed */
+char *tracefs_find_tracing_dir(void);
+
+#endif /* _TRACE_FS_H */
diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index be4d3f5..a3dda27 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -20,6 +20,7 @@
 #include <errno.h>
 #include <glob.h>
 
+#include "tracefs.h"
 #include "trace-cmd-local.h"
 #include "list.h"
 #include "trace-msg.h"
@@ -238,7 +239,7 @@ static tsize_t copy_file(struct tracecmd_output *handle,
 static const char *find_tracing_dir(struct tracecmd_output *handle)
 {
 	if (!handle->tracing_dir)
-		handle->tracing_dir = tracecmd_find_tracing_dir();
+		handle->tracing_dir = tracefs_find_tracing_dir();
 
 	return handle->tracing_dir;
 }
diff --git a/lib/trace-cmd/trace-recorder.c b/lib/trace-cmd/trace-recorder.c
index 36c9a96..2a6e2b6 100644
--- a/lib/trace-cmd/trace-recorder.c
+++ b/lib/trace-cmd/trace-recorder.c
@@ -12,6 +12,7 @@
 #include <unistd.h>
 #include <errno.h>
 
+#include "tracefs.h"
 #include "trace-cmd.h"
 #include "event-utils.h"
 
@@ -306,7 +307,7 @@ struct tracecmd_recorder *tracecmd_create_recorder_fd(int fd, int cpu, unsigned
 {
 	const char *tracing;
 
-	tracing = tracecmd_get_tracing_dir();
+	tracing = tracefs_get_tracing_dir();
 	if (!tracing) {
 		errno = ENODEV;
 		return NULL;
@@ -319,7 +320,7 @@ struct tracecmd_recorder *tracecmd_create_recorder(const char *file, int cpu, un
 {
 	const char *tracing;
 
-	tracing = tracecmd_get_tracing_dir();
+	tracing = tracefs_get_tracing_dir();
 	if (!tracing) {
 		errno = ENODEV;
 		return NULL;
@@ -333,7 +334,7 @@ tracecmd_create_recorder_maxkb(const char *file, int cpu, unsigned flags, int ma
 {
 	const char *tracing;
 
-	tracing = tracecmd_get_tracing_dir();
+	tracing = tracefs_get_tracing_dir();
 	if (!tracing) {
 		errno = ENODEV;
 		return NULL;
diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
index b5bb0d5..1394469 100644
--- a/lib/trace-cmd/trace-util.c
+++ b/lib/trace-cmd/trace-util.c
@@ -23,8 +23,6 @@
 #include "event-utils.h"
 
 #define LOCAL_PLUGIN_DIR ".trace-cmd/plugins"
-#define TRACEFS_PATH "/sys/kernel/tracing"
-#define DEBUGFS_PATH "/sys/kernel/debug"
 #define PROC_STACK_FILE "/proc/sys/kernel/stack_tracer_enabled"
 
 int tracecmd_disable_sys_plugins;
@@ -33,9 +31,6 @@ static bool debug;
 
 static FILE *logfp;
 
-#define _STR(x) #x
-#define STR(x) _STR(x)
-
 /**
  * tracecmd_set_debug - Set debug mode of the tracecmd library
  * @set_debug: The new "debug" mode. If true, the tracecmd library is
@@ -147,113 +142,6 @@ void tracecmd_parse_ftrace_printk(struct tep_handle *pevent,
 	}
 }
 
-static int mount_debugfs(void)
-{
-	struct stat st;
-	int ret;
-
-	/* make sure debugfs exists */
-	ret = stat(DEBUGFS_PATH, &st);
-	if (ret < 0)
-		return -1;
-
-	ret = mount("nodev", DEBUGFS_PATH,
-		    "debugfs", 0, NULL);
-
-	return ret;
-}
-
-static int mount_tracefs(void)
-{
-	struct stat st;
-	int ret;
-
-	/* make sure debugfs exists */
-	ret = stat(TRACEFS_PATH, &st);
-	if (ret < 0)
-		return -1;
-
-	ret = mount("nodev", TRACEFS_PATH,
-		    "tracefs", 0, NULL);
-
-	return ret;
-}
-
-char *tracecmd_find_tracing_dir(void)
-{
-	char *debug_str = NULL;
-	char fspath[PATH_MAX+1];
-	char *tracing_dir;
-	char type[100];
-	int use_debug = 0;
-	FILE *fp;
-
-	if ((fp = fopen("/proc/mounts","r")) == NULL) {
-		warning("Can't open /proc/mounts for read");
-		return NULL;
-	}
-
-	while (fscanf(fp, "%*s %"
-		      STR(PATH_MAX)
-		      "s %99s %*s %*d %*d\n",
-		      fspath, type) == 2) {
-		if (strcmp(type, "tracefs") == 0)
-			break;
-		if (!debug_str && strcmp(type, "debugfs") == 0) {
-			debug_str = strdup(fspath);
-			if (!debug_str) {
-				fclose(fp);
-				return NULL;
-			}
-		}
-	}
-	fclose(fp);
-
-	if (strcmp(type, "tracefs") != 0) {
-		if (mount_tracefs() < 0) {
-			if (debug_str) {
-				strncpy(fspath, debug_str, PATH_MAX);
-				fspath[PATH_MAX] = 0;
-			} else {
-				if (mount_debugfs() < 0) {
-					warning("debugfs not mounted, please mount");
-					free(debug_str);
-					return NULL;
-				}
-				strcpy(fspath, DEBUGFS_PATH);
-			}
-			use_debug = 1;
-		} else
-			strcpy(fspath, TRACEFS_PATH);
-	}
-	free(debug_str);
-
-	if (use_debug) {
-		int ret;
-
-		ret = asprintf(&tracing_dir, "%s/tracing", fspath);
-		if (ret < 0)
-			return NULL;
-	} else {
-		tracing_dir = strdup(fspath);
-		if (!tracing_dir)
-			return NULL;
-	}
-
-	return tracing_dir;
-}
-
-const char *tracecmd_get_tracing_dir(void)
-{
-	static const char *tracing_dir;
-
-	if (tracing_dir)
-		return tracing_dir;
-
-	tracing_dir = tracecmd_find_tracing_dir();
-	return tracing_dir;
-}
-
 /* FIXME: append_file() is duplicated and could be consolidated */
 static char *append_file(const char *dir, const char *name)
 {
@@ -863,30 +751,6 @@ void trace_util_free_plugin_files(char **files)
 	free(files);
 }
 
-char *tracecmd_get_tracing_file(const char *name)
-{
-	static const char *tracing;
-	char *file;
-	int ret;
-
-	if (!tracing) {
-		tracing = tracecmd_find_tracing_dir();
-		if (!tracing)
-			return NULL;
-	}
-
-	ret = asprintf(&file, "%s/%s", tracing, name);
-	if (ret < 0)
-		return NULL;
-
-	return file;
-}
-
-void tracecmd_put_tracing_file(char *name)
-{
-	free(name);
-}
-
 void __noreturn __vdie(const char *fmt, va_list ap)
 {
 	int ret = errno;
diff --git a/lib/tracefs/Makefile b/lib/tracefs/Makefile
new file mode 100644
index 0000000..86d7845
--- /dev/null
+++ b/lib/tracefs/Makefile
@@ -0,0 +1,46 @@
+
+
+include $(src)/scripts/utils.mk
+
+bdir:=$(obj)/lib/tracefs
+
+DEFAULT_TARGET = $(bdir)/libtracefs.a
+
+OBJS =
+OBJS += tracefs-utils.o
+
+OBJS := $(OBJS:%.o=$(bdir)/%.o)
+DEPS := $(OBJS:$(bdir)/%.o=$(bdir)/.%.d)
+
+all: $(DEFAULT_TARGET)
+
+$(bdir):
+	@mkdir -p $(bdir)
+
+$(OBJS): | $(bdir)
+$(DEPS): | $(bdir)
+
+$(bdir)/libtracefs.a: $(OBJS)
+	$(Q)$(call do_build_static_lib)
+
+$(bdir)/libtracefs.so: $(OBJS)
+	$(Q)$(call do_compile_shared_library)
+
+$(bdir)/%.o: %.c
+	$(Q)$(call do_fpic_compile)
+
+$(DEPS): $(bdir)/.%.d: %.c
+	$(Q)$(CC) -M $(CPPFLAGS) $(CFLAGS) $< > $@
+
+$(OBJS): $(bdir)/%.o : $(bdir)/.%.d
+
+dep_includes := $(wildcard $(DEPS))
+
+ifneq ($(dep_includes),)
+  include $(dep_includes)
+endif
+
+clean:
+	$(RM) $(bdir)/*.a $(bdir)/*.so $(bdir)/*.o $(bdir)/.*.d
+
+.PHONY: clean
diff --git a/lib/tracefs/include/tracefs-local.h b/lib/tracefs/include/tracefs-local.h
new file mode 100644
index 0000000..231edd1
--- /dev/null
+++ b/lib/tracefs/include/tracefs-local.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: LGPL-2.1 */
+/*
+ * Copyright (C) 2019, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
+ *
+ */
+#ifndef _TRACE_FS_LOCAL_H
+#define _TRACE_FS_LOCAL_H
+
+/* Can be overridden */
+void warning(const char *fmt, ...);
+
+#endif /* _TRACE_FS_LOCAL_H */
diff --git a/lib/tracefs/tracefs-utils.c b/lib/tracefs/tracefs-utils.c
new file mode 100644
index 0000000..0ef16fc
--- /dev/null
+++ b/lib/tracefs/tracefs-utils.c
@@ -0,0 +1,183 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Copyright (C) 2008, 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
+ *
+ * Updates:
+ * Copyright (C) 2019, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
+ *
+ */
+#include <stdlib.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+#include <linux/limits.h>
+
+#include "tracefs.h"
+
+#define TRACEFS_PATH "/sys/kernel/tracing"
+#define DEBUGFS_PATH "/sys/kernel/debug"
+
+#define _STR(x) #x
+#define STR(x) _STR(x)
+
+void __attribute__((weak)) warning(const char *fmt, ...)
+{
+}
+
+static int mount_tracefs(void)
+{
+	struct stat st;
+	int ret;
+
+	/* make sure debugfs exists */
+	ret = stat(TRACEFS_PATH, &st);
+	if (ret < 0)
+		return -1;
+
+	ret = mount("nodev", TRACEFS_PATH,
+		    "tracefs", 0, NULL);
+
+	return ret;
+}
+
+static int mount_debugfs(void)
+{
+	struct stat st;
+	int ret;
+
+	/* make sure debugfs exists */
+	ret = stat(DEBUGFS_PATH, &st);
+	if (ret < 0)
+		return -1;
+
+	ret = mount("nodev", DEBUGFS_PATH,
+		    "debugfs", 0, NULL);
+
+	return ret;
+}
+
+/**
+ * tracefs_find_tracing_dir - Find tracing directory
+ *
+ * Returns string containing the full path to the system's tracing directory.
+ * The string must be freed by free()
+ */
+char *tracefs_find_tracing_dir(void)
+{
+	char *debug_str = NULL;
+	char fspath[PATH_MAX+1];
+	char *tracing_dir;
+	char type[100];
+	int use_debug = 0;
+	FILE *fp;
+
+	fp = fopen("/proc/mounts", "r");
+	if (!fp) {
+		warning("Can't open /proc/mounts for read");
+		return NULL;
+	}
+
+	while (fscanf(fp, "%*s %"
+		      STR(PATH_MAX)
+		      "s %99s %*s %*d %*d\n",
+		      fspath, type) == 2) {
+		if (strcmp(type, "tracefs") == 0)
+			break;
+		if (!debug_str && strcmp(type, "debugfs") == 0) {
+			debug_str = strdup(fspath);
+			if (!debug_str) {
+				fclose(fp);
+				return NULL;
+			}
+		}
+	}
+	fclose(fp);
+
+	if (strcmp(type, "tracefs") != 0) {
+		if (mount_tracefs() < 0) {
+			if (debug_str) {
+				strncpy(fspath, debug_str, PATH_MAX);
+				fspath[PATH_MAX] = 0;
+			} else {
+				if (mount_debugfs() < 0) {
+					warning("debugfs not mounted, please mount");
+					free(debug_str);
+					return NULL;
+				}
+				strcpy(fspath, DEBUGFS_PATH);
+			}
+			use_debug = 1;
+		} else
+			strcpy(fspath, TRACEFS_PATH);
+	}
+	free(debug_str);
+
+	if (use_debug) {
+		int ret;
+
+		ret = asprintf(&tracing_dir, "%s/tracing", fspath);
+		if (ret < 0)
+			return NULL;
+	} else {
+		tracing_dir = strdup(fspath);
+		if (!tracing_dir)
+			return NULL;
+	}
+
+	return tracing_dir;
+}
+
+/**
+ * tracefs_get_tracing_dir - Get tracing directory
+ *
+ * Returns string containing the full path to the system's tracing directory.
+ * Must use tracefs_put_tracing_file() to free the returned string.
+ */
+const char *tracefs_get_tracing_dir(void)
+{
+	static const char *tracing_dir;
+
+	if (tracing_dir)
+		return tracing_dir;
+
+	tracing_dir = tracefs_find_tracing_dir();
+	return tracing_dir;
+}
+
+/**
+ * tracefs_get_tracing_file - Get tracing file
+ * @name: tracing file name
+ *
+ * Returns string containing the full path to a tracing file in
+ * the system's tracing directory.
+ *
+ * Must use tracefs_put_tracing_file() to free the returned string.
+ */
+char *tracefs_get_tracing_file(const char *name)
+{
+	static const char *tracing;
+	char *file;
+	int ret;
+
+	if (!tracing) {
+		tracing = tracefs_find_tracing_dir();
+		if (!tracing)
+			return NULL;
+	}
+
+	ret = asprintf(&file, "%s/%s", tracing, name);
+	if (ret < 0)
+		return NULL;
+
+	return file;
+}
+
+/**
+ * tracefs_put_tracing_file - Free tracing file or directory name
+ *
+ * Frees tracing file or directory, returned by
+ * tracefs_get_tracing_file() or tracefs_get_tracing_dir() APIs
+ */
+void tracefs_put_tracing_file(char *name)
+{
+	free(name);
+}
diff --git a/tracecmd/Makefile b/tracecmd/Makefile
index 29a623b..d00d8e0 100644
--- a/tracecmd/Makefile
+++ b/tracecmd/Makefile
@@ -62,7 +62,7 @@ $(all_objs): | $(bdir)
 $(bdir)/trace-cmd: $(ALL_OBJS)
 	$(Q)$(do_app_build)
 
-$(bdir)/trace-cmd: $(LIBTRACECMD_STATIC) $(LIBTRACEEVENT_STATIC)
+$(bdir)/trace-cmd: $(LIBTRACECMD_STATIC) $(LIBTRACEEVENT_STATIC) $(LIBTRACEFS_STATIC)
 
 $(bdir)/%.o: %.c
 	$(Q)$(call do_compile)
diff --git a/tracecmd/trace-check-events.c b/tracecmd/trace-check-events.c
index b09fcd0..a8ee60a 100644
--- a/tracecmd/trace-check-events.c
+++ b/tracecmd/trace-check-events.c
@@ -7,6 +7,7 @@
 #include <getopt.h>
 #include <errno.h>
 
+#include "tracefs.h"
 #include "trace-local.h"
 
 void trace_check_events(int argc, char **argv)
@@ -28,7 +29,7 @@ void trace_check_events(int argc, char **argv)
 			break;
 		}
 	}
-	tracing = tracecmd_get_tracing_dir();
+	tracing = tracefs_get_tracing_dir();
 
 	if (!tracing) {
 		printf("Can not find or mount tracing directory!\n"
diff --git a/tracecmd/trace-list.c b/tracecmd/trace-list.c
index 65099a5..86e3358 100644
--- a/tracecmd/trace-list.c
+++ b/tracecmd/trace-list.c
@@ -6,6 +6,7 @@
 
 #include <stdlib.h>
 
+#include "tracefs.h"
 #include "trace-local.h"
 
 
@@ -35,7 +36,7 @@ void show_instance_file(struct buffer_instance *instance, const char *name)
 
 	path = get_instance_file(instance, name);
 	dump_file_content(path);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 }
 
 enum {
@@ -49,9 +50,9 @@ void show_file(const char *name)
 {
 	char *path;
 
-	path = tracecmd_get_tracing_file(name);
+	path = tracefs_get_tracing_file(name);
 	dump_file_content(path);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 }
 
 typedef int (*process_file_func)(char *buf, int len);
@@ -86,11 +87,11 @@ static void process_file_re(process_file_func func,
 
 	free(str);
 
-	path = tracecmd_get_tracing_file(name);
+	path = tracefs_get_tracing_file(name);
 	fp = fopen(path, "r");
 	if (!fp)
 		die("reading %s", path);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	do {
 		n = getline(&buf, &l, fp);
@@ -132,12 +133,12 @@ static char *get_event_file(const char *type, char *buf, int len)
 	if (!event)
 		die("no event found in %s\n", buf);
 
-	path = tracecmd_get_tracing_file("events");
+	path = tracefs_get_tracing_file("events");
 	ret = asprintf(&file, "%s/%s/%s/%s", path, system, event, type);
 	if (ret < 0)
 		die("Failed to allocate event file %s %s", system, event);
 
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	return file;
 }
@@ -282,9 +283,9 @@ static void show_buffers(void)
 	char *path;
 	int printed = 0;
 
-	path = tracecmd_get_tracing_file("instances");
+	path = tracefs_get_tracing_file("instances");
 	dir = opendir(path);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	if (!dir)
 		die("Can not read instance directory");
 
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 80b2234..5355813 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -39,6 +39,7 @@
 #include <linux/vm_sockets.h>
 #endif
 
+#include "tracefs.h"
 #include "version.h"
 #include "trace-local.h"
 #include "trace-msg.h"
@@ -337,21 +338,21 @@ static void test_set_event_pid(void)
 	if (tested)
 		return;
 
-	path = tracecmd_get_tracing_file("set_event_pid");
+	path = tracefs_get_tracing_file("set_event_pid");
 	ret = stat(path, &st);
 	if (!ret) {
 		have_set_event_pid = 1;
 		reset_save_file(path, RESET_DEFAULT_PRIO);
 	}
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
-	path = tracecmd_get_tracing_file("options/event-fork");
+	path = tracefs_get_tracing_file("options/event-fork");
 	ret = stat(path, &st);
 	if (!ret) {
 		have_event_fork = 1;
 		reset_save_file(path, RESET_DEFAULT_PRIO);
 	}
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	tested = 1;
 }
@@ -442,13 +443,13 @@ static int __add_all_instances(const char *tracing_dir)
  */
 void add_all_instances(void)
 {
-	char *tracing_dir = tracecmd_find_tracing_dir();
+	char *tracing_dir = tracefs_find_tracing_dir();
 	if (!tracing_dir)
 		die("malloc");
 
 	__add_all_instances(tracing_dir);
 
-	tracecmd_put_tracing_file(tracing_dir);
+	tracefs_put_tracing_file(tracing_dir);
 }
 
 /**
@@ -474,7 +475,7 @@ void tracecmd_stat_cpu_instance(struct buffer_instance *instance,
 	path = get_instance_file(instance, file);
 	free(file);
 	fd = open(path, O_RDONLY);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	if (fd < 0)
 		return;
 
@@ -772,9 +773,9 @@ static int set_ftrace(int set, int use_proc)
 	int ret;
 
 	/* First check if the function-trace option exists */
-	path = tracecmd_get_tracing_file("options/function-trace");
+	path = tracefs_get_tracing_file("options/function-trace");
 	ret = set_ftrace_enable(path, set);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	/* Always enable ftrace_enable proc file when set is true */
 	if (ret < 0 || set || use_proc)
@@ -790,7 +791,7 @@ static int set_ftrace(int set, int use_proc)
  *
  * Returns the path name of the @file for the given @instance.
  *
- * Must use tracecmd_put_tracing_file() to free the returned string.
+ * Must use tracefs_put_tracing_file() to free the returned string.
  */
 char *
 get_instance_file(struct buffer_instance *instance, const char *file)
@@ -803,10 +804,10 @@ get_instance_file(struct buffer_instance *instance, const char *file)
 		ret = asprintf(&buf, "instances/%s/%s", instance->name, file);
 		if (ret < 0)
 			die("Failed to allocate name for %s/%s", instance->name, file);
-		path = tracecmd_get_tracing_file(buf);
+		path = tracefs_get_tracing_file(buf);
 		free(buf);
 	} else
-		path = tracecmd_get_tracing_file(file);
+		path = tracefs_get_tracing_file(file);
 
 	return path;
 }
@@ -825,7 +826,7 @@ get_instance_dir(struct buffer_instance *instance)
 	ret = asprintf(&buf, "instances/%s", instance->name);
 	if (ret < 0)
 		die("Failed to allocate for instance %s", instance->name);
-	path = tracecmd_get_tracing_file(buf);
+	path = tracefs_get_tracing_file(buf);
 	free(buf);
 
 	return path;
@@ -868,7 +869,7 @@ write_instance_file(struct buffer_instance *instance,
 	ret = stat(path, &st);
 	if (ret == 0)
 		ret = write_file(path, str, type);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	return ret;
 }
@@ -886,7 +887,7 @@ static void __clear_trace(struct buffer_instance *instance)
 	fp = fopen(path, "w");
 	if (!fp)
 		die("writing to '%s'", path);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	fwrite("0", 1, 1, fp);
 	fclose(fp);
 }
@@ -905,11 +906,11 @@ static void clear_trace(void)
 	char *path;
 
 	/* reset the trace */
-	path = tracecmd_get_tracing_file("trace");
+	path = tracefs_get_tracing_file("trace");
 	fp = fopen(path, "w");
 	if (!fp)
 		die("writing to '%s'", path);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	fwrite("0", 1, 1, fp);
 	fclose(fp);
 }
@@ -956,7 +957,7 @@ static void update_ftrace_pid(const char *pid, int reset)
 		if (fd >= 0)
 			close(fd);
 		if (path)
-			tracecmd_put_tracing_file(path);
+			tracefs_put_tracing_file(path);
 		fd = -1;
 		path = NULL;
 		return;
@@ -970,7 +971,7 @@ static void update_ftrace_pid(const char *pid, int reset)
 
 	if (fd < 0) {
 		if (!path)
-			path = tracecmd_get_tracing_file("set_ftrace_pid");
+			path = tracefs_get_tracing_file("set_ftrace_pid");
 		if (!path)
 			return;
 		ret = stat(path, &st);
@@ -1601,12 +1602,12 @@ set_plugin_instance(struct buffer_instance *instance, const char *name)
 		 * plugin for those if name is "nop".
 		 */
 		if (!strncmp(name, "nop", 3)) {
-			tracecmd_put_tracing_file(path);
+			tracefs_put_tracing_file(path);
 			return;
 		}
 		die("writing to '%s'", path);
 	}
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	fwrite(name, 1, strlen(name), fp);
 	fclose(fp);
@@ -1619,11 +1620,11 @@ set_plugin_instance(struct buffer_instance *instance, const char *name)
 	path = get_instance_file(instance, "options/func_stack_trace");
 	fp = fopen(path, "w");
 	if (!fp) {
-		tracecmd_put_tracing_file(path);
-		path = tracecmd_get_tracing_file("options/func_stack_trace");
+		tracefs_put_tracing_file(path);
+		path = tracefs_get_tracing_file("options/func_stack_trace");
 		fp = fopen(path, "w");
 		if (!fp) {
-			tracecmd_put_tracing_file(path);
+			tracefs_put_tracing_file(path);
 			return;
 		}
 	}
@@ -1632,7 +1633,7 @@ set_plugin_instance(struct buffer_instance *instance, const char *name)
 	 * the original content.
 	 */
 	add_reset_file(path, "0", RESET_HIGH_PRIO);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	fwrite(&zero, 1, 1, fp);
 	fclose(fp);
 }
@@ -1662,11 +1663,11 @@ static int set_option(const char *option)
 	FILE *fp;
 	char *path;
 
-	path = tracecmd_get_tracing_file("trace_options");
+	path = tracefs_get_tracing_file("trace_options");
 	fp = fopen(path, "w");
 	if (!fp)
 		warning("writing to '%s'", path);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	if (!fp)
 		return -1;
@@ -1693,7 +1694,7 @@ static void disable_func_stack_trace_instance(struct buffer_instance *instance)
 
 	path = get_instance_file(instance, "current_tracer");
 	ret = stat(path, &st);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	if (ret < 0)
 		return;
 
@@ -1727,7 +1728,7 @@ static void add_reset_options(void)
 	if (keep)
 		return;
 
-	path = tracecmd_get_tracing_file("trace_options");
+	path = tracefs_get_tracing_file("trace_options");
 	content = get_file_content(path);
 
 	for (opt = options; opt; opt = opt->next) {
@@ -1790,7 +1791,7 @@ static void add_reset_options(void)
 
 		add_reset_file(path, option, RESET_DEFAULT_PRIO);
 	}
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	free(content);
 }
 
@@ -1819,14 +1820,14 @@ static void set_saved_cmdlines_size(struct common_record_context *ctx)
 	if (!ctx->saved_cmdlines_size)
 		return;
 
-	path = tracecmd_get_tracing_file("saved_cmdlines_size");
+	path = tracefs_get_tracing_file("saved_cmdlines_size");
 	if (!path)
 		goto err;
 
 	reset_save_file(path, RESET_DEFAULT_PRIO);
 
 	fd = open(path, O_WRONLY);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	if (fd < 0)
 		goto err;
 
@@ -1852,7 +1853,7 @@ static int trace_check_file_exists(struct buffer_instance *instance, char *file)
 
 	path = get_instance_file(instance, file);
 	ret = stat(path, &st);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	return ret < 0 ? 0 : 1;
 }
@@ -1884,11 +1885,11 @@ static void old_update_events(const char *name, char update)
 		name = "*:*";
 
 	/* need to use old way */
-	path = tracecmd_get_tracing_file("set_event");
+	path = tracefs_get_tracing_file("set_event");
 	fp = fopen(path, "w");
 	if (!fp)
 		die("opening '%s'", path);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	/* Disable the event with "!" */
 	if (update == '0')
@@ -1935,12 +1936,12 @@ reset_events_instance(struct buffer_instance *instance)
 		die("opening to '%s'", path);
 	ret = write(fd, &c, 1);
 	close(fd);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	path = get_instance_file(instance, "events/*/filter");
 	globbuf.gl_offs = 0;
 	ret = glob(path, 0, NULL, &globbuf);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	if (ret < 0)
 		return;
 
@@ -2182,9 +2183,9 @@ static void check_tracing_enabled(void)
 	char *path;
 
 	if (fd < 0) {
-		path = tracecmd_get_tracing_file("tracing_enabled");
+		path = tracefs_get_tracing_file("tracing_enabled");
 		fd = open(path, O_WRONLY | O_CLOEXEC);
-		tracecmd_put_tracing_file(path);
+		tracefs_put_tracing_file(path);
 
 		if (fd < 0)
 			return;
@@ -2205,7 +2206,7 @@ static int open_instance_fd(struct buffer_instance *instance,
 		if (is_top_instance(instance))
 			die("opening '%s'", path);
 	}
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	return fd;
 }
@@ -2526,7 +2527,7 @@ static void set_mask(struct buffer_instance *instance)
 
 	close(fd);
  out:
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	free(instance->cpumask);
 	instance->cpumask = NULL;
 }
@@ -2583,7 +2584,7 @@ static void set_clock(struct buffer_instance *instance)
 	add_reset_file(path, str, RESET_DEFAULT_PRIO);
 
 	free(content);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	write_instance_file(instance, "trace_clock", instance->clock, "clock");
 }
@@ -2598,7 +2599,7 @@ static void set_max_graph_depth(struct buffer_instance *instance, char *max_grap
 
 	path = get_instance_file(instance, "max_graph_depth");
 	reset_save_file(path, RESET_DEFAULT_PRIO);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	ret = write_instance_file(instance, "max_graph_depth", max_graph_depth,
 				  NULL);
 	if (ret < 0)
@@ -2721,7 +2722,7 @@ static int expand_event_files(struct buffer_instance *instance,
 
 	globbuf.gl_offs = 0;
 	ret = glob(path, 0, NULL, &globbuf);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	free(p);
 
 	if (ret < 0)
@@ -3182,7 +3183,7 @@ create_recorder_instance_pipe(struct buffer_instance *instance,
 	if (instance->name)
 		path = get_instance_dir(instance);
 	else
-		path = tracecmd_find_tracing_dir();
+		path = tracefs_find_tracing_dir();
 
 	if (!path)
 		die("malloc");
@@ -3193,7 +3194,7 @@ create_recorder_instance_pipe(struct buffer_instance *instance,
 	recorder = tracecmd_create_buffer_recorder_fd(brass[1], cpu, flags, path);
 
 	if (instance->name)
-		tracecmd_put_tracing_file(path);
+		tracefs_put_tracing_file(path);
 
 	return recorder;
 }
@@ -3234,7 +3235,7 @@ create_recorder_instance(struct buffer_instance *instance, const char *file, int
 
 	record = tracecmd_create_buffer_recorder_maxkb(file, cpu, recorder_flags,
 						       path, max_kb);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	return record;
 }
@@ -3287,9 +3288,9 @@ static int create_recorder(struct buffer_instance *instance, int cpu,
 		if (instance->name && !is_agent(instance))
 			path = get_instance_dir(instance);
 		else
-			path = tracecmd_find_tracing_dir();
+			path = tracefs_find_tracing_dir();
 		recorder = tracecmd_create_buffer_recorder_fd(fd, cpu, flags, path);
-		tracecmd_put_tracing_file(path);
+		tracefs_put_tracing_file(path);
 	} else {
 		file = get_temp_file(instance, cpu);
 		recorder = create_recorder_instance(instance, file, cpu, brass);
@@ -4124,7 +4125,7 @@ static int write_func_file(struct buffer_instance *instance,
 	close(fd);
 	ret = 0;
  free:
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	return ret;
  failed:
 	die("Failed to write %s to %s.\n"
@@ -4142,7 +4143,7 @@ static int functions_filtered(struct buffer_instance *instance)
 
 	path = get_instance_file(instance, "set_ftrace_filter");
 	fd = open(path, O_RDONLY);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	if (fd < 0) {
 		if (is_top_instance(instance))
 			warning("Can not set set_ftrace_filter");
@@ -4265,7 +4266,7 @@ static unsigned long long find_time_stamp(struct tep_handle *pevent)
 	int fd;
 	int r;
 
-	path = tracecmd_get_tracing_file("per_cpu");
+	path = tracefs_get_tracing_file("per_cpu");
 	if (!path)
 		return 0;
 
@@ -4304,7 +4305,7 @@ static unsigned long long find_time_stamp(struct tep_handle *pevent)
 	closedir(dir);
 
  out:
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	return ts;
 }
 
@@ -4319,7 +4320,7 @@ static char *read_instance_file(struct buffer_instance *instance, char *file, in
 
 	path = get_instance_file(instance, file);
 	fd = open(path, O_RDONLY);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	if (fd < 0) {
 		warning("%s not found, --date ignored", file);
 		return NULL;
@@ -4400,9 +4401,9 @@ static char *get_date_to_ts(void)
 		goto out_pevent;
 	}
 
-	path = tracecmd_get_tracing_file("trace_marker");
+	path = tracefs_get_tracing_file("trace_marker");
 	tfd = open(path, O_WRONLY);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	if (tfd < 0) {
 		warning("Can not open 'trace_marker', --date ignored");
 		goto out_pevent;
@@ -4491,7 +4492,7 @@ static void set_buffer_size_instance(struct buffer_instance *instance)
 		warning("Can't write to %s", path);
 	close(fd);
  out:
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 }
 
 void set_buffer_size(void)
@@ -4560,7 +4561,7 @@ static void clear_instance_triggers(struct buffer_instance *instance)
 
 	trace_event_iter_free(iter);
 
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 }
 
 static void
@@ -4621,7 +4622,7 @@ static void clear_instance_filters(struct buffer_instance *instance)
 
 	trace_event_iter_free(iter);
 
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 }
 
 static void clear_filters(void)
@@ -4687,7 +4688,7 @@ static void clear_func_filters(void)
 		for (i = 0; files[i]; i++) {
 			path = get_instance_file(instance, files[i]);
 			clear_func_filter(path);
-			tracecmd_put_tracing_file(path);
+			tracefs_put_tracing_file(path);
 		}
 	}
 }
@@ -4712,7 +4713,7 @@ static void make_instances(void)
 		} else
 			/* Don't delete instances that already exist */
 			instance->flags |= BUFFER_FL_KEEP;
-		tracecmd_put_tracing_file(path);
+		tracefs_put_tracing_file(path);
 	}
 }
 
@@ -4734,7 +4735,7 @@ void tracecmd_remove_instances(void)
 		ret = rmdir(path);
 		if (ret < 0)
 			die("rmdir %s", path);
-		tracecmd_put_tracing_file(path);
+		tracefs_put_tracing_file(path);
 	}
 }
 
@@ -5043,7 +5044,7 @@ static int test_stacktrace_trigger(struct buffer_instance *instance)
 		ret = 1;
 	close(fd);
  out:
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	return ret;
 }
diff --git a/tracecmd/trace-show.c b/tracecmd/trace-show.c
index 96bfe77..391d329 100644
--- a/tracecmd/trace-show.c
+++ b/tracecmd/trace-show.c
@@ -7,6 +7,7 @@
 #include <getopt.h>
 #include <errno.h>
 
+#include "tracefs.h"
 #include "trace-local.h"
 
 enum {
@@ -157,9 +158,9 @@ void trace_show(int argc, char **argv)
 
 	if (show_name) {
 		char *name;
-		name = tracecmd_get_tracing_file(file);
+		name = tracefs_get_tracing_file(file);
 		printf("%s\n", name);
-		tracecmd_put_tracing_file(name);
+		tracefs_put_tracing_file(name);
 	}
 	show_file(file);
 	if (buffer)
diff --git a/tracecmd/trace-snapshot.c b/tracecmd/trace-snapshot.c
index a9a512a..34630b4 100644
--- a/tracecmd/trace-snapshot.c
+++ b/tracecmd/trace-snapshot.c
@@ -12,6 +12,7 @@
 #include <fcntl.h>
 #include <unistd.h>
 
+#include "tracefs.h"
 #include "trace-local.h"
 
 static void write_file(const char *name, char *val)
@@ -20,7 +21,7 @@ static void write_file(const char *name, char *val)
 	int fd;
 	ssize_t n;
 
-	path = tracecmd_get_tracing_file(name);
+	path = tracefs_get_tracing_file(name);
 	fd = open(path, O_WRONLY);
 	if (fd < 0)
 		die("writing %s", path);
@@ -29,7 +30,7 @@ static void write_file(const char *name, char *val)
 	if (n < 0)
 		die("failed to write to %s\n", path);
 
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	close(fd);
 }
 
@@ -91,11 +92,11 @@ void trace_snapshot (int argc, char **argv)
 		file = cpu_path;
 	}
 
-	name = tracecmd_get_tracing_file(file);
+	name = tracefs_get_tracing_file(file);
 	ret = stat(name, &st);
 	if (ret < 0)
 		die("Snapshot feature is not supported by this kernel");
-	tracecmd_put_tracing_file(name);
+	tracefs_put_tracing_file(name);
 
 	if (!reset_snap && !take_snap && !free_snap) {
 		show_file(file);
diff --git a/tracecmd/trace-stack.c b/tracecmd/trace-stack.c
index bb002c0..5e88b36 100644
--- a/tracecmd/trace-stack.c
+++ b/tracecmd/trace-stack.c
@@ -15,6 +15,7 @@
 #include <unistd.h>
 #include <errno.h>
 
+#include "tracefs.h"
 #include "trace-local.h"
 
 #define PROC_FILE "/proc/sys/kernel/stack_tracer_enabled"
@@ -86,7 +87,7 @@ static void reset_trace(void)
 	int fd;
 	int n;
 
-	path = tracecmd_get_tracing_file("stack_max_size");
+	path = tracefs_get_tracing_file("stack_max_size");
 	fd = open(path, O_WRONLY);
 	if (fd < 0)
 		die("writing %s", path);
@@ -95,7 +96,7 @@ static void reset_trace(void)
 	n = write(fd, buf, 1);
 	if (n < 0)
 		die("writing into %s", path);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 	close(fd);
 }
 
@@ -116,11 +117,11 @@ static void read_trace(void)
 	else
 		printf("(stack tracer not running)\n");
 
-	path = tracecmd_get_tracing_file("stack_trace");
+	path = tracefs_get_tracing_file("stack_trace");
 	fp = fopen(path, "r");
 	if (!fp)
 		die("reading to '%s'", path);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	while ((r = getline(&buf, &n, fp)) >= 0) {
 		/*
diff --git a/tracecmd/trace-stat.c b/tracecmd/trace-stat.c
index 538f4ad..7a1d9bb 100644
--- a/tracecmd/trace-stat.c
+++ b/tracecmd/trace-stat.c
@@ -13,6 +13,7 @@
 #include <fcntl.h>
 #include <ctype.h>
 
+#include "tracefs.h"
 #include "trace-local.h"
 
 #ifndef BUFSIZ
@@ -32,7 +33,7 @@ static int get_instance_file_fd(struct buffer_instance *instance,
 
 	path = get_instance_file(instance, file);
 	fd = open(path, O_RDONLY);
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	return fd;
 }
@@ -383,7 +384,7 @@ static void report_events(struct buffer_instance *instance)
 	if (!processed && !processed_part)
 		printf("  (none enabled)\n");
 
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 }
 
 static void
@@ -456,7 +457,7 @@ static void report_event_filters(struct buffer_instance *instance)
 
 	trace_event_iter_free(iter);
 
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 }
 
 static void
@@ -529,7 +530,7 @@ static void report_event_triggers(struct buffer_instance *instance)
 
 	trace_event_iter_free(iter);
 
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 }
 
 enum func_states {
@@ -604,7 +605,7 @@ static void report_graph_funcs(struct buffer_instance *instance)
 
 	list_functions(path, "Function Graph Filter");
 	
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	path = get_instance_file(instance, "set_graph_notrace");
 	if (!path)
@@ -612,7 +613,7 @@ static void report_graph_funcs(struct buffer_instance *instance)
 
 	list_functions(path, "Function Graph No Trace");
 	
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 }
 
 static void report_ftrace_filters(struct buffer_instance *instance)
@@ -625,7 +626,7 @@ static void report_ftrace_filters(struct buffer_instance *instance)
 
 	list_functions(path, "Function Filter");
 	
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	path = get_instance_file(instance, "set_ftrace_notrace");
 	if (!path)
@@ -633,7 +634,7 @@ static void report_ftrace_filters(struct buffer_instance *instance)
 
 	list_functions(path, "Function No Trace");
 	
-	tracecmd_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 }
 
 static void report_buffers(struct buffer_instance *instance)
-- 
2.24.1


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

* [PATCH v2 2/5] kernel-shark: Use new tracefs library
  2020-01-06 15:40 [PATCH v2 0/5] tracefs library Tzvetomir Stoyanov (VMware)
  2020-01-06 15:40 ` [PATCH v2 1/5] trace-cmd: Introduce libtracefs library Tzvetomir Stoyanov (VMware)
@ 2020-01-06 15:40 ` Tzvetomir Stoyanov (VMware)
  2020-01-08 19:05   ` Steven Rostedt
  2020-01-08 20:09   ` Steven Rostedt
  2020-01-06 15:40 ` [PATCH v2 3/5] trace-cmd: New libtracefs APIs for ftrace instances Tzvetomir Stoyanov (VMware)
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-01-06 15:40 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Link libtracefs to kernel-shark and use its API:
  tracefs_get_tracing_dir()

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 kernel-shark/CMakeLists.txt           |  3 ++-
 kernel-shark/build/FindTraceCmd.cmake | 30 +++++++++++++++++++++++++++
 kernel-shark/src/CMakeLists.txt       |  2 ++
 kernel-shark/src/KsCaptureDialog.cpp  |  4 ++--
 kernel-shark/src/libkshark.h          |  1 +
 5 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/kernel-shark/CMakeLists.txt b/kernel-shark/CMakeLists.txt
index 20478b9..8786b83 100644
--- a/kernel-shark/CMakeLists.txt
+++ b/kernel-shark/CMakeLists.txt
@@ -76,7 +76,8 @@ endif (CMAKE_BUILD_TYPE MATCHES Package)
 include_directories(${KS_DIR}/src/
                     ${KS_DIR}/build/src/
                     ${JSONC_INCLUDE_DIR}
-                    ${TRACECMD_INCLUDE_DIR})
+                    ${TRACECMD_INCLUDE_DIR}
+                    ${TRACEFS_INCLUDE_DIR})
 
 message("")
 message(STATUS "C flags      : " ${CMAKE_C_FLAGS})
diff --git a/kernel-shark/build/FindTraceCmd.cmake b/kernel-shark/build/FindTraceCmd.cmake
index d3e145c..f27fafe 100644
--- a/kernel-shark/build/FindTraceCmd.cmake
+++ b/kernel-shark/build/FindTraceCmd.cmake
@@ -6,6 +6,8 @@
 #  TRACEEVENT_FOUND, If false, do not try to use traceevent.
 #
 #  TRACECMD_INCLUDE_DIR, where to find trace-cmd header.
+#  TRACEFS_INCLUDE_DIR, where to find tracefs header.
+#  TRACEFS_LIBRARY, the tracefs library.
 #  TRACECMD_LIBRARY, the trace-cmd library.
 #  TRACECMD_FOUND, If false, do not try to use trace-cmd.
 
@@ -31,12 +33,21 @@ find_path(TRACECMD_INCLUDE_DIR  NAMES  trace-cmd/trace-cmd.h
                                 PATHS  $ENV{TRACE_CMD}/include/
                                        ${CMAKE_SOURCE_DIR}/../include/
                                 NO_DEFAULT_PATH)
+find_path(TRACEFS_INCLUDE_DIR   NAMES  tracefs/tracefs.h
+                                PATHS  $ENV{TRACE_CMD}/include/
+                                       ${CMAKE_SOURCE_DIR}/../include/
+                                NO_DEFAULT_PATH)
 
 find_library(TRACECMD_LIBRARY   NAMES  trace-cmd/libtracecmd.a
                                 PATHS  $ENV{TRACE_CMD}/lib/
                                        ${CMAKE_SOURCE_DIR}/../lib/
                                 NO_DEFAULT_PATH)
 
+find_library(TRACEFS_LIBRARY    NAMES  trace-cmd/libtracefs.a
+                                PATHS  $ENV{TRACE_CMD}/lib/
+                                       ${CMAKE_SOURCE_DIR}/../lib/
+                                NO_DEFAULT_PATH)
+
 find_library(TRACEEVENT_LIBRARY NAMES  traceevent/libtraceevent.a
                                 PATHS  $ENV{TRACE_CMD}/lib/
                                        ${CMAKE_SOURCE_DIR}/../lib/
@@ -46,7 +57,9 @@ find_library(TRACEEVENT_LIBRARY NAMES  traceevent/libtraceevent.a
 # search was successful "find_path" will do nothing this time.
 find_program(TRACECMD_EXECUTABLE   NAMES  trace-cmd)
 find_path(TRACECMD_INCLUDE_DIR  NAMES  trace-cmd/trace-cmd.h)
+find_path(TRACEFS_INCLUDE_DIR   NAMES  tracefs/tracefs.h)
 find_library(TRACECMD_LIBRARY   NAMES  trace-cmd/libtracecmd.so)
+find_library(TRACEFS_LIBRARY    NAMES  tracefs/libtracefs.so)
 find_library(TRACEEVENT_LIBRARY NAMES  traceevent/libtraceevent.so)
 
 IF (TRACECMD_INCLUDE_DIR AND TRACECMD_LIBRARY)
@@ -65,6 +78,23 @@ ELSE (TRACECMD_FOUND)
 
 ENDIF (TRACECMD_FOUND)
 
+IF (TRACEFS_INCLUDE_DIR AND TRACEFS_LIBRARY)
+
+  SET(TRACEFS_FOUND TRUE)
+
+ENDIF (TRACEFS_INCLUDE_DIR AND TRACEFS_LIBRARY)
+
+IF (TRACEFS_FOUND)
+
+  MESSAGE(STATUS "Found tracefs: ${TRACEFS_LIBRARY}")
+
+ELSE (TRACEFS_FOUND)
+
+  MESSAGE(FATAL_ERROR "\nCould not find tracefs!\n")
+
+ENDIF (TRACEFS_FOUND)
+
+
 IF (TRACEEVENT_LIBRARY)
 
   SET(TRACEEVENT_FOUND TRUE)
diff --git a/kernel-shark/src/CMakeLists.txt b/kernel-shark/src/CMakeLists.txt
index e20a030..33b5db8 100644
--- a/kernel-shark/src/CMakeLists.txt
+++ b/kernel-shark/src/CMakeLists.txt
@@ -9,6 +9,7 @@ add_library(kshark SHARED libkshark.c
 
 target_link_libraries(kshark ${TRACEEVENT_LIBRARY}
                              ${TRACECMD_LIBRARY}
+                             ${TRACEFS_LIBRARY}
                              ${JSONC_LIBRARY}
                              ${CMAKE_DL_LIBS})
 
@@ -69,6 +70,7 @@ if (Qt5Widgets_FOUND AND Qt5Network_FOUND)
                                      Qt5::Network
                                      ${TRACEEVENT_LIBRARY}
                                      ${TRACECMD_LIBRARY}
+                                     ${TRACEFS_LIBRARY}
                                      ${CMAKE_DL_LIBS})
 
     set_target_properties(kshark-gui PROPERTIES  SUFFIX ".so.${KS_VERSION_STRING}")
diff --git a/kernel-shark/src/KsCaptureDialog.cpp b/kernel-shark/src/KsCaptureDialog.cpp
index ad05917..548b6fb 100644
--- a/kernel-shark/src/KsCaptureDialog.cpp
+++ b/kernel-shark/src/KsCaptureDialog.cpp
@@ -26,7 +26,7 @@ extern "C" {
 
 static inline tep_handle *local_events()
 {
-	return tracecmd_local_events(tracecmd_get_tracing_dir());
+	return tracecmd_local_events(tracefs_get_tracing_dir());
 }
 
 /**
@@ -204,7 +204,7 @@ QStringList KsCaptureControl::_getPlugins()
 	QStringList pluginList;
 	char **all_plugins;
 
-	all_plugins = tracecmd_local_plugins(tracecmd_get_tracing_dir());
+	all_plugins = tracecmd_local_plugins(tracefs_get_tracing_dir());
 
 	if (!all_plugins)
 		return pluginList;
diff --git a/kernel-shark/src/libkshark.h b/kernel-shark/src/libkshark.h
index 3407db1..b05aa90 100644
--- a/kernel-shark/src/libkshark.h
+++ b/kernel-shark/src/libkshark.h
@@ -28,6 +28,7 @@ extern "C" {
 #include "trace-cmd/trace-cmd.h"
 #include "trace-cmd/trace-filter-hash.h"
 #include "traceevent/event-parse.h"
+#include "tracefs/tracefs.h"
 
 // KernelShark
 #include "libkshark-plugin.h"
-- 
2.24.1


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

* [PATCH v2 3/5] trace-cmd: New libtracefs APIs for ftrace instances
  2020-01-06 15:40 [PATCH v2 0/5] tracefs library Tzvetomir Stoyanov (VMware)
  2020-01-06 15:40 ` [PATCH v2 1/5] trace-cmd: Introduce libtracefs library Tzvetomir Stoyanov (VMware)
  2020-01-06 15:40 ` [PATCH v2 2/5] kernel-shark: Use new tracefs library Tzvetomir Stoyanov (VMware)
@ 2020-01-06 15:40 ` Tzvetomir Stoyanov (VMware)
  2020-01-08 19:37   ` Steven Rostedt
  2020-01-06 15:40 ` [PATCH v2 4/5] trace-cmd,kernel-shark: New libtracefs APIs for ftrace events and systems Tzvetomir Stoyanov (VMware)
  2020-01-06 15:40 ` [PATCH v2 5/5] trace-cmd,kernel-shark: New libtracefs APIs for loading ftrace events Tzvetomir Stoyanov (VMware)
  4 siblings, 1 reply; 15+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-01-06 15:40 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

The functionality related to ftrace instances is moved from
trace-cmd application to libtracefs. The following new
library APIs are introduced:
    struct tracefs_instance;
    struct tracefs_instance *tracefs_alloc_instance(const char *name);
    int tracefs_free_instance(struct tracefs_instance *instance);
    int tracefs_make_instance(struct tracefs_instance *instance);
    int tracefs_remove_instance(struct tracefs_instance *instance);
    char *tracefs_get_instance_name(struct tracefs_instance *instance);
    char *tracefs_get_instance_file(struct tracefs_instance *instance, const char *file);
    char *tracefs_get_instance_dir(struct tracefs_instance *instance);
    int tracefs_write_instance_file(struct tracefs_instance *instance,
				 const char *file, const char *str, const char *type);
    char *tracefs_read_instance_file(struct tracefs_instance *instance, char *file, int *psize);

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/tracefs/tracefs.h           |  17 ++
 lib/tracefs/Makefile                |   1 +
 lib/tracefs/include/tracefs-local.h |   1 +
 lib/tracefs/tracefs-instance.c      | 267 ++++++++++++++++++++++++++++
 lib/tracefs/tracefs-utils.c         |  43 +++++
 tracecmd/include/trace-local.h      |   5 +-
 tracecmd/trace-list.c               |   2 +-
 tracecmd/trace-record.c             | 264 +++++++++------------------
 tracecmd/trace-show.c               |   2 +
 tracecmd/trace-stat.c               |  21 ++-
 10 files changed, 431 insertions(+), 192 deletions(-)
 create mode 100644 lib/tracefs/tracefs-instance.c

diff --git a/include/tracefs/tracefs.h b/include/tracefs/tracefs.h
index e844c75..6b27ff7 100644
--- a/include/tracefs/tracefs.h
+++ b/include/tracefs/tracefs.h
@@ -17,4 +17,21 @@ const char *tracefs_get_tracing_dir(void);
 /* tracefs_find_tracing_dir must be freed */
 char *tracefs_find_tracing_dir(void);
 
+/* ftarce instances */
+struct tracefs_instance;
+
+struct tracefs_instance *tracefs_alloc_instance(const char *name);
+int tracefs_free_instance(struct tracefs_instance *instance);
+int tracefs_make_instance(struct tracefs_instance *instance);
+int tracefs_remove_instance(struct tracefs_instance *instance);
+char *tracefs_get_instance_name(struct tracefs_instance *instance);
+char *
+tracefs_get_instance_file(struct tracefs_instance *instance, const char *file);
+char *tracefs_get_instance_dir(struct tracefs_instance *instance);
+int tracefs_write_instance_file(struct tracefs_instance *instance,
+				const char *file, const char *str,
+				const char *type);
+char *tracefs_read_instance_file(struct tracefs_instance *instance,
+				 char *file, int *psize);
+
 #endif /* _TRACE_FS_H */
diff --git a/lib/tracefs/Makefile b/lib/tracefs/Makefile
index 86d7845..4030272 100644
--- a/lib/tracefs/Makefile
+++ b/lib/tracefs/Makefile
@@ -8,6 +8,7 @@ DEFAULT_TARGET = $(bdir)/libtracefs.a
 
 OBJS =
 OBJS += tracefs-utils.o
+OBJS += tracefs-instance.o
 
 OBJS := $(OBJS:%.o=$(bdir)/%.o)
 DEPS := $(OBJS:$(bdir)/%.o=$(bdir)/.%.d)
diff --git a/lib/tracefs/include/tracefs-local.h b/lib/tracefs/include/tracefs-local.h
index 231edd1..fe327a0 100644
--- a/lib/tracefs/include/tracefs-local.h
+++ b/lib/tracefs/include/tracefs-local.h
@@ -8,5 +8,6 @@
 
 /* Can be overridden */
 void warning(const char *fmt, ...);
+int str_read_file(const char *file, char **buffer);
 
 #endif /* _TRACE_FS_LOCAL_H */
diff --git a/lib/tracefs/tracefs-instance.c b/lib/tracefs/tracefs-instance.c
new file mode 100644
index 0000000..14e8eed
--- /dev/null
+++ b/lib/tracefs/tracefs-instance.c
@@ -0,0 +1,267 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Copyright (C) 2008, 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
+ *
+ * Updates:
+ * Copyright (C) 2019, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
+ *
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <errno.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+#include "tracefs.h"
+#include "tracefs-local.h"
+
+struct tracefs_instance {
+	char *name;
+};
+
+/**
+ * tracefs_alloc_instance - allocate a new ftrace instance
+ * @name: The name of the instance (instance will point to this)
+ *
+ * Returns a newly allocated instance, or NULL in case of an error.
+ */
+struct tracefs_instance *tracefs_alloc_instance(const char *name)
+{
+	struct tracefs_instance *instance;
+
+	instance = calloc(1, sizeof(*instance));
+	if (!instance)
+		return NULL;
+	if (name)
+		instance->name = strdup(name);
+
+	return instance;
+}
+
+/**
+ * tracefs_free_instance - Free an instance, previously allocated by
+			   tracefs_alloc_instance()
+ * @instance: Pointer to the instance to be freed
+ *
+ * Returns -1 in case of an error, or 0 otherwise.
+ */
+int tracefs_free_instance(struct tracefs_instance *instance)
+{
+	if (!instance)
+		return -1;
+
+	free(instance->name);
+	free(instance);
+	return 0;
+}
+
+/**
+ * tracefs_make_instance - Create a new ftrace instance
+ * @instance: Pointer to the instance to be created
+ *
+ * Returns 1 if the instance already exist, 0 if the instance
+ * is created successful or -1 in case of an error
+ */
+int tracefs_make_instance(struct tracefs_instance *instance)
+{
+	struct stat st;
+	char *path;
+	int ret;
+
+	path = tracefs_get_instance_dir(instance);
+	ret = stat(path, &st);
+	if (ret < 0) {
+		ret = mkdir(path, 0777);
+		if (ret < 0)
+			return ret;
+
+	} else
+		ret = 1;
+	tracefs_put_tracing_file(path);
+	return ret;
+}
+
+/**
+ * tracefs_remove_instance - Remove a ftrace instance
+ * @instance: Pointer to the instance to be removed
+ *
+ * Returns -1 in case of an error, or 0 otherwise.
+ */
+int tracefs_remove_instance(struct tracefs_instance *instance)
+{
+	char *path;
+	int ret;
+
+	if (!instance || !instance->name) {
+		warning("Cannot remove top instance");
+		return -1;
+	}
+
+	path = tracefs_get_instance_dir(instance);
+	ret = rmdir(path);
+	tracefs_put_tracing_file(path);
+
+	return ret;
+}
+
+/**
+ * tracefs_get_instance_file - return the path to an instance file.
+ * @instance: ftrace instance, can be NULL for the top instance
+ * @file: name of file to return
+ *
+ * Returns the path of the @file for the given @instance, or NULL in
+ * case of an error.
+ *
+ * Must use tracefs_put_tracing_file() to free the returned string.
+ */
+char *
+tracefs_get_instance_file(struct tracefs_instance *instance, const char *file)
+{
+	char *path;
+	char *buf;
+	int ret;
+
+	if (instance && instance->name) {
+		ret = asprintf(&buf, "instances/%s/%s", instance->name, file);
+		if (ret < 0) {
+			warning("Failed to allocate name for %s/%s",
+				 instance->name, file);
+			return NULL;
+		}
+		path = tracefs_get_tracing_file(buf);
+		free(buf);
+	} else
+		path = tracefs_get_tracing_file(file);
+
+	return path;
+}
+
+/**
+ * tracefs_get_instance_dir - return the path to the instance directory.
+ * @instance: ftrace instance, can be NULL for the top instance
+ *
+ * Returns the full path to the instance directory
+ *
+ * Must use tracefs_put_tracing_file() to free the returned string.
+ */
+char *tracefs_get_instance_dir(struct tracefs_instance *instance)
+{
+	char *buf;
+	char *path;
+	int ret;
+
+	if (instance && instance->name) {
+		ret = asprintf(&buf, "instances/%s", instance->name);
+		if (ret < 0) {
+			warning("Failed to allocate path for instance %s",
+				 instance->name);
+			return NULL;
+		}
+		path = tracefs_get_tracing_file(buf);
+		free(buf);
+	} else
+		path = tracefs_find_tracing_dir();
+
+	return path;
+}
+
+/**
+ * tracefs_get_instance_name - return the name of an instance
+ * @instance: ftrace instance
+ *
+ * Returns the name of the given @instance.
+ * The returned string must *not* be freed.
+ */
+char *tracefs_get_instance_name(struct tracefs_instance *instance)
+{
+	if (instance)
+		return instance->name;
+	return NULL;
+}
+
+static int write_file(const char *file, const char *str, const char *type)
+{
+	char buf[BUFSIZ];
+	int ret;
+	int fd;
+
+	fd = open(file, O_WRONLY | O_TRUNC);
+	if (fd < 0) {
+		warning("Failed to open '%s'", file);
+		return -1;
+	}
+	ret = write(fd, str, strlen(str));
+	close(fd);
+	if (ret < 0 && type) {
+		/* write failed */
+		fd = open(file, O_RDONLY);
+		if (fd < 0) {
+			warning("Failed to write in '%s'", file);
+			return -1;
+		}
+
+		while ((ret = read(fd, buf, BUFSIZ)) > 0)
+			fprintf(stderr, "%.*s", ret, buf);
+		warning("Failed %s of %s\n", type, file);
+		close(fd);
+	}
+	return ret;
+}
+
+
+/**
+ * tracefs_write_instance_file - Write in trace file of specific instance.
+ * @instance: ftrace instance, can be NULL for the top instance
+ * @file: name of the file
+ * @str: nul terminated string, that will be written in the file.
+ * @type: nul terminated string, describing the current write operation.
+ *	  Used for logging purposes.
+ *
+ * Returns the number of written bytes, or -1 in case of an error
+ */
+int tracefs_write_instance_file(struct tracefs_instance *instance,
+				 const char *file, const char *str,
+				 const char *type)
+{
+	struct stat st;
+	char *path;
+	int ret;
+
+	path = tracefs_get_instance_file(instance, file);
+	ret = stat(path, &st);
+	if (ret == 0)
+		ret = write_file(path, str, type);
+	tracefs_put_tracing_file(path);
+
+	return ret;
+}
+
+/**
+ * tracefs_read_instance_file - Read from a trace file of specific instance.
+ * @instance: ftrace instance, can be NULL for the top instance
+ * @file: name of the file
+ * @psize: returns the number of bytes read
+ *
+ * Returns a pointer to a nul terminated string, read from the file, or NULL in
+ * case of an error.
+ * The return string must be freed by free()
+ */
+char *tracefs_read_instance_file(struct tracefs_instance *instance,
+				  char *file, int *psize)
+{
+	char *buf = NULL;
+	int size = 0;
+	char *path;
+
+	path = tracefs_get_instance_file(instance, file);
+
+	size = str_read_file(path, &buf);
+
+	tracefs_put_tracing_file(path);
+	if (buf && psize)
+		*psize = size;
+
+	return buf;
+}
diff --git a/lib/tracefs/tracefs-utils.c b/lib/tracefs/tracefs-utils.c
index 0ef16fc..bdab130 100644
--- a/lib/tracefs/tracefs-utils.c
+++ b/lib/tracefs/tracefs-utils.c
@@ -10,6 +10,9 @@
 #include <sys/mount.h>
 #include <sys/stat.h>
 #include <linux/limits.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
 
 #include "tracefs.h"
 
@@ -181,3 +184,43 @@ void tracefs_put_tracing_file(char *name)
 {
 	free(name);
 }
+
+int str_read_file(const char *file, char **buffer)
+{
+	char stbuf[BUFSIZ];
+	char *buf = NULL;
+	int size = 0;
+	char *nbuf;
+	int fd;
+	int r;
+
+	fd = open(file, O_RDONLY);
+	if (fd < 0) {
+		warning("File %s not found", file);
+		return -1;
+	}
+
+	do {
+		r = read(fd, stbuf, BUFSIZ);
+		if (r <= 0)
+			continue;
+		nbuf = realloc(buf, size+r+1);
+		if (!nbuf) {
+			warning("Failed to allocate file buffer");
+			size = -1;
+			break;
+		}
+		buf = nbuf;
+		memcpy(buf+size, stbuf, r);
+		size += r;
+	} while (r);
+
+	close(fd);
+	if (size > 0) {
+		buf[size] = '\0';
+		*buffer = buf;
+	} else
+		free(buf);
+
+	return size;
+}
diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index fedc0b7..c7310e5 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -181,7 +181,7 @@ struct pid_addr_maps {
 
 struct buffer_instance {
 	struct buffer_instance	*next;
-	const char		*name;
+	struct tracefs_instance	*tracefs;
 	char			*cpumask;
 	struct event_list	*events;
 	struct event_list	**event_next;
@@ -225,6 +225,8 @@ struct buffer_instance {
 	bool			use_fifos;
 };
 
+void init_top_instance(void);
+
 extern struct buffer_instance top_instance;
 extern struct buffer_instance *buffer_instances;
 extern struct buffer_instance *first_instance;
@@ -238,7 +240,6 @@ extern struct buffer_instance *first_instance;
 
 struct buffer_instance *create_instance(const char *name);
 void add_instance(struct buffer_instance *instance, int cpu_count);
-char *get_instance_file(struct buffer_instance *instance, const char *file);
 void update_first_instance(struct buffer_instance *instance, int topt);
 
 void show_instance_file(struct buffer_instance *instance, const char *name);
diff --git a/tracecmd/trace-list.c b/tracecmd/trace-list.c
index 86e3358..d5c0707 100644
--- a/tracecmd/trace-list.c
+++ b/tracecmd/trace-list.c
@@ -34,7 +34,7 @@ void show_instance_file(struct buffer_instance *instance, const char *name)
 {
 	char *path;
 
-	path = get_instance_file(instance, name);
+	path = tracefs_get_instance_file(instance->tracefs, name);
 	dump_file_content(path);
 	tracefs_put_tracing_file(path);
 }
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 5355813..62f67d5 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -174,7 +174,7 @@ static struct reset_file *reset_files;
 /* Triggers need to be cleared in a special way */
 static struct reset_file *reset_triggers;
 
-struct buffer_instance top_instance = { .flags = BUFFER_FL_KEEP };
+struct buffer_instance top_instance;
 struct buffer_instance *buffer_instances;
 struct buffer_instance *first_instance;
 
@@ -372,7 +372,11 @@ struct buffer_instance *create_instance(const char *name)
 	if (!instance)
 		return NULL;
 	memset(instance, 0, sizeof(*instance));
-	instance->name = name;
+	instance->tracefs = tracefs_alloc_instance(name);
+	if (!instance->tracefs) {
+		free(instance);
+		return NULL;
+	}
 
 	return instance;
 }
@@ -472,7 +476,7 @@ void tracecmd_stat_cpu_instance(struct buffer_instance *instance,
 		return;
 	snprintf(file, 40, "per_cpu/cpu%d/stats", cpu);
 
-	path = get_instance_file(instance, file);
+	path = tracefs_get_instance_file(instance->tracefs, file);
 	free(file);
 	fd = open(path, O_RDONLY);
 	tracefs_put_tracing_file(path);
@@ -511,10 +515,11 @@ static void reset_event_list(struct buffer_instance *instance)
 
 static char *get_temp_file(struct buffer_instance *instance, int cpu)
 {
-	const char *name = instance->name;
+	const char *name;
 	char *file = NULL;
 	int size;
 
+	name = tracefs_get_instance_name(instance->tracefs);
 	if (name) {
 		size = snprintf(file, 0, "%s.%s.cpu%d", output_file, name, cpu);
 		file = malloc(size + 1);
@@ -558,9 +563,10 @@ static void put_temp_file(char *file)
 
 static void delete_temp_file(struct buffer_instance *instance, int cpu)
 {
-	const char *name = instance->name;
+	const char *name;
 	char file[PATH_MAX];
 
+	name = tracefs_get_instance_name(instance->tracefs);
 	if (name)
 		snprintf(file, PATH_MAX, "%s.%s.cpu%d", output_file, name, cpu);
 	else
@@ -784,54 +790,6 @@ static int set_ftrace(int set, int use_proc)
 	return 0;
 }
 
-/**
- * get_instance_file - return the path to a instance file.
- * @instance: buffer instance for the file
- * @file: name of file to return
- *
- * Returns the path name of the @file for the given @instance.
- *
- * Must use tracefs_put_tracing_file() to free the returned string.
- */
-char *
-get_instance_file(struct buffer_instance *instance, const char *file)
-{
-	char *buf;
-	char *path;
-	int ret;
-
-	if (instance->name) {
-		ret = asprintf(&buf, "instances/%s/%s", instance->name, file);
-		if (ret < 0)
-			die("Failed to allocate name for %s/%s", instance->name, file);
-		path = tracefs_get_tracing_file(buf);
-		free(buf);
-	} else
-		path = tracefs_get_tracing_file(file);
-
-	return path;
-}
-
-static char *
-get_instance_dir(struct buffer_instance *instance)
-{
-	char *buf;
-	char *path;
-	int ret;
-
-	/* only works for instances */
-	if (!instance->name)
-		return NULL;
-
-	ret = asprintf(&buf, "instances/%s", instance->name);
-	if (ret < 0)
-		die("Failed to allocate for instance %s", instance->name);
-	path = tracefs_get_tracing_file(buf);
-	free(buf);
-
-	return path;
-}
-
 static int write_file(const char *file, const char *str, const char *type)
 {
 	char buf[BUFSIZ];
@@ -857,23 +815,6 @@ static int write_file(const char *file, const char *str, const char *type)
 	return ret;
 }
 
-static int
-write_instance_file(struct buffer_instance *instance,
-		    const char *file, const char *str, const char *type)
-{
-	struct stat st;
-	char *path;
-	int ret;
-
-	path = get_instance_file(instance, file);
-	ret = stat(path, &st);
-	if (ret == 0)
-		ret = write_file(path, str, type);
-	tracefs_put_tracing_file(path);
-
-	return ret;
-}
-
 static void __clear_trace(struct buffer_instance *instance)
 {
 	FILE *fp;
@@ -883,7 +824,7 @@ static void __clear_trace(struct buffer_instance *instance)
 		return;
 
 	/* reset the trace */
-	path = get_instance_file(instance, "trace");
+	path = tracefs_get_instance_file(instance->tracefs, "trace");
 	fp = fopen(path, "w");
 	if (!fp)
 		die("writing to '%s'", path);
@@ -917,8 +858,8 @@ static void clear_trace(void)
 
 static void reset_max_latency(struct buffer_instance *instance)
 {
-	 write_instance_file(instance,
-			     "tracing_max_latency", "0", "max_latency");
+	tracefs_write_instance_file(instance->tracefs,
+				    "tracing_max_latency", "0", "max_latency");
 }
 
 static void add_filter_pid(int pid, int exclude)
@@ -1364,7 +1305,8 @@ static void add_event_pid(const char *buf)
 	struct buffer_instance *instance;
 
 	for_all_instances(instance)
-		write_instance_file(instance, "set_event_pid", buf, "event_pid");
+		tracefs_write_instance_file(instance->tracefs,
+					    "set_event_pid", buf, "event_pid");
 }
 
 static void add_new_filter_pid(int pid)
@@ -1593,7 +1535,7 @@ set_plugin_instance(struct buffer_instance *instance, const char *name)
 	if (is_guest(instance))
 		return;
 
-	path = get_instance_file(instance, "current_tracer");
+	path = tracefs_get_instance_file(instance->tracefs, "current_tracer");
 	fp = fopen(path, "w");
 	if (!fp) {
 		/*
@@ -1617,7 +1559,7 @@ set_plugin_instance(struct buffer_instance *instance, const char *name)
 
 	/* Make sure func_stack_trace option is disabled */
 	/* First try instance file, then top level */
-	path = get_instance_file(instance, "options/func_stack_trace");
+	path = tracefs_get_instance_file(instance->tracefs, "options/func_stack_trace");
 	fp = fopen(path, "w");
 	if (!fp) {
 		tracefs_put_tracing_file(path);
@@ -1678,8 +1620,6 @@ static int set_option(const char *option)
 	return 0;
 }
 
-static char *read_instance_file(struct buffer_instance *instance, char *file, int *psize);
-
 static void disable_func_stack_trace_instance(struct buffer_instance *instance)
 {
 	struct stat st;
@@ -1692,13 +1632,14 @@ static void disable_func_stack_trace_instance(struct buffer_instance *instance)
 	if (is_guest(instance))
 		return;
 
-	path = get_instance_file(instance, "current_tracer");
+	path = tracefs_get_instance_file(instance->tracefs, "current_tracer");
 	ret = stat(path, &st);
 	tracefs_put_tracing_file(path);
 	if (ret < 0)
 		return;
 
-	content = read_instance_file(instance, "current_tracer", &size);
+	content = tracefs_read_instance_file(instance->tracefs,
+					     "current_tracer", &size);
 	cond = strstrip(content);
 	if (memcmp(cond, "function", size - (cond - content)) !=0)
 		goto out;
@@ -1851,7 +1792,7 @@ static int trace_check_file_exists(struct buffer_instance *instance, char *file)
 	char *path;
 	int ret;
 
-	path = get_instance_file(instance, file);
+	path = tracefs_get_instance_file(instance->tracefs, file);
 	ret = stat(path, &st);
 	tracefs_put_tracing_file(path);
 
@@ -1930,7 +1871,7 @@ reset_events_instance(struct buffer_instance *instance)
 	}
 
 	c = '0';
-	path = get_instance_file(instance, "events/enable");
+	path = tracefs_get_instance_file(instance->tracefs, "events/enable");
 	fd = open(path, O_WRONLY);
 	if (fd < 0)
 		die("opening to '%s'", path);
@@ -1938,7 +1879,7 @@ reset_events_instance(struct buffer_instance *instance)
 	close(fd);
 	tracefs_put_tracing_file(path);
 
-	path = get_instance_file(instance, "events/*/filter");
+	path = tracefs_get_instance_file(instance->tracefs, "events/*/filter");
 	globbuf.gl_offs = 0;
 	ret = glob(path, 0, NULL, &globbuf);
 	tracefs_put_tracing_file(path);
@@ -2199,7 +2140,7 @@ static int open_instance_fd(struct buffer_instance *instance,
 	int fd;
 	char *path;
 
-	path = get_instance_file(instance, file);
+	path = tracefs_get_instance_file(instance->tracefs, file);
 	fd = open(path, flags);
 	if (fd < 0) {
 		/* instances may not be created yet */
@@ -2508,7 +2449,7 @@ static void set_mask(struct buffer_instance *instance)
 	if (!instance->cpumask)
 		return;
 
-	path = get_instance_file(instance, "tracing_cpumask");
+	path = tracefs_get_instance_file(instance->tracefs, "tracing_cpumask");
 	if (!path)
 		die("could not allocate path");
 	reset_save_file(path, RESET_DEFAULT_PRIO);
@@ -2569,7 +2510,8 @@ static void set_clock(struct buffer_instance *instance)
 		return;
 
 	/* The current clock is in brackets, reset it when we are done */
-	content = read_instance_file(instance, "trace_clock", NULL);
+	content = tracefs_read_instance_file(instance->tracefs,
+					     "trace_clock", NULL);
 
 	/* check if first clock is set */
 	if (*content == '[')
@@ -2580,13 +2522,14 @@ static void set_clock(struct buffer_instance *instance)
 			die("Can not find clock in trace_clock");
 		str = strtok(NULL, "]");
 	}
-	path = get_instance_file(instance, "trace_clock");
+	path = tracefs_get_instance_file(instance->tracefs, "trace_clock");
 	add_reset_file(path, str, RESET_DEFAULT_PRIO);
 
 	free(content);
 	tracefs_put_tracing_file(path);
 
-	write_instance_file(instance, "trace_clock", instance->clock, "clock");
+	tracefs_write_instance_file(instance->tracefs,
+				    "trace_clock", instance->clock, "clock");
 }
 
 static void set_max_graph_depth(struct buffer_instance *instance, char *max_graph_depth)
@@ -2597,11 +2540,11 @@ static void set_max_graph_depth(struct buffer_instance *instance, char *max_grap
 	if (is_guest(instance))
 		return;
 
-	path = get_instance_file(instance, "max_graph_depth");
+	path = tracefs_get_instance_file(instance->tracefs, "max_graph_depth");
 	reset_save_file(path, RESET_DEFAULT_PRIO);
 	tracefs_put_tracing_file(path);
-	ret = write_instance_file(instance, "max_graph_depth", max_graph_depth,
-				  NULL);
+	ret = tracefs_write_instance_file(instance->tracefs, "max_graph_depth",
+					  max_graph_depth, NULL);
 	if (ret < 0)
 		die("could not write to max_graph_depth");
 }
@@ -2718,7 +2661,7 @@ static int expand_event_files(struct buffer_instance *instance,
 	if (ret < 0)
 		die("Failed to allocate event filter path for %s", file);
 
-	path = get_instance_file(instance, p);
+	path = tracefs_get_instance_file(instance->tracefs, p);
 
 	globbuf.gl_offs = 0;
 	ret = glob(path, 0, NULL, &globbuf);
@@ -3180,10 +3123,7 @@ create_recorder_instance_pipe(struct buffer_instance *instance,
 	unsigned flags = recorder_flags | TRACECMD_RECORD_BLOCK;
 	char *path;
 
-	if (instance->name)
-		path = get_instance_dir(instance);
-	else
-		path = tracefs_find_tracing_dir();
+	path = tracefs_get_instance_dir(instance->tracefs);
 
 	if (!path)
 		die("malloc");
@@ -3193,8 +3133,7 @@ create_recorder_instance_pipe(struct buffer_instance *instance,
 
 	recorder = tracecmd_create_buffer_recorder_fd(brass[1], cpu, flags, path);
 
-	if (instance->name)
-		tracefs_put_tracing_file(path);
+	tracefs_put_tracing_file(path);
 
 	return recorder;
 }
@@ -3228,10 +3167,10 @@ create_recorder_instance(struct buffer_instance *instance, const char *file, int
 	if (brass)
 		return create_recorder_instance_pipe(instance, cpu, brass);
 
-	if (!instance->name)
+	if (!tracefs_get_instance_name(instance->tracefs))
 		return tracecmd_create_recorder_maxkb(file, cpu, recorder_flags, max_kb);
 
-	path = get_instance_dir(instance);
+	path = tracefs_get_instance_dir(instance->tracefs);
 
 	record = tracecmd_create_buffer_recorder_maxkb(file, cpu, recorder_flags,
 						       path, max_kb);
@@ -3285,8 +3224,8 @@ static int create_recorder(struct buffer_instance *instance, int cpu,
 		}
 		if (fd < 0)
 			die("Failed connecting to client");
-		if (instance->name && !is_agent(instance))
-			path = get_instance_dir(instance);
+		if (tracefs_get_instance_name(instance->tracefs) && !is_agent(instance))
+			path = tracefs_get_instance_dir(instance->tracefs);
 		else
 			path = tracefs_find_tracing_dir();
 		recorder = tracecmd_create_buffer_recorder_fd(fd, cpu, flags, path);
@@ -3597,9 +3536,11 @@ static void connect_to_agent(struct buffer_instance *instance)
 	unsigned int *ports;
 	int i, *fds = NULL;
 	bool use_fifos = false;
+	char *name;
 
+	name = tracefs_get_instance_name(instance->tracefs);
 	if (!no_fifos) {
-		nr_fifos = open_guest_fifos(instance->name, &fds);
+		nr_fifos = open_guest_fifos(name, &fds);
 		use_fifos = nr_fifos > 0;
 	}
 
@@ -3626,7 +3567,7 @@ static void connect_to_agent(struct buffer_instance *instance)
 		if (nr_cpus != nr_fifos) {
 			warning("number of FIFOs (%d) for guest %s differs "
 				"from number of virtual CPUs (%d)",
-				nr_fifos, instance->name, nr_cpus);
+				nr_fifos, name, nr_cpus);
 			nr_cpus = nr_cpus < nr_fifos ? nr_cpus : nr_fifos;
 		}
 		free(ports);
@@ -3652,7 +3593,8 @@ static void setup_guest(struct buffer_instance *instance)
 	int fd;
 
 	/* Create a place to store the guest meta data */
-	file = get_guest_file(output_file, instance->name);
+	file = get_guest_file(output_file,
+			      tracefs_get_instance_name(instance->tracefs));
 	if (!file)
 		die("Failed to allocate memory");
 
@@ -3828,7 +3770,8 @@ add_buffer_stat(struct tracecmd_output *handle, struct buffer_instance *instance
 	int i;
 
 	trace_seq_init(&s);
-	trace_seq_printf(&s, "\nBuffer: %s\n\n", instance->name);
+	trace_seq_printf(&s, "\nBuffer: %s\n\n",
+			tracefs_get_instance_name(instance->tracefs));
 	tracecmd_add_option(handle, TRACECMD_OPTION_CPUSTAT,
 			    s.len+1, s.buffer);
 	trace_seq_destroy(&s);
@@ -3894,7 +3837,8 @@ static void print_stat(struct buffer_instance *instance)
 		return;
 
 	if (!is_top_instance(instance))
-		printf("\nBuffer: %s\n\n", instance->name);
+		printf("\nBuffer: %s\n\n",
+			tracefs_get_instance_name(instance->tracefs));
 
 	for (cpu = 0; cpu < instance->cpu_count; cpu++)
 		trace_seq_do_printf(&instance->s_print[cpu]);
@@ -3934,7 +3878,8 @@ static void write_guest_file(struct buffer_instance *instance)
 	char **temp_files;
 	int i, fd;
 
-	file = get_guest_file(output_file, instance->name);
+	file = get_guest_file(output_file,
+			      tracefs_get_instance_name(instance->tracefs));
 	if (!file)
 		die("Failed to allocate memory");
 
@@ -4050,7 +3995,7 @@ static void record_data(struct common_record_context *ctx)
 					continue;
 
 				buffer_options[i++] = tracecmd_add_buffer_option(handle,
-										 instance->name,
+										 tracefs_get_instance_name(instance->tracefs),
 										 cpus);
 				add_buffer_stat(handle, instance);
 			}
@@ -4097,7 +4042,7 @@ static int write_func_file(struct buffer_instance *instance,
 	if (!*list)
 		return 0;
 
-	path = get_instance_file(instance, file);
+	path = tracefs_get_instance_file(instance->tracefs, file);
 
 	fd = open(path, O_WRONLY | O_TRUNC);
 	if (fd < 0)
@@ -4141,7 +4086,7 @@ static int functions_filtered(struct buffer_instance *instance)
 	char *path;
 	int fd;
 
-	path = get_instance_file(instance, "set_ftrace_filter");
+	path = tracefs_get_instance_file(instance->tracefs, "set_ftrace_filter");
 	fd = open(path, O_RDONLY);
 	tracefs_put_tracing_file(path);
 	if (fd < 0) {
@@ -4149,7 +4094,7 @@ static int functions_filtered(struct buffer_instance *instance)
 			warning("Can not set set_ftrace_filter");
 		else
 			warning("Can not set set_ftrace_filter for %s",
-				instance->name);
+				tracefs_get_instance_name(instance->tracefs));
 		return 0;
 	}
 
@@ -4309,45 +4254,9 @@ static unsigned long long find_time_stamp(struct tep_handle *pevent)
 	return ts;
 }
 
-static char *read_instance_file(struct buffer_instance *instance, char *file, int *psize)
-{
-	char buffer[BUFSIZ];
-	char *path;
-	char *buf;
-	int size = 0;
-	int fd;
-	int r;
-
-	path = get_instance_file(instance, file);
-	fd = open(path, O_RDONLY);
-	tracefs_put_tracing_file(path);
-	if (fd < 0) {
-		warning("%s not found, --date ignored", file);
-		return NULL;
-	}
-	do {
-		r = read(fd, buffer, BUFSIZ);
-		if (r <= 0)
-			continue;
-		if (size)
-			buf = realloc(buf, size+r+1);
-		else
-			buf = malloc(r+1);
-		if (!buf)
-			die("Failed to allocate instance file buffer");
-		memcpy(buf+size, buffer, r);
-		size += r;
-	} while (r);
-
-	buf[size] = '\0';
-	if (psize)
-		*psize = size;
-	return buf;
-}
-
 static char *read_file(char *file, int *psize)
 {
-	return read_instance_file(&top_instance, file, psize);
+	return tracefs_read_instance_file(top_instance.tracefs, file, psize);
 }
 
 /*
@@ -4480,7 +4389,7 @@ static void set_buffer_size_instance(struct buffer_instance *instance)
 
 	snprintf(buf, BUFSIZ, "%d", buffer_size);
 
-	path = get_instance_file(instance, "buffer_size_kb");
+	path = tracefs_get_instance_file(instance->tracefs, "buffer_size_kb");
 	fd = open(path, O_WRONLY);
 	if (fd < 0) {
 		warning("can't open %s", path);
@@ -4541,7 +4450,7 @@ static void clear_instance_triggers(struct buffer_instance *instance)
 	enum event_iter_type type;
 	enum event_process processed = PROCESSED_NONE;
 
-	path = get_instance_file(instance, "events");
+	path = tracefs_get_instance_file(instance->tracefs, "events");
 	if (!path)
 		die("malloc");
 
@@ -4602,7 +4511,7 @@ static void clear_instance_filters(struct buffer_instance *instance)
 	enum event_iter_type type;
 	enum event_process processed = PROCESSED_NONE;
 
-	path = get_instance_file(instance, "events");
+	path = tracefs_get_instance_file(instance->tracefs, "events");
 	if (!path)
 		die("malloc");
 
@@ -4638,7 +4547,8 @@ static void reset_clock(void)
 	struct buffer_instance *instance;
 
 	for_all_instances(instance)
-		write_instance_file(instance, "trace_clock", "local", "clock");
+		tracefs_write_instance_file(instance->tracefs,
+					    "trace_clock", "local", "clock");
 }
 
 static void reset_cpu_mask(void)
@@ -4657,7 +4567,8 @@ static void reset_cpu_mask(void)
 		strcat(buf, ",ffffffff");
 
 	for_all_instances(instance)
-		write_instance_file(instance, "tracing_cpumask", buf, "cpumask");
+		tracefs_write_instance_file(instance->tracefs,
+					    "tracing_cpumask", buf, "cpumask");
 }
 
 static void reset_event_pid(void)
@@ -4686,7 +4597,7 @@ static void clear_func_filters(void)
 
 	for_all_instances(instance) {
 		for (i = 0; files[i]; i++) {
-			path = get_instance_file(instance, files[i]);
+			path = tracefs_get_instance_file(instance->tracefs, files[i]);
 			clear_func_filter(path);
 			tracefs_put_tracing_file(path);
 		}
@@ -4696,32 +4607,20 @@ static void clear_func_filters(void)
 static void make_instances(void)
 {
 	struct buffer_instance *instance;
-	struct stat st;
-	char *path;
-	int ret;
 
 	for_each_instance(instance) {
 		if (is_guest(instance))
 			continue;
-
-		path = get_instance_dir(instance);
-		ret = stat(path, &st);
-		if (ret < 0) {
-			ret = mkdir(path, 0777);
-			if (ret < 0)
-				die("mkdir %s", path);
-		} else
+		if (tracefs_make_instance(instance->tracefs) > 0) {
 			/* Don't delete instances that already exist */
 			instance->flags |= BUFFER_FL_KEEP;
-		tracefs_put_tracing_file(path);
+		}
 	}
 }
 
 void tracecmd_remove_instances(void)
 {
 	struct buffer_instance *instance;
-	char *path;
-	int ret;
 
 	for_each_instance(instance) {
 		/* Only delete what we created */
@@ -4731,11 +4630,7 @@ void tracecmd_remove_instances(void)
 			close(instance->tracing_on_fd);
 			instance->tracing_on_fd = 0;
 		}
-		path = get_instance_dir(instance);
-		ret = rmdir(path);
-		if (ret < 0)
-			die("rmdir %s", path);
-		tracefs_put_tracing_file(path);
+		tracefs_remove_instance(instance->tracefs);
 	}
 }
 
@@ -5029,7 +4924,8 @@ static int test_stacktrace_trigger(struct buffer_instance *instance)
 	int ret = 0;
 	int fd;
 
-	path = get_instance_file(instance, "events/sched/sched_switch/trigger");
+	path = tracefs_get_instance_file(instance->tracefs,
+					 "events/sched/sched_switch/trigger");
 
 	clear_trigger(path);
 
@@ -5211,6 +5107,15 @@ void update_first_instance(struct buffer_instance *instance, int topt)
 		first_instance = buffer_instances;
 }
 
+void init_top_instance(void)
+{
+	if (!top_instance.tracefs)
+		top_instance.tracefs = tracefs_alloc_instance(NULL);
+	top_instance.cpu_count = count_cpus();
+	top_instance.flags = BUFFER_FL_KEEP;
+	init_instance(&top_instance);
+}
+
 enum {
 	OPT_user		= 243,
 	OPT_procmap		= 244,
@@ -5235,7 +5140,7 @@ void trace_stop(int argc, char **argv)
 	int topt = 0;
 	struct buffer_instance *instance = &top_instance;
 
-	init_instance(instance);
+	init_top_instance();
 
 	for (;;) {
 		int c;
@@ -5276,7 +5181,7 @@ void trace_restart(int argc, char **argv)
 	int topt = 0;
 	struct buffer_instance *instance = &top_instance;
 
-	init_instance(instance);
+	init_top_instance();
 
 	for (;;) {
 		int c;
@@ -5318,7 +5223,7 @@ void trace_reset(int argc, char **argv)
 	int topt = 0;
 	struct buffer_instance *instance = &top_instance;
 
-	init_instance(instance);
+	init_top_instance();
 
 	/* if last arg is -a, then -b and -d apply to all instances */
 	int last_specified_all = 0;
@@ -5403,9 +5308,8 @@ static void init_common_record_context(struct common_record_context *ctx,
 	memset(ctx, 0, sizeof(*ctx));
 	ctx->instance = &top_instance;
 	ctx->curr_cmd = curr_cmd;
-	init_instance(ctx->instance);
 	local_cpu_count = count_cpus();
-	ctx->instance->cpu_count = local_cpu_count;
+	init_top_instance();
 }
 
 #define IS_EXTRACT(ctx) ((ctx)->curr_cmd == CMD_extract)
diff --git a/tracecmd/trace-show.c b/tracecmd/trace-show.c
index 391d329..a6f2102 100644
--- a/tracecmd/trace-show.c
+++ b/tracecmd/trace-show.c
@@ -52,6 +52,8 @@ void trace_show(int argc, char **argv)
 		{NULL, 0, NULL, 0}
 	};
 
+	init_top_instance();
+
 	while ((c = getopt_long(argc-1, argv+1, "B:c:fsp",
 				long_options, &option_index)) >= 0) {
 		switch (c) {
diff --git a/tracecmd/trace-stat.c b/tracecmd/trace-stat.c
index 7a1d9bb..948118c 100644
--- a/tracecmd/trace-stat.c
+++ b/tracecmd/trace-stat.c
@@ -31,7 +31,7 @@ static int get_instance_file_fd(struct buffer_instance *instance,
 	char *path;
 	int fd;
 
-	path = get_instance_file(instance, file);
+	path = tracefs_get_instance_file(instance->tracefs, file);
 	fd = open(path, O_RDONLY);
 	tracefs_put_tracing_file(path);
 
@@ -348,7 +348,7 @@ static void report_events(struct buffer_instance *instance)
 
 	free(str);
 
-	path = get_instance_file(instance, "events");
+	path = tracefs_get_instance_file(instance->tracefs, "events");
 	if (!path)
 		die("malloc");
 
@@ -437,7 +437,7 @@ static void report_event_filters(struct buffer_instance *instance)
 	enum event_iter_type type;
 	enum event_process processed = PROCESSED_NONE;
 
-	path = get_instance_file(instance, "events");
+	path = tracefs_get_instance_file(instance->tracefs, "events");
 	if (!path)
 		die("malloc");
 
@@ -510,7 +510,7 @@ static void report_event_triggers(struct buffer_instance *instance)
 	enum event_iter_type type;
 	enum event_process processed = PROCESSED_NONE;
 
-	path = get_instance_file(instance, "events");
+	path = tracefs_get_instance_file(instance->tracefs, "events");
 	if (!path)
 		die("malloc");
 
@@ -599,7 +599,7 @@ static void report_graph_funcs(struct buffer_instance *instance)
 {
 	char *path;
 
-	path = get_instance_file(instance, "set_graph_function");
+	path = tracefs_get_instance_file(instance->tracefs, "set_graph_function");
 	if (!path)
 		die("malloc");
 
@@ -607,7 +607,7 @@ static void report_graph_funcs(struct buffer_instance *instance)
 	
 	tracefs_put_tracing_file(path);
 
-	path = get_instance_file(instance, "set_graph_notrace");
+	path = tracefs_get_instance_file(instance->tracefs, "set_graph_notrace");
 	if (!path)
 		die("malloc");
 
@@ -620,7 +620,7 @@ static void report_ftrace_filters(struct buffer_instance *instance)
 {
 	char *path;
 
-	path = get_instance_file(instance, "set_ftrace_filter");
+	path = tracefs_get_instance_file(instance->tracefs, "set_ftrace_filter");
 	if (!path)
 		die("malloc");
 
@@ -628,7 +628,7 @@ static void report_ftrace_filters(struct buffer_instance *instance)
 	
 	tracefs_put_tracing_file(path);
 
-	path = get_instance_file(instance, "set_ftrace_notrace");
+	path = tracefs_get_instance_file(instance->tracefs, "set_ftrace_notrace");
 	if (!path)
 		die("malloc");
 
@@ -858,7 +858,8 @@ static void stat_instance(struct buffer_instance *instance)
 	if (instance != &top_instance) {
 		if (instance != first_instance)
 			printf("---------------\n");
-		printf("Instance: %s\n", instance->name);
+		printf("Instance: %s\n",
+			tracefs_get_instance_name(instance->tracefs));
 	}
 
 	report_plugin(instance);
@@ -883,6 +884,8 @@ void trace_stat (int argc, char **argv)
 	int status;
 	int c;
 
+	init_top_instance();
+
 	for (;;) {
 		c = getopt(argc-1, argv+1, "tB:");
 		if (c == -1)
-- 
2.24.1


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

* [PATCH v2 4/5] trace-cmd,kernel-shark: New libtracefs APIs for ftrace events and systems
  2020-01-06 15:40 [PATCH v2 0/5] tracefs library Tzvetomir Stoyanov (VMware)
                   ` (2 preceding siblings ...)
  2020-01-06 15:40 ` [PATCH v2 3/5] trace-cmd: New libtracefs APIs for ftrace instances Tzvetomir Stoyanov (VMware)
@ 2020-01-06 15:40 ` Tzvetomir Stoyanov (VMware)
  2020-01-08 20:22   ` Steven Rostedt
                     ` (2 more replies)
  2020-01-06 15:40 ` [PATCH v2 5/5] trace-cmd,kernel-shark: New libtracefs APIs for loading ftrace events Tzvetomir Stoyanov (VMware)
  4 siblings, 3 replies; 15+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-01-06 15:40 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

The functionality related to ftrace events and systems
is moved from trace-cmd application and libtracecmd to libtracefs.

The following libtracecmd APIs are removed:
  tracecmd_read_page_record();
  tracecmd_event_systems();
  tracecmd_system_events();
  tracecmd_local_plugins();
  tracecmd_add_list();
  tracecmd_free_list();

The following new library APIs are introduced:
  tracefs_read_page_record();
  tracefs_event_systems();
  tracefs_system_events();
  tracefs_local_plugins();
  tracefs_iterate_raw_events();

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h        |   8 -
 include/tracefs/tracefs.h            |  15 +
 kernel-shark/src/KsCaptureDialog.cpp |   2 +-
 lib/trace-cmd/trace-input.c          |  95 ------
 lib/trace-cmd/trace-util.c           | 265 ---------------
 lib/tracefs/Makefile                 |   1 +
 lib/tracefs/tracefs-events.c         | 476 +++++++++++++++++++++++++++
 tracecmd/trace-record.c              |   2 +-
 8 files changed, 494 insertions(+), 370 deletions(-)
 create mode 100644 lib/tracefs/tracefs-events.c

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 66736ae..30bb144 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -24,15 +24,10 @@ void tracecmd_parse_ftrace_printk(struct tep_handle *pevent, char *file, unsigne
 extern int tracecmd_disable_sys_plugins;
 extern int tracecmd_disable_plugins;
 
-char **tracecmd_event_systems(const char *tracing_dir);
-char **tracecmd_system_events(const char *tracing_dir, const char *system);
 struct tep_handle *tracecmd_local_events(const char *tracing_dir);
 int tracecmd_fill_local_events(const char *tracing_dir,
 			       struct tep_handle *pevent, int *parsing_failures);
-char **tracecmd_local_plugins(const char *tracing_dir);
 
-char **tracecmd_add_list(char **list, const char *name, int len);
-void tracecmd_free_list(char **list);
 int *tracecmd_add_id(int *list, int id, int len);
 
 enum {
@@ -143,9 +138,6 @@ void tracecmd_print_stats(struct tracecmd_input *handle);
 void tracecmd_print_uname(struct tracecmd_input *handle);
 void tracecmd_print_version(struct tracecmd_input *handle);
 
-struct tep_record *
-tracecmd_read_page_record(struct tep_handle *pevent, void *page, int size,
-			  struct tep_record *last_record);
 struct tep_record *
 tracecmd_peek_data(struct tracecmd_input *handle, int cpu);
 
diff --git a/include/tracefs/tracefs.h b/include/tracefs/tracefs.h
index 6b27ff7..c66250c 100644
--- a/include/tracefs/tracefs.h
+++ b/include/tracefs/tracefs.h
@@ -34,4 +34,19 @@ int tracefs_write_instance_file(struct tracefs_instance *instance,
 char *tracefs_read_instance_file(struct tracefs_instance *instance,
 				 char *file, int *psize);
 
+/* events */
+struct tep_record *
+tracefs_read_page_record(struct tep_handle *tep, void *page, int size,
+			  struct tep_record *last_record);
+char **tracefs_event_systems(const char *tracing_dir);
+char **tracefs_system_events(const char *tracing_dir, const char *system);
+int tracefs_iterate_raw_events(struct tep_handle *tep,
+				struct tracefs_instance *instance,
+				int (*callback)(struct tep_event *,
+						struct tep_record *,
+						int, void *),
+				void *callback_context);
+
+char **tracefs_local_plugins(const char *tracing_dir);
+
 #endif /* _TRACE_FS_H */
diff --git a/kernel-shark/src/KsCaptureDialog.cpp b/kernel-shark/src/KsCaptureDialog.cpp
index 548b6fb..49c8755 100644
--- a/kernel-shark/src/KsCaptureDialog.cpp
+++ b/kernel-shark/src/KsCaptureDialog.cpp
@@ -204,7 +204,7 @@ QStringList KsCaptureControl::_getPlugins()
 	QStringList pluginList;
 	char **all_plugins;
 
-	all_plugins = tracecmd_local_plugins(tracefs_get_tracing_dir());
+	all_plugins = tracefs_local_plugins(tracefs_get_tracing_dir());
 
 	if (!all_plugins)
 		return pluginList;
diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index 3b187e3..67c7236 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -1663,101 +1663,6 @@ tracecmd_translate_data(struct tracecmd_input *handle,
 	return record;
 }
 
-/**
- * tracecmd_read_page_record - read a record off of a page
- * @pevent: pevent used to parse the page
- * @page: the page to read
- * @size: the size of the page
- * @last_record: last record read from this page.
- *
- * If a ring buffer page is available, and the need to parse it
- * without having a handle, then this function can be used.
- *
- * The @pevent needs to be initialized to have the page header information
- * already available.
- *
- * The @last_record is used to know where to read the next record from.
- * If @last_record is NULL, the first record on the page will be read.
- *
- * Returns:
- *  A newly allocated record that must be freed with free_record() if
- *  a record is found. Otherwise NULL is returned if the record is bad
- *  or no more records exist.
- */
-struct tep_record *
-tracecmd_read_page_record(struct tep_handle *pevent, void *page, int size,
-			  struct tep_record *last_record)
-{
-	unsigned long long ts;
-	struct kbuffer *kbuf;
-	struct tep_record *record = NULL;
-	enum kbuffer_long_size long_size;
-	enum kbuffer_endian endian;
-	void *ptr;
-
-	if (tep_is_file_bigendian(pevent))
-		endian = KBUFFER_ENDIAN_BIG;
-	else
-		endian = KBUFFER_ENDIAN_LITTLE;
-
-	if (tep_get_header_page_size(pevent) == 8)
-		long_size = KBUFFER_LSIZE_8;
-	else
-		long_size = KBUFFER_LSIZE_4;
-
-	kbuf = kbuffer_alloc(long_size, endian);
-	if (!kbuf)
-		return NULL;
-
-	kbuffer_load_subbuffer(kbuf, page);
-	if (kbuffer_subbuffer_size(kbuf) > size) {
-		warning("tracecmd_read_page_record: page_size > size");
-		goto out_free;
-	}
-
-	if (last_record) {
-		if (last_record->data < page || last_record->data >= (page + size)) {
-			warning("tracecmd_read_page_record: bad last record (size=%u)",
-				last_record->size);
-			goto out_free;
-		}
-
-		ptr = kbuffer_read_event(kbuf, &ts);
-		while (ptr < last_record->data) {
-			ptr = kbuffer_next_event(kbuf, NULL);
-			if (!ptr)
-				break;
-			if (ptr == last_record->data)
-				break;
-		}
-		if (ptr != last_record->data) {
-			warning("tracecmd_read_page_record: could not find last_record");
-			goto out_free;
-		}
-		ptr = kbuffer_next_event(kbuf, &ts);
-	} else
-		ptr = kbuffer_read_event(kbuf, &ts);
-
-	if (!ptr)
-		goto out_free;
-
-	record = malloc(sizeof(*record));
-	if (!record)
-		return NULL;
-	memset(record, 0, sizeof(*record));
-
-	record->ts = ts;
-	record->size = kbuffer_event_size(kbuf);
-	record->record_size = kbuffer_curr_size(kbuf);
-	record->cpu = 0;
-	record->data = ptr;
-	record->ref_count = 1;
-
- out_free:
-	kbuffer_free(kbuf);
-	return record;
-}
-
 /**
  * tracecmd_peek_data - return the record at the current location.
  * @handle: input handle for the trace.dat file
diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
index 1394469..1554e88 100644
--- a/lib/trace-cmd/trace-util.c
+++ b/lib/trace-cmd/trace-util.c
@@ -153,56 +153,6 @@ static char *append_file(const char *dir, const char *name)
 	return ret < 0 ? NULL : file;
 }
 
-/**
- * tracecmd_add_list - add an new string to a string list.
- * @list: list to add the string to (may be NULL)
- * @name: the string to add
- * @len: current length of list of strings.
- *
- * The typical usage is:
- *
- *    systems = tracecmd_add_list(systems, name, len++);
- *
- * Returns the new allocated list with an allocated name added.
- * The list will end with NULL.
- */
-char **tracecmd_add_list(char **list, const char *name, int len)
-{
-	if (!list)
-		list = malloc(sizeof(*list) * 2);
-	else
-		list = realloc(list, sizeof(*list) * (len + 2));
-	if (!list)
-		return NULL;
-
-	list[len] = strdup(name);
-	if (!list[len])
-		return NULL;
-
-	list[len + 1] = NULL;
-
-	return list;
-}
-
-/**
- * tracecmd_free_list - free a list created with tracecmd_add_list.
- * @list: The list to free.
- *
- * Frees the list as well as the names within the list.
- */
-void tracecmd_free_list(char **list)
-{
-	int i;
-
-	if (!list)
-		return;
-
-	for (i = 0; list[i]; i++)
-		free(list[i]);
-
-	free(list);
-}
-
 /**
  * tracecmd_add_id - add an int to the event id list
  * @list: list to add the id to
@@ -233,160 +183,6 @@ int *tracecmd_add_id(int *list, int id, int len)
 	return list;
 }
 
-/**
- * tracecmd_event_systems - return list of systems for tracing
- * @tracing_dir: directory holding the "events" directory
- *
- * Returns an allocated list of system names. Both the names and
- * the list must be freed with free().
- * The list returned ends with a "NULL" pointer.
- */
-char **tracecmd_event_systems(const char *tracing_dir)
-{
-	struct dirent *dent;
-	char **systems = NULL;
-	char *events_dir;
-	struct stat st;
-	DIR *dir;
-	int len = 0;
-	int ret;
-
-	if (!tracing_dir)
-		return NULL;
-
-	events_dir = append_file(tracing_dir, "events");
-	if (!events_dir)
-		return NULL;
-
-	/*
-	 * Search all the directories in the events directory,
- 	 * and collect the ones that have the "enable" file.
-	 */
-	ret = stat(events_dir, &st);
-	if (ret < 0 || !S_ISDIR(st.st_mode))
-		goto out_free;
-
-	dir = opendir(events_dir);
-	if (!dir)
-		goto out_free;
-
-	while ((dent = readdir(dir))) {
-		const char *name = dent->d_name;
-		char *enable;
-		char *sys;
-
-		if (strcmp(name, ".") == 0 ||
-		    strcmp(name, "..") == 0)
-			continue;
-
-		sys = append_file(events_dir, name);
-		ret = stat(sys, &st);
-		if (ret < 0 || !S_ISDIR(st.st_mode)) {
-			free(sys);
-			continue;
-		}
-
-		enable = append_file(sys, "enable");
-
-		ret = stat(enable, &st);
-		if (ret >= 0)
-			systems = tracecmd_add_list(systems, name, len++);
-
-		free(enable);
-		free(sys);
-	}
-
-	closedir(dir);
-
- out_free:
-	free(events_dir);
-	return systems;
-}
-
-/**
- * tracecmd_system_events - return list of events for system
- * @tracing_dir: directory holding the "events" directory
- * @system: the system to return the events for
- *
- * Returns an allocated list of event names. Both the names and
- * the list must be freed with free().
- * The list returned ends with a "NULL" pointer.
- */
-char **tracecmd_system_events(const char *tracing_dir, const char *system)
-{
-	struct dirent *dent;
-	char **events = NULL;
-	char *events_dir;
-	char *system_dir;
-	struct stat st;
-	DIR *dir;
-	int len = 0;
-	int ret;
-
-	if (!tracing_dir || !system)
-		return NULL;
-
-	events_dir = append_file(tracing_dir, "events");
-	if (!events_dir)
-		return NULL;
-
-	/*
-	 * Search all the directories in the systems directory,
-	 * and collect the ones that have the "enable" file.
-	 */
-	ret = stat(events_dir, &st);
-	if (ret < 0 || !S_ISDIR(st.st_mode))
-		goto out_free;
-
-	system_dir = append_file(events_dir, system);
-	if (!system_dir)
-		goto out_free;
-
-	ret = stat(system_dir, &st);
-	if (ret < 0 || !S_ISDIR(st.st_mode))
-		goto out_free_sys;
-
-	dir = opendir(system_dir);
-	if (!dir)
-		goto out_free_sys;
-
-	while ((dent = readdir(dir))) {
-		const char *name = dent->d_name;
-		char *enable;
-		char *event;
-
-		if (strcmp(name, ".") == 0 ||
-		    strcmp(name, "..") == 0)
-			continue;
-
-		event = append_file(system_dir, name);
-		ret = stat(event, &st);
-		if (ret < 0 || !S_ISDIR(st.st_mode)) {
-			free(event);
-			continue;
-		}
-
-		enable = append_file(event, "enable");
-
-		ret = stat(enable, &st);
-		if (ret >= 0)
-			events = tracecmd_add_list(events, name, len++);
-
-		free(enable);
-		free(event);
-	}
-
-	closedir(dir);
-
- out_free_sys:
-	free(system_dir);
-
- out_free:
-	free(events_dir);
-
-	return events;
-}
-
 static int read_file(const char *file, char **buffer)
 {
 	char *buf;
@@ -604,67 +400,6 @@ int tracecmd_fill_local_events(const char *tracing_dir,
 	return ret;
 }
 
-/**
- * tracecmd_local_plugins - returns an array of available tracer plugins
- * @tracing_dir: The directory that contains the tracing directory
- *
- * Returns an allocate list of plugins. The array ends with NULL.
- * Both the plugin names and array must be freed with free().
- */
-char **tracecmd_local_plugins(const char *tracing_dir)
-{
-	char *available_tracers;
-	struct stat st;
-	char **plugins = NULL;
-	char *buf;
-	char *str, *saveptr;
-	char *plugin;
-	int slen;
-	int len;
-	int ret;
-
-	if (!tracing_dir)
-		return NULL;
-
-	available_tracers = append_file(tracing_dir, "available_tracers");
-	if (!available_tracers)
-		return NULL;
-
-	ret = stat(available_tracers, &st);
-	if (ret < 0)
-		goto out_free;
-
-	len = read_file(available_tracers, &buf);
-	if (len < 0)
-		goto out_free;
-
-	len = 0;
-	for (str = buf; ; str = NULL) {
-		plugin = strtok_r(str, " ", &saveptr);
-		if (!plugin)
-			break;
-		if (!(slen = strlen(plugin)))
-			continue;
-
-		/* chop off any newlines */
-		if (plugin[slen - 1] == '\n')
-			plugin[slen - 1] = '\0';
-
-		/* Skip the non tracers */
-		if (strcmp(plugin, "nop") == 0 ||
-		    strcmp(plugin, "none") == 0)
-			continue;
-
-		plugins = tracecmd_add_list(plugins, plugin, len++);
-	}
-	free(buf);
-
- out_free:
-	free(available_tracers);
-
-	return plugins;
-}
-
 struct add_plugin_data {
 	int ret;
 	int index;
diff --git a/lib/tracefs/Makefile b/lib/tracefs/Makefile
index 4030272..5763e06 100644
--- a/lib/tracefs/Makefile
+++ b/lib/tracefs/Makefile
@@ -9,6 +9,7 @@ DEFAULT_TARGET = $(bdir)/libtracefs.a
 OBJS =
 OBJS += tracefs-utils.o
 OBJS += tracefs-instance.o
+OBJS += tracefs-events.o
 
 OBJS := $(OBJS:%.o=$(bdir)/%.o)
 DEPS := $(OBJS:$(bdir)/%.o=$(bdir)/.%.d)
diff --git a/lib/tracefs/tracefs-events.c b/lib/tracefs/tracefs-events.c
new file mode 100644
index 0000000..5d40f06
--- /dev/null
+++ b/lib/tracefs/tracefs-events.c
@@ -0,0 +1,476 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Copyright (C) 2008, 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
+ *
+ * Updates:
+ * Copyright (C) 2019, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
+ *
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <dirent.h>
+#include <unistd.h>
+#include <errno.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+#include "kbuffer.h"
+#include "tracefs.h"
+#include "tracefs-local.h"
+
+/**
+ * tracefs_read_page_record - read a record off of a page
+ * @tep: tep used to parse the page
+ * @page: the page to read
+ * @size: the size of the page
+ * @last_record: last record read from this page
+ *
+ * If a ring buffer page is available, and the need to parse it
+ * without having a handle, then this function can be used
+ *
+ * The @tep needs to be initialized to have the page header information
+ * already available.
+ *
+ * The @last_record is used to know where to read the next record from
+ * If @last_record is NULL, the first record on the page will be read
+ *
+ * Returns:
+ *  A newly allocated record that must be freed with free_record() if
+ *  a record is found. Otherwise NULL is returned if the record is bad
+ *  or no more records exist
+ */
+struct tep_record *
+tracefs_read_page_record(struct tep_handle *tep, void *page, int size,
+			  struct tep_record *last_record)
+{
+	unsigned long long ts;
+	struct kbuffer *kbuf;
+	struct tep_record *record = NULL;
+	enum kbuffer_long_size long_size;
+	enum kbuffer_endian endian;
+	void *ptr;
+
+	if (tep_is_file_bigendian(tep))
+		endian = KBUFFER_ENDIAN_BIG;
+	else
+		endian = KBUFFER_ENDIAN_LITTLE;
+
+	if (tep_get_header_page_size(tep) == 8)
+		long_size = KBUFFER_LSIZE_8;
+	else
+		long_size = KBUFFER_LSIZE_4;
+
+	kbuf = kbuffer_alloc(long_size, endian);
+	if (!kbuf)
+		return NULL;
+
+	kbuffer_load_subbuffer(kbuf, page);
+	if (kbuffer_subbuffer_size(kbuf) > size) {
+		warning("%s: page_size > size", __func__);
+		goto out_free;
+	}
+
+	if (last_record) {
+		if (last_record->data < page || last_record->data >= (page + size)) {
+			warning("%s: bad last record (size=%u)",
+				__func__, last_record->size);
+			goto out_free;
+		}
+
+		ptr = kbuffer_read_event(kbuf, &ts);
+		while (ptr < last_record->data) {
+			ptr = kbuffer_next_event(kbuf, NULL);
+			if (!ptr)
+				break;
+			if (ptr == last_record->data)
+				break;
+		}
+		if (ptr != last_record->data) {
+			warning("$s: could not find last_record", __func__);
+			goto out_free;
+		}
+		ptr = kbuffer_next_event(kbuf, &ts);
+	} else
+		ptr = kbuffer_read_event(kbuf, &ts);
+
+	if (!ptr)
+		goto out_free;
+
+	record = malloc(sizeof(*record));
+	if (!record)
+		return NULL;
+	memset(record, 0, sizeof(*record));
+
+	record->ts = ts;
+	record->size = kbuffer_event_size(kbuf);
+	record->record_size = kbuffer_curr_size(kbuf);
+	record->cpu = 0;
+	record->data = ptr;
+	record->ref_count = 1;
+
+ out_free:
+	kbuffer_free(kbuf);
+	return record;
+}
+
+static void free_record(struct tep_record *record)
+{
+	if (!record)
+		return;
+
+	if (record->ref_count > 0)
+		record->ref_count--;
+	if (record->ref_count)
+		return;
+
+	free(record);
+}
+
+static int
+get_events_in_page(struct tep_handle *tep, void *page,
+		   int size, int cpu,
+		   int (*callback)(struct tep_event *,
+				   struct tep_record *,
+				   int, void *),
+		   void *callback_context)
+{
+	struct tep_record *last_record = NULL;
+	struct tep_event *event = NULL;
+	struct tep_record *record;
+	int id, cnt = 0;
+
+	if (size <= 0)
+		return 0;
+
+	while (true) {
+		event = NULL;
+		record = tracefs_read_page_record(tep, page, size, last_record);
+		if (!record)
+			break;
+		free_record(last_record);
+		id = tep_data_type(tep, record);
+		event = tep_find_event(tep, id);
+		if (event && callback) {
+			if (callback(event, record, cpu, callback_context))
+				break;
+		}
+		last_record = record;
+	}
+	free_record(last_record);
+
+	return cnt;
+}
+
+/*
+ * tracefs_iterate_raw_events - Iterate through events in trace_pipe_raw
+ *				 per CPU trace files
+ * @tep: a handle to the trace event parser context
+ * @instance: ftrace instance, can be NULL for the top instance
+ * @callback: A user function, called for each record from the file
+ * @callback_context: A custom context, passed to the user callback function
+ *
+ * If the @callback returns non-zero, the iteration stops - in that case all
+ * records from the current page will be lost from future reads
+ *
+ * Returns -1 in case of an error, or 0 otherwise
+ */
+int tracefs_iterate_raw_events(struct tep_handle *tep,
+				struct tracefs_instance *instance,
+				int (*callback)(struct tep_event *,
+						struct tep_record *,
+						int, void *),
+				void *callback_context)
+{
+	unsigned int p_size;
+	struct dirent *dent;
+	char file[PATH_MAX];
+	void *page = NULL;
+	struct stat st;
+	char *path;
+	DIR *dir;
+	int ret;
+	int cpu;
+	int fd;
+	int r;
+
+	p_size = getpagesize();
+	path = tracefs_get_instance_file(instance, "per_cpu");
+	if (!path)
+		return -1;
+	dir = opendir(path);
+	if (!dir) {
+		ret = -1;
+		goto error;
+	}
+	page = malloc(p_size);
+	if (!page) {
+		ret = -1;
+		goto error;
+	}
+	while ((dent = readdir(dir))) {
+		const char *name = dent->d_name;
+
+		if (strlen(name) < 4 || strncmp(name, "cpu", 3) != 0)
+			continue;
+		cpu = atoi(name + 3);
+		sprintf(file, "%s/%s", path, name);
+		ret = stat(file, &st);
+		if (ret < 0 || !S_ISDIR(st.st_mode))
+			continue;
+
+		sprintf(file, "%s/%s/trace_pipe_raw", path, name);
+		fd = open(file, O_RDONLY | O_NONBLOCK);
+		if (fd < 0)
+			continue;
+		do {
+			r = read(fd, page, p_size);
+			if (r > 0)
+				get_events_in_page(tep, page, r, cpu,
+						   callback, callback_context);
+		} while (r > 0);
+		close(fd);
+	}
+	ret = 0;
+
+error:
+	if (dir)
+		closedir(dir);
+	free(page);
+	tracefs_put_tracing_file(path);
+	return ret;
+}
+
+static char **add_list_string(char **list, const char *name, int len)
+{
+	if (!list)
+		list = malloc(sizeof(*list) * 2);
+	else
+		list = realloc(list, sizeof(*list) * (len + 2));
+	if (!list)
+		return NULL;
+
+	list[len] = strdup(name);
+	if (!list[len])
+		return NULL;
+
+	list[len + 1] = NULL;
+
+	return list;
+}
+
+static char *append_file(const char *dir, const char *name)
+{
+	char *file;
+	int ret;
+
+	ret = asprintf(&file, "%s/%s", dir, name);
+
+	return ret < 0 ? NULL : file;
+}
+
+/**
+ * tracefs_event_systems - return list of systems for tracing
+ * @tracing_dir: directory holding the "events" directory
+ *		 if NULL, top tracing directory is used
+ *
+ * Returns an allocated list of system names. Both the names and
+ * the list must be freed with free()
+ * The list returned ends with a "NULL" pointer
+ */
+char **tracefs_event_systems(const char *tracing_dir)
+{
+	struct dirent *dent;
+	char **systems = NULL;
+	char *events_dir;
+	struct stat st;
+	DIR *dir;
+	int len = 0;
+	int ret;
+
+	if (!tracing_dir)
+		tracing_dir = tracefs_get_tracing_dir();
+
+	if (!tracing_dir)
+		return NULL;
+
+	events_dir = append_file(tracing_dir, "events");
+	if (!events_dir)
+		return NULL;
+
+	/*
+	 * Search all the directories in the events directory,
+	 * and collect the ones that have the "enable" file.
+	 */
+	ret = stat(events_dir, &st);
+	if (ret < 0 || !S_ISDIR(st.st_mode))
+		goto out_free;
+
+	dir = opendir(events_dir);
+	if (!dir)
+		goto out_free;
+
+	while ((dent = readdir(dir))) {
+		const char *name = dent->d_name;
+		char *enable;
+		char *sys;
+
+		if (strcmp(name, ".") == 0 ||
+		    strcmp(name, "..") == 0)
+			continue;
+
+		sys = append_file(events_dir, name);
+		ret = stat(sys, &st);
+		if (ret < 0 || !S_ISDIR(st.st_mode)) {
+			free(sys);
+			continue;
+		}
+
+		enable = append_file(sys, "enable");
+
+		ret = stat(enable, &st);
+		if (ret >= 0)
+			systems = add_list_string(systems, name, len++);
+
+		free(enable);
+		free(sys);
+	}
+
+	closedir(dir);
+
+ out_free:
+	free(events_dir);
+	return systems;
+}
+
+/**
+ * tracefs_system_events - return list of events for system
+ * @tracing_dir: directory holding the "events" directory
+ * @system: the system to return the events for
+ *
+ * Returns an allocated list of event names. Both the names and
+ * the list must be freed with free()
+ * The list returned ends with a "NULL" pointer
+ */
+char **tracefs_system_events(const char *tracing_dir, const char *system)
+{
+	struct dirent *dent;
+	char **events = NULL;
+	char *system_dir = NULL;
+	struct stat st;
+	DIR *dir;
+	int len = 0;
+	int ret;
+
+	if (!tracing_dir)
+		tracing_dir = tracefs_get_tracing_dir();
+
+	if (!tracing_dir || !system)
+		return NULL;
+
+	asprintf(&system_dir, "%s/events/%s", tracing_dir, system);
+	if (!system_dir)
+		return NULL;
+
+	ret = stat(system_dir, &st);
+	if (ret < 0 || !S_ISDIR(st.st_mode))
+		goto out_free;
+
+	dir = opendir(system_dir);
+	if (!dir)
+		goto out_free;
+
+	while ((dent = readdir(dir))) {
+		const char *name = dent->d_name;
+		char *enable;
+		char *event;
+
+		if (strcmp(name, ".") == 0 ||
+		    strcmp(name, "..") == 0)
+			continue;
+
+		event = append_file(system_dir, name);
+		ret = stat(event, &st);
+		if (ret < 0 || !S_ISDIR(st.st_mode)) {
+			free(event);
+			continue;
+		}
+
+		enable = append_file(event, "enable");
+
+		ret = stat(enable, &st);
+		if (ret >= 0)
+			events = add_list_string(events, name, len++);
+
+		free(enable);
+		free(event);
+	}
+
+	closedir(dir);
+
+ out_free:
+	free(system_dir);
+
+	return events;
+}
+
+/**
+ * tracefs_local_plugins - returns an array of available tracer plugins
+ * @tracing_dir: The directory that contains the tracing directory
+ *
+ * Returns an allocate list of plugins. The array ends with NULL
+ * Both the plugin names and array must be freed with free()
+ */
+char **tracefs_local_plugins(const char *tracing_dir)
+{
+	char *available_tracers;
+	struct stat st;
+	char **plugins = NULL;
+	char *buf;
+	char *str, *saveptr;
+	char *plugin;
+	int slen;
+	int len;
+	int ret;
+
+	if (!tracing_dir)
+		return NULL;
+
+	available_tracers = append_file(tracing_dir, "available_tracers");
+	if (!available_tracers)
+		return NULL;
+
+	ret = stat(available_tracers, &st);
+	if (ret < 0)
+		goto out_free;
+
+	len = str_read_file(available_tracers, &buf);
+	if (len < 0)
+		goto out_free;
+
+	len = 0;
+	for (str = buf; ; str = NULL) {
+		plugin = strtok_r(str, " ", &saveptr);
+		if (!plugin)
+			break;
+		slen = strlen(plugin);
+		if (!slen)
+			continue;
+
+		/* chop off any newlines */
+		if (plugin[slen - 1] == '\n')
+			plugin[slen - 1] = '\0';
+
+		/* Skip the non tracers */
+		if (strcmp(plugin, "nop") == 0 ||
+		    strcmp(plugin, "none") == 0)
+			continue;
+
+		plugins = add_list_string(plugins, plugin, len++);
+	}
+	free(buf);
+
+ out_free:
+	free(available_tracers);
+
+	return plugins;
+}
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 62f67d5..1d91e87 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -4178,7 +4178,7 @@ find_ts_in_page(struct tep_handle *pevent, void *page, int size)
 		return 0;
 
 	while (!ts) {
-		record = tracecmd_read_page_record(pevent, page, size,
+		record = tracefs_read_page_record(pevent, page, size,
 						   last_record);
 		if (!record)
 			break;
-- 
2.24.1


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

* [PATCH v2 5/5] trace-cmd,kernel-shark: New libtracefs APIs for loading ftrace events
  2020-01-06 15:40 [PATCH v2 0/5] tracefs library Tzvetomir Stoyanov (VMware)
                   ` (3 preceding siblings ...)
  2020-01-06 15:40 ` [PATCH v2 4/5] trace-cmd,kernel-shark: New libtracefs APIs for ftrace events and systems Tzvetomir Stoyanov (VMware)
@ 2020-01-06 15:40 ` Tzvetomir Stoyanov (VMware)
  4 siblings, 0 replies; 15+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-01-06 15:40 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

The functionality related to loading local ftrace events
to a pet handler is moved from libtracecmd to libtracefs.

The following libtracecmd APIs are removed:
  tracecmd_local_events();
  tracecmd_fill_local_events();

The following new library APIs are introduced:
  tracefs_local_events();
  tracefs_fill_local_events();

The APIs are reimplemented to reuse functionality from
 tracefs_event_systems() and
 tracefs_system_events()

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h        |   4 -
 include/tracefs/tracefs.h            |   4 +
 kernel-shark/src/KsCaptureDialog.cpp |   2 +-
 lib/trace-cmd/trace-util.c           | 228 ---------------------------
 lib/tracefs/tracefs-events.c         | 185 ++++++++++++++++++++++
 tracecmd/trace-check-events.c        |   2 +-
 6 files changed, 191 insertions(+), 234 deletions(-)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 30bb144..da5bd2d 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -24,10 +24,6 @@ void tracecmd_parse_ftrace_printk(struct tep_handle *pevent, char *file, unsigne
 extern int tracecmd_disable_sys_plugins;
 extern int tracecmd_disable_plugins;
 
-struct tep_handle *tracecmd_local_events(const char *tracing_dir);
-int tracecmd_fill_local_events(const char *tracing_dir,
-			       struct tep_handle *pevent, int *parsing_failures);
-
 int *tracecmd_add_id(int *list, int id, int len);
 
 enum {
diff --git a/include/tracefs/tracefs.h b/include/tracefs/tracefs.h
index c66250c..998671b 100644
--- a/include/tracefs/tracefs.h
+++ b/include/tracefs/tracefs.h
@@ -49,4 +49,8 @@ int tracefs_iterate_raw_events(struct tep_handle *tep,
 
 char **tracefs_local_plugins(const char *tracing_dir);
 
+struct tep_handle *tracefs_local_events(const char *tracing_dir);
+int tracefs_fill_local_events(const char *tracing_dir,
+			       struct tep_handle *tep, int *parsing_failures);
+
 #endif /* _TRACE_FS_H */
diff --git a/kernel-shark/src/KsCaptureDialog.cpp b/kernel-shark/src/KsCaptureDialog.cpp
index 49c8755..409eb0e 100644
--- a/kernel-shark/src/KsCaptureDialog.cpp
+++ b/kernel-shark/src/KsCaptureDialog.cpp
@@ -26,7 +26,7 @@ extern "C" {
 
 static inline tep_handle *local_events()
 {
-	return tracecmd_local_events(tracefs_get_tracing_dir());
+	return tracefs_local_events(tracefs_get_tracing_dir());
 }
 
 /**
diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
index 1554e88..795e83d 100644
--- a/lib/trace-cmd/trace-util.c
+++ b/lib/trace-cmd/trace-util.c
@@ -142,17 +142,6 @@ void tracecmd_parse_ftrace_printk(struct tep_handle *pevent,
 	}
 }
 
-/* FIXME: append_file() is duplicated and could be consolidated */
-static char *append_file(const char *dir, const char *name)
-{
-	char *file;
-	int ret;
-
-	ret = asprintf(&file, "%s/%s", dir, name);
-
-	return ret < 0 ? NULL : file;
-}
-
 /**
  * tracecmd_add_id - add an int to the event id list
  * @list: list to add the id to
@@ -183,223 +172,6 @@ int *tracecmd_add_id(int *list, int id, int len)
 	return list;
 }
 
-static int read_file(const char *file, char **buffer)
-{
-	char *buf;
-	int len = 0;
-	int fd;
-	int r;
-
-	fd = open(file, O_RDONLY);
-	if (fd < 0)
-		return -1;
-
-	buf = malloc(BUFSIZ + 1);
-	if (!buf) {
-		len = -1;
-		goto out;
-	}
-
-	while ((r = read(fd, buf + len, BUFSIZ)) > 0) {
-		len += r;
-		buf = realloc(buf, len + BUFSIZ + 1);
-		if (!buf) {
-			len = -1;
-			goto out;
-		}
-	}
-
-	*buffer = buf;
-	buf[len] = 0;
- out:
-	close(fd);
-
-	return len;
-}
-
-static int load_events(struct tep_handle *pevent, const char *system,
-			const char *sys_dir)
-{
-	struct dirent *dent;
-	struct stat st;
-	DIR *dir;
-	int len = 0;
-	int ret = 0, failure = 0;
-
-	ret = stat(sys_dir, &st);
-	if (ret < 0 || !S_ISDIR(st.st_mode))
-		return EINVAL;
-
-	dir = opendir(sys_dir);
-	if (!dir)
-		return errno;
-
-	while ((dent = readdir(dir))) {
-		const char *name = dent->d_name;
-		char *event;
-		char *format;
-		char *buf;
-
-		if (strcmp(name, ".") == 0 ||
-		    strcmp(name, "..") == 0)
-			continue;
-
-		event = append_file(sys_dir, name);
-		ret = stat(event, &st);
-		if (ret < 0 || !S_ISDIR(st.st_mode))
-			goto free_event;
-
-		format = append_file(event, "format");
-		ret = stat(format, &st);
-		if (ret < 0)
-			goto free_format;
-
-		len = read_file(format, &buf);
-		if (len < 0)
-			goto free_format;
-
-		ret = tep_parse_event(pevent, buf, len, system);
-		free(buf);
- free_format:
-		free(format);
- free_event:
-		free(event);
-		if (ret)
-			failure = ret;
-	}
-
-	closedir(dir);
-	return failure;
-}
-
-static int read_header(struct tep_handle *pevent, const char *events_dir)
-{
-	struct stat st;
-	char *header;
-	char *buf;
-	int len;
-	int ret = -1;
-
-	header = append_file(events_dir, "header_page");
-
-	ret = stat(header, &st);
-	if (ret < 0)
-		goto out;
-
-	len = read_file(header, &buf);
-	if (len < 0)
-		goto out;
-
-	tep_parse_header_page(pevent, buf, len, sizeof(long));
-
-	free(buf);
-
-	ret = 0;
- out:
-	free(header);
-	return ret;
-}
-
-/**
- * tracecmd_local_events - create a pevent from the events on system
- * @tracing_dir: The directory that contains the events.
- *
- * Returns a pevent structure that contains the pevents local to
- * the system.
- */
-struct tep_handle *tracecmd_local_events(const char *tracing_dir)
-{
-	struct tep_handle *pevent = NULL;
-
-	pevent = tep_alloc();
-	if (!pevent)
-		return NULL;
-
-	if (tracecmd_fill_local_events(tracing_dir, pevent, NULL)) {
-		tep_free(pevent);
-		pevent = NULL;
-	}
-
-	return pevent;
-}
-
-/**
- * tracecmd_fill_local_events - Fill a pevent with the events on system
- * @tracing_dir: The directory that contains the events.
- * @pevent: Allocated pevent which will be filled
- * @parsing_failures: return number of failures while parsing the event files
- *
- * Returns whether the operation succeeded
- */
-int tracecmd_fill_local_events(const char *tracing_dir,
-			       struct tep_handle *pevent, int *parsing_failures)
-{
-	struct dirent *dent;
-	char *events_dir;
-	struct stat st;
-	DIR *dir;
-	int ret;
-
-	if (!tracing_dir)
-		return -1;
-	if (parsing_failures)
-		*parsing_failures = 0;
-
-	events_dir = append_file(tracing_dir, "events");
-	if (!events_dir)
-		return -1;
-
-	ret = stat(events_dir, &st);
-	if (ret < 0 || !S_ISDIR(st.st_mode)) {
-		ret = -1;
-		goto out_free;
-	}
-
-	dir = opendir(events_dir);
-	if (!dir) {
-		ret = -1;
-		goto out_free;
-	}
-
-	ret = read_header(pevent, events_dir);
-	if (ret < 0) {
-		ret = -1;
-		goto out_free;
-	}
-
-	while ((dent = readdir(dir))) {
-		const char *name = dent->d_name;
-		char *sys;
-
-		if (strcmp(name, ".") == 0 ||
-		    strcmp(name, "..") == 0)
-			continue;
-
-		sys = append_file(events_dir, name);
-		ret = stat(sys, &st);
-		if (ret < 0 || !S_ISDIR(st.st_mode)) {
-			free(sys);
-			continue;
-		}
-
-		ret = load_events(pevent, name, sys);
-
-		free(sys);
-
-		if (ret && parsing_failures)
-			(*parsing_failures)++;
-	}
-
-	closedir(dir);
-	/* always succeed because parsing failures are not critical */
-	ret = 0;
-
- out_free:
-	free(events_dir);
-
-	return ret;
-}
-
 struct add_plugin_data {
 	int ret;
 	int index;
diff --git a/lib/tracefs/tracefs-events.c b/lib/tracefs/tracefs-events.c
index 5d40f06..805db86 100644
--- a/lib/tracefs/tracefs-events.c
+++ b/lib/tracefs/tracefs-events.c
@@ -474,3 +474,188 @@ char **tracefs_local_plugins(const char *tracing_dir)
 
 	return plugins;
 }
+
+static int load_events(struct tep_handle *tep,
+		       const char *tracing_dir, const char *system)
+{
+	int ret = 0, failure = 0;
+	char **events = NULL;
+	struct stat st;
+	int len = 0;
+	int i;
+
+	events = tracefs_system_events(tracing_dir, system);
+	if (!events)
+		return -ENOENT;
+
+	for (i = 0; events[i]; i++) {
+		char *format;
+		char *buf;
+
+		ret = asprintf(&format, "%s/events/%s/%s/format",
+			       tracing_dir, system, events[i]);
+		if (ret < 0) {
+			failure = -ENOMEM;
+			break;
+		}
+
+		ret = stat(format, &st);
+		if (ret < 0)
+			goto next_event;
+
+		len = str_read_file(format, &buf);
+		if (len < 0)
+			goto next_event;
+
+		ret = tep_parse_event(tep, buf, len, system);
+		free(buf);
+next_event:
+		free(format);
+		if (ret)
+			failure = ret;
+	}
+
+	if (events) {
+		for (i = 0; events[i]; i++)
+			free(events[i]);
+		free(events);
+	}
+	return failure;
+}
+
+static int read_header(struct tep_handle *tep, const char *tracing_dir)
+{
+	struct stat st;
+	char *header;
+	char *buf;
+	int len;
+	int ret = -1;
+
+	header = append_file(tracing_dir, "events/header_page");
+
+	ret = stat(header, &st);
+	if (ret < 0)
+		goto out;
+
+	len = str_read_file(header, &buf);
+	if (len < 0)
+		goto out;
+
+	tep_parse_header_page(tep, buf, len, sizeof(long));
+
+	free(buf);
+
+	ret = 0;
+ out:
+	free(header);
+	return ret;
+}
+
+static bool contains(const char *name, const char * const *names)
+{
+	if (!names)
+		return false;
+	for (; *names; names++)
+		if (strcmp(name, *names) == 0)
+			return true;
+	return false;
+}
+
+static int tracecmd_fill_local_events_system(const char *tracing_dir,
+					     struct tep_handle *tep,
+					     const char * const *sys_names,
+					     int *parsing_failures)
+{
+	char **systems = NULL;
+	int ret;
+	int i;
+
+	if (!tracing_dir)
+		tracing_dir = tracefs_get_tracing_dir();
+	if (!tracing_dir)
+		return -1;
+
+	systems = tracefs_event_systems(tracing_dir);
+	if (!systems)
+		return -1;
+
+	ret = read_header(tep, tracing_dir);
+	if (ret < 0) {
+		ret = -1;
+		goto out;
+	}
+
+	if (parsing_failures)
+		*parsing_failures = 0;
+
+	for (i = 0; systems[i]; i++) {
+		if (sys_names && !contains(systems[i], sys_names))
+			continue;
+		ret = load_events(tep, tracing_dir, systems[i]);
+		if (ret && parsing_failures)
+			(*parsing_failures)++;
+	}
+	/* always succeed because parsing failures are not critical */
+	ret = 0;
+out:
+	if (systems) {
+		for (i = 0; systems[i]; i++)
+			free(systems[i]);
+		free(systems);
+	}
+	return ret;
+}
+
+/**
+ * tracecmd_local_events_system - create a tep from the events of the specified subsystem.
+ *
+ * @tracing_dir: The directory that contains the events.
+ * @sys_name: Array of system names, to load the events from.
+ * The last element from the array must be NULL
+ *
+ * Returns a tep structure that contains the tep local to
+ * the system.
+ */
+struct tep_handle *tracecmd_local_events_system(const char *tracing_dir,
+						const char * const *sys_names)
+{
+	struct tep_handle *tep = NULL;
+
+	tep = tep_alloc();
+	if (!tep)
+		return NULL;
+
+	if (tracecmd_fill_local_events_system(tracing_dir, tep, sys_names, NULL)) {
+		tep_free(tep);
+		tep = NULL;
+	}
+
+	return tep;
+}
+
+/**
+ * tracefs_local_events - create a tep from the events on system
+ * @tracing_dir: The directory that contains the events.
+ *
+ * Returns a tep structure that contains the teps local to
+ * the system.
+ */
+struct tep_handle *tracefs_local_events(const char *tracing_dir)
+{
+	return tracecmd_local_events_system(tracing_dir, NULL);
+}
+
+/**
+ * tracefs_fill_local_events - Fill a tep with the events on system
+ * @tracing_dir: The directory that contains the events.
+ * @tep: Allocated tep handler which will be filled
+ * @parsing_failures: return number of failures while parsing the event files
+ *
+ * Returns whether the operation succeeded
+ */
+int tracefs_fill_local_events(const char *tracing_dir,
+			       struct tep_handle *tep, int *parsing_failures)
+{
+	return tracecmd_fill_local_events_system(tracing_dir, tep,
+						 NULL, parsing_failures);
+}
diff --git a/tracecmd/trace-check-events.c b/tracecmd/trace-check-events.c
index a8ee60a..d37af67 100644
--- a/tracecmd/trace-check-events.c
+++ b/tracecmd/trace-check-events.c
@@ -50,7 +50,7 @@ void trace_check_events(int argc, char **argv)
 		tep_set_flag(pevent, TEP_DISABLE_SYS_PLUGINS);
 
 	list = tep_load_plugins(pevent);
-	ret = tracecmd_fill_local_events(tracing, pevent, &parsing_failures);
+	ret = tracefs_fill_local_events(tracing, pevent, &parsing_failures);
 	if (ret || parsing_failures)
 		ret = EINVAL;
 	tep_unload_plugins(list, pevent);
-- 
2.24.1


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

* Re: [PATCH v2 2/5] kernel-shark: Use new tracefs library
  2020-01-06 15:40 ` [PATCH v2 2/5] kernel-shark: Use new tracefs library Tzvetomir Stoyanov (VMware)
@ 2020-01-08 19:05   ` Steven Rostedt
  2020-01-08 20:09   ` Steven Rostedt
  1 sibling, 0 replies; 15+ messages in thread
From: Steven Rostedt @ 2020-01-08 19:05 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Mon,  6 Jan 2020 17:40:55 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Link libtracefs to kernel-shark and use its API:
>   tracefs_get_tracing_dir()
> 

This needs to be merged into the first patch, because the first patch
breaks the build, with breaks bisectability, which is a no-no.

-- Steve

> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  kernel-shark/CMakeLists.txt           |  3 ++-
>  kernel-shark/build/FindTraceCmd.cmake | 30 +++++++++++++++++++++++++++
>  kernel-shark/src/CMakeLists.txt       |  2 ++
>  kernel-shark/src/KsCaptureDialog.cpp  |  4 ++--
>  kernel-shark/src/libkshark.h          |  1 +
>  5 files changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel-shark/CMakeLists.txt b/kernel-shark/CMakeLists.txt
> index 20478b9..8786b83 100644
> --- a/kernel-shark/CMakeLists.txt
> +++ b/kernel-shark/CMakeLists.txt
> @@ -76,7 +76,8 @@ endif (CMAKE_BUILD_TYPE MATCHES Package)
>  include_directories(${KS_DIR}/src/
>                      ${KS_DIR}/build/src/
>                      ${JSONC_INCLUDE_DIR}
> -                    ${TRACECMD_INCLUDE_DIR})
> +                    ${TRACECMD_INCLUDE_DIR}
> +                    ${TRACEFS_INCLUDE_DIR})
>  
>  message("")
>  message(STATUS "C flags      : " ${CMAKE_C_FLAGS})
> diff --git a/kernel-shark/build/FindTraceCmd.cmake b/kernel-shark/build/FindTraceCmd.cmake
> index d3e145c..f27fafe 100644
> --- a/kernel-shark/build/FindTraceCmd.cmake
> +++ b/kernel-shark/build/FindTraceCmd.cmake
> @@ -6,6 +6,8 @@
>  #  TRACEEVENT_FOUND, If false, do not try to use traceevent.
>  #
>  #  TRACECMD_INCLUDE_DIR, where to find trace-cmd header.
> +#  TRACEFS_INCLUDE_DIR, where to find tracefs header.
> +#  TRACEFS_LIBRARY, the tracefs library.
>  #  TRACECMD_LIBRARY, the trace-cmd library.
>  #  TRACECMD_FOUND, If false, do not try to use trace-cmd.
>  
> @@ -31,12 +33,21 @@ find_path(TRACECMD_INCLUDE_DIR  NAMES  trace-cmd/trace-cmd.h
>                                  PATHS  $ENV{TRACE_CMD}/include/
>                                         ${CMAKE_SOURCE_DIR}/../include/
>                                  NO_DEFAULT_PATH)
> +find_path(TRACEFS_INCLUDE_DIR   NAMES  tracefs/tracefs.h
> +                                PATHS  $ENV{TRACE_CMD}/include/
> +                                       ${CMAKE_SOURCE_DIR}/../include/
> +                                NO_DEFAULT_PATH)
>  
>  find_library(TRACECMD_LIBRARY   NAMES  trace-cmd/libtracecmd.a
>                                  PATHS  $ENV{TRACE_CMD}/lib/
>                                         ${CMAKE_SOURCE_DIR}/../lib/
>                                  NO_DEFAULT_PATH)
>  
> +find_library(TRACEFS_LIBRARY    NAMES  trace-cmd/libtracefs.a
> +                                PATHS  $ENV{TRACE_CMD}/lib/
> +                                       ${CMAKE_SOURCE_DIR}/../lib/
> +                                NO_DEFAULT_PATH)
> +
>  find_library(TRACEEVENT_LIBRARY NAMES  traceevent/libtraceevent.a
>                                  PATHS  $ENV{TRACE_CMD}/lib/
>                                         ${CMAKE_SOURCE_DIR}/../lib/
> @@ -46,7 +57,9 @@ find_library(TRACEEVENT_LIBRARY NAMES  traceevent/libtraceevent.a
>  # search was successful "find_path" will do nothing this time.
>  find_program(TRACECMD_EXECUTABLE   NAMES  trace-cmd)
>  find_path(TRACECMD_INCLUDE_DIR  NAMES  trace-cmd/trace-cmd.h)
> +find_path(TRACEFS_INCLUDE_DIR   NAMES  tracefs/tracefs.h)
>  find_library(TRACECMD_LIBRARY   NAMES  trace-cmd/libtracecmd.so)
> +find_library(TRACEFS_LIBRARY    NAMES  tracefs/libtracefs.so)
>  find_library(TRACEEVENT_LIBRARY NAMES  traceevent/libtraceevent.so)
>  
>  IF (TRACECMD_INCLUDE_DIR AND TRACECMD_LIBRARY)
> @@ -65,6 +78,23 @@ ELSE (TRACECMD_FOUND)
>  
>  ENDIF (TRACECMD_FOUND)
>  
> +IF (TRACEFS_INCLUDE_DIR AND TRACEFS_LIBRARY)
> +
> +  SET(TRACEFS_FOUND TRUE)
> +
> +ENDIF (TRACEFS_INCLUDE_DIR AND TRACEFS_LIBRARY)
> +
> +IF (TRACEFS_FOUND)
> +
> +  MESSAGE(STATUS "Found tracefs: ${TRACEFS_LIBRARY}")
> +
> +ELSE (TRACEFS_FOUND)
> +
> +  MESSAGE(FATAL_ERROR "\nCould not find tracefs!\n")
> +
> +ENDIF (TRACEFS_FOUND)
> +
> +
>  IF (TRACEEVENT_LIBRARY)
>  
>    SET(TRACEEVENT_FOUND TRUE)
> diff --git a/kernel-shark/src/CMakeLists.txt b/kernel-shark/src/CMakeLists.txt
> index e20a030..33b5db8 100644
> --- a/kernel-shark/src/CMakeLists.txt
> +++ b/kernel-shark/src/CMakeLists.txt
> @@ -9,6 +9,7 @@ add_library(kshark SHARED libkshark.c
>  
>  target_link_libraries(kshark ${TRACEEVENT_LIBRARY}
>                               ${TRACECMD_LIBRARY}
> +                             ${TRACEFS_LIBRARY}
>                               ${JSONC_LIBRARY}
>                               ${CMAKE_DL_LIBS})
>  
> @@ -69,6 +70,7 @@ if (Qt5Widgets_FOUND AND Qt5Network_FOUND)
>                                       Qt5::Network
>                                       ${TRACEEVENT_LIBRARY}
>                                       ${TRACECMD_LIBRARY}
> +                                     ${TRACEFS_LIBRARY}
>                                       ${CMAKE_DL_LIBS})
>  
>      set_target_properties(kshark-gui PROPERTIES  SUFFIX ".so.${KS_VERSION_STRING}")
> diff --git a/kernel-shark/src/KsCaptureDialog.cpp b/kernel-shark/src/KsCaptureDialog.cpp
> index ad05917..548b6fb 100644
> --- a/kernel-shark/src/KsCaptureDialog.cpp
> +++ b/kernel-shark/src/KsCaptureDialog.cpp
> @@ -26,7 +26,7 @@ extern "C" {
>  
>  static inline tep_handle *local_events()
>  {
> -	return tracecmd_local_events(tracecmd_get_tracing_dir());
> +	return tracecmd_local_events(tracefs_get_tracing_dir());
>  }
>  
>  /**
> @@ -204,7 +204,7 @@ QStringList KsCaptureControl::_getPlugins()
>  	QStringList pluginList;
>  	char **all_plugins;
>  
> -	all_plugins = tracecmd_local_plugins(tracecmd_get_tracing_dir());
> +	all_plugins = tracecmd_local_plugins(tracefs_get_tracing_dir());
>  
>  	if (!all_plugins)
>  		return pluginList;
> diff --git a/kernel-shark/src/libkshark.h b/kernel-shark/src/libkshark.h
> index 3407db1..b05aa90 100644
> --- a/kernel-shark/src/libkshark.h
> +++ b/kernel-shark/src/libkshark.h
> @@ -28,6 +28,7 @@ extern "C" {
>  #include "trace-cmd/trace-cmd.h"
>  #include "trace-cmd/trace-filter-hash.h"
>  #include "traceevent/event-parse.h"
> +#include "tracefs/tracefs.h"
>  
>  // KernelShark
>  #include "libkshark-plugin.h"


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

* Re: [PATCH v2 3/5] trace-cmd: New libtracefs APIs for ftrace instances
  2020-01-06 15:40 ` [PATCH v2 3/5] trace-cmd: New libtracefs APIs for ftrace instances Tzvetomir Stoyanov (VMware)
@ 2020-01-08 19:37   ` Steven Rostedt
  2020-01-09 13:29     ` Tzvetomir Stoyanov
  0 siblings, 1 reply; 15+ messages in thread
From: Steven Rostedt @ 2020-01-08 19:37 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Mon,  6 Jan 2020 17:40:56 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> The functionality related to ftrace instances is moved from
> trace-cmd application to libtracefs. The following new
> library APIs are introduced:
>     struct tracefs_instance;
>     struct tracefs_instance *tracefs_alloc_instance(const char *name);
>     int tracefs_free_instance(struct tracefs_instance *instance);
>     int tracefs_make_instance(struct tracefs_instance *instance);
>     int tracefs_remove_instance(struct tracefs_instance *instance);

I'm thinking that "make" should be "create" and "remove" should be
"destroy". The "make" and "remove" does match "mkdir" and "rmdir" but
that's just the behind-the-scenes of what the OS does. But from an
application point of view, "create" and "destroy" seem more
appropriate.

What do you think?


>     char *tracefs_get_instance_name(struct tracefs_instance *instance);
>     char *tracefs_get_instance_file(struct tracefs_instance *instance, const char *file);
>     char *tracefs_get_instance_dir(struct tracefs_instance *instance);
>     int tracefs_write_instance_file(struct tracefs_instance *instance,
> 				 const char *file, const char *str, const char *type);
>     char *tracefs_read_instance_file(struct tracefs_instance *instance, char *file, int *psize);

Also, another naming convention used commonly, is when you have a set
of functions working on the same thing, is to use the name of what it
is working on first. That is, instead of the above, we should have:

 tracefs_instance_alloc()
 tracefs_instance_free()
 tracefs_instance_create() (instead of "make")
 tracefs_instance_destroy() (instead of "remove")
 tracefs_instance_get_name()
 tracefs_instance_get_file()
 tracefs_instance_file_read()
 tracefs_instance_file_write()

That way it is easier to find these functions with just searching for
"tracefs_instance"

The "get_*" is ok to keep, but as I showed above, I think "_file_read"
and "_file_write" is a better name.

[ more below ]

> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  include/tracefs/tracefs.h           |  17 ++
>  lib/tracefs/Makefile                |   1 +
>  lib/tracefs/include/tracefs-local.h |   1 +
>  lib/tracefs/tracefs-instance.c      | 267 ++++++++++++++++++++++++++++
>  lib/tracefs/tracefs-utils.c         |  43 +++++
>  tracecmd/include/trace-local.h      |   5 +-
>  tracecmd/trace-list.c               |   2 +-
>  tracecmd/trace-record.c             | 264 +++++++++------------------
>  tracecmd/trace-show.c               |   2 +
>  tracecmd/trace-stat.c               |  21 ++-
>  10 files changed, 431 insertions(+), 192 deletions(-)
>  create mode 100644 lib/tracefs/tracefs-instance.c
> 
> diff --git a/include/tracefs/tracefs.h b/include/tracefs/tracefs.h
> index e844c75..6b27ff7 100644
> --- a/include/tracefs/tracefs.h
> +++ b/include/tracefs/tracefs.h
> @@ -17,4 +17,21 @@ const char *tracefs_get_tracing_dir(void);
>  /* tracefs_find_tracing_dir must be freed */
>  char *tracefs_find_tracing_dir(void);
>  
> +/* ftarce instances */
> +struct tracefs_instance;
> +
> +struct tracefs_instance *tracefs_alloc_instance(const char *name);
> +int tracefs_free_instance(struct tracefs_instance *instance);
> +int tracefs_make_instance(struct tracefs_instance *instance);
> +int tracefs_remove_instance(struct tracefs_instance *instance);
> +char *tracefs_get_instance_name(struct tracefs_instance *instance);
> +char *
> +tracefs_get_instance_file(struct tracefs_instance *instance, const char *file);
> +char *tracefs_get_instance_dir(struct tracefs_instance *instance);
> +int tracefs_write_instance_file(struct tracefs_instance *instance,
> +				const char *file, const char *str,
> +				const char *type);
> +char *tracefs_read_instance_file(struct tracefs_instance *instance,
> +				 char *file, int *psize);
> +
>  #endif /* _TRACE_FS_H */
> diff --git a/lib/tracefs/Makefile b/lib/tracefs/Makefile
> index 86d7845..4030272 100644
> --- a/lib/tracefs/Makefile
> +++ b/lib/tracefs/Makefile
> @@ -8,6 +8,7 @@ DEFAULT_TARGET = $(bdir)/libtracefs.a
>  
>  OBJS =
>  OBJS += tracefs-utils.o
> +OBJS += tracefs-instance.o
>  
>  OBJS := $(OBJS:%.o=$(bdir)/%.o)
>  DEPS := $(OBJS:$(bdir)/%.o=$(bdir)/.%.d)
> diff --git a/lib/tracefs/include/tracefs-local.h b/lib/tracefs/include/tracefs-local.h
> index 231edd1..fe327a0 100644
> --- a/lib/tracefs/include/tracefs-local.h
> +++ b/lib/tracefs/include/tracefs-local.h
> @@ -8,5 +8,6 @@
>  
>  /* Can be overridden */
>  void warning(const char *fmt, ...);
> +int str_read_file(const char *file, char **buffer);
>  
>  #endif /* _TRACE_FS_LOCAL_H */
> diff --git a/lib/tracefs/tracefs-instance.c b/lib/tracefs/tracefs-instance.c
> new file mode 100644
> index 0000000..14e8eed
> --- /dev/null
> +++ b/lib/tracefs/tracefs-instance.c
> @@ -0,0 +1,267 @@
> +// SPDX-License-Identifier: LGPL-2.1
> +/*
> + * Copyright (C) 2008, 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
> + *
> + * Updates:
> + * Copyright (C) 2019, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
> + *
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +
> +#include "tracefs.h"
> +#include "tracefs-local.h"
> +
> +struct tracefs_instance {
> +	char *name;
> +};
> +
> +/**
> + * tracefs_alloc_instance - allocate a new ftrace instance
> + * @name: The name of the instance (instance will point to this)
> + *
> + * Returns a newly allocated instance, or NULL in case of an error.
> + */
> +struct tracefs_instance *tracefs_alloc_instance(const char *name)
> +{
> +	struct tracefs_instance *instance;
> +
> +	instance = calloc(1, sizeof(*instance));
> +	if (!instance)
> +		return NULL;
> +	if (name)
> +		instance->name = strdup(name);

We can remove make this a little simpler (and with the name check):

	instance = calloc(1, sizeof(*instance));
	if (instance && name) {
		instance->name = strdup(name);
		if (!instance->name) {
			free(instance);
			instance = NULL;
		}
	}

	return instance;

> +
> +	return instance;
> +}
> +
> +/**
> + * tracefs_free_instance - Free an instance, previously allocated by
> +			   tracefs_alloc_instance()
> + * @instance: Pointer to the instance to be freed
> + *
> + * Returns -1 in case of an error, or 0 otherwise.

I don't think the free should return a value. Make it void.

> + */
> +int tracefs_free_instance(struct tracefs_instance *instance)
> +{
> +	if (!instance)
> +		return -1;
> +
> +	free(instance->name);
> +	free(instance);
> +	return 0;
> +}
> +
> +/**
> + * tracefs_make_instance - Create a new ftrace instance
> + * @instance: Pointer to the instance to be created
> + *
> + * Returns 1 if the instance already exist, 0 if the instance
> + * is created successful or -1 in case of an error
> + */
> +int tracefs_make_instance(struct tracefs_instance *instance)
> +{
> +	struct stat st;
> +	char *path;
> +	int ret;
> +
> +	path = tracefs_get_instance_dir(instance);
> +	ret = stat(path, &st);
> +	if (ret < 0) {
> +		ret = mkdir(path, 0777);
> +		if (ret < 0)
> +			return ret;

Remove the check of ret here, as the return skips the freeing of path.

Should be just:

	if (ret < 0)
		ret = mkdir(path, 0777);
	else
		ret = 1;

> +
> +	} else
> +		ret = 1;
> +	tracefs_put_tracing_file(path);
> +	return ret;
> +}
> +
> +/**
> + * tracefs_remove_instance - Remove a ftrace instance
> + * @instance: Pointer to the instance to be removed
> + *
> + * Returns -1 in case of an error, or 0 otherwise.
> + */
> +int tracefs_remove_instance(struct tracefs_instance *instance)
> +{
> +	char *path;
> +	int ret;
> +
> +	if (!instance || !instance->name) {
> +		warning("Cannot remove top instance");
> +		return -1;
> +	}
> +
> +	path = tracefs_get_instance_dir(instance);

path could be NULL here due to failed allocation. Need to check that
and return an error.

> +	ret = rmdir(path);
> +	tracefs_put_tracing_file(path);
> +
> +	return ret;
> +}
> +
> +/**
> + * tracefs_get_instance_file - return the path to an instance file.
> + * @instance: ftrace instance, can be NULL for the top instance
> + * @file: name of file to return
> + *
> + * Returns the path of the @file for the given @instance, or NULL in
> + * case of an error.
> + *
> + * Must use tracefs_put_tracing_file() to free the returned string.
> + */
> +char *
> +tracefs_get_instance_file(struct tracefs_instance *instance, const char *file)
> +{
> +	char *path;
> +	char *buf;
> +	int ret;
> +
> +	if (instance && instance->name) {
> +		ret = asprintf(&buf, "instances/%s/%s", instance->name, file);
> +		if (ret < 0) {
> +			warning("Failed to allocate name for %s/%s",
> +				 instance->name, file);

For this being in a library, we should probably remove warnings.

> +			return NULL;
> +		}
> +		path = tracefs_get_tracing_file(buf);
> +		free(buf);
> +	} else
> +		path = tracefs_get_tracing_file(file);
> +
> +	return path;
> +}
> +
> +/**
> + * tracefs_get_instance_dir - return the path to the instance directory.
> + * @instance: ftrace instance, can be NULL for the top instance
> + *
> + * Returns the full path to the instance directory
> + *
> + * Must use tracefs_put_tracing_file() to free the returned string.
> + */
> +char *tracefs_get_instance_dir(struct tracefs_instance *instance)
> +{
> +	char *buf;
> +	char *path;
> +	int ret;
> +
> +	if (instance && instance->name) {
> +		ret = asprintf(&buf, "instances/%s", instance->name);
> +		if (ret < 0) {
> +			warning("Failed to allocate path for instance %s",
> +				 instance->name);
> +			return NULL;
> +		}
> +		path = tracefs_get_tracing_file(buf);
> +		free(buf);
> +	} else
> +		path = tracefs_find_tracing_dir();
> +
> +	return path;
> +}
> +
> +/**
> + * tracefs_get_instance_name - return the name of an instance
> + * @instance: ftrace instance
> + *
> + * Returns the name of the given @instance.
> + * The returned string must *not* be freed.
> + */
> +char *tracefs_get_instance_name(struct tracefs_instance *instance)
> +{
> +	if (instance)
> +		return instance->name;
> +	return NULL;
> +}
> +
> +static int write_file(const char *file, const char *str, const char *type)
> +{
> +	char buf[BUFSIZ];
> +	int ret;
> +	int fd;
> +
> +	fd = open(file, O_WRONLY | O_TRUNC);
> +	if (fd < 0) {
> +		warning("Failed to open '%s'", file);
> +		return -1;
> +	}
> +	ret = write(fd, str, strlen(str));
> +	close(fd);
> +	if (ret < 0 && type) {
> +		/* write failed */
> +		fd = open(file, O_RDONLY);
> +		if (fd < 0) {
> +			warning("Failed to write in '%s'", file);
> +			return -1;
> +		}
> +
> +		while ((ret = read(fd, buf, BUFSIZ)) > 0)
> +			fprintf(stderr, "%.*s", ret, buf);
> +		warning("Failed %s of %s\n", type, file);

The library function should not bother reading the file on error. This
is only applicable for some (one or two) files in the instance
directory. Thus remove this code.

> +		close(fd);
> +	}
> +	return ret;
> +}
> +
> +
> +/**
> + * tracefs_write_instance_file - Write in trace file of specific instance.
> + * @instance: ftrace instance, can be NULL for the top instance
> + * @file: name of the file
> + * @str: nul terminated string, that will be written in the file.
> + * @type: nul terminated string, describing the current write operation.
> + *	  Used for logging purposes.
> + *
> + * Returns the number of written bytes, or -1 in case of an error
> + */
> +int tracefs_write_instance_file(struct tracefs_instance *instance,
> +				 const char *file, const char *str,
> +				 const char *type)

Let's get rid of the "type", it's confusing for a library function. If
the output should be read, that should be for a different operation.


> +{
> +	struct stat st;
> +	char *path;
> +	int ret;
> +
> +	path = tracefs_get_instance_file(instance, file);

Need to check for path == NULL.

> +	ret = stat(path, &st);
> +	if (ret == 0)
> +		ret = write_file(path, str, type);
> +	tracefs_put_tracing_file(path);
> +
> +	return ret;
> +}
> +
> +/**
> + * tracefs_read_instance_file - Read from a trace file of specific instance.
> + * @instance: ftrace instance, can be NULL for the top instance
> + * @file: name of the file
> + * @psize: returns the number of bytes read
> + *
> + * Returns a pointer to a nul terminated string, read from the file, or NULL in
> + * case of an error.
> + * The return string must be freed by free()
> + */
> +char *tracefs_read_instance_file(struct tracefs_instance *instance,
> +				  char *file, int *psize)
> +{
> +	char *buf = NULL;
> +	int size = 0;
> +	char *path;
> +
> +	path = tracefs_get_instance_file(instance, file);

Need to check for path == NULL.

> +
> +	size = str_read_file(path, &buf);
> +
> +	tracefs_put_tracing_file(path);
> +	if (buf && psize)
> +		*psize = size;
> +
> +	return buf;
> +}
> diff --git a/lib/tracefs/tracefs-utils.c b/lib/tracefs/tracefs-utils.c
> index 0ef16fc..bdab130 100644
> --- a/lib/tracefs/tracefs-utils.c
> +++ b/lib/tracefs/tracefs-utils.c
> @@ -10,6 +10,9 @@
>  #include <sys/mount.h>
>  #include <sys/stat.h>
>  #include <linux/limits.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <unistd.h>
>  
>  #include "tracefs.h"
>  
> @@ -181,3 +184,43 @@ void tracefs_put_tracing_file(char *name)
>  {
>  	free(name);
>  }
> +
> +int str_read_file(const char *file, char **buffer)
> +{
> +	char stbuf[BUFSIZ];
> +	char *buf = NULL;
> +	int size = 0;
> +	char *nbuf;
> +	int fd;
> +	int r;
> +
> +	fd = open(file, O_RDONLY);
> +	if (fd < 0) {
> +		warning("File %s not found", file);
> +		return -1;
> +	}
> +
> +	do {
> +		r = read(fd, stbuf, BUFSIZ);
> +		if (r <= 0)
> +			continue;
> +		nbuf = realloc(buf, size+r+1);
> +		if (!nbuf) {
> +			warning("Failed to allocate file buffer");

Again, we should remove warnings from the library function. But this
can be done later. I just wanted to document that we need to do that.

> +			size = -1;
> +			break;
> +		}
> +		buf = nbuf;
> +		memcpy(buf+size, stbuf, r);
> +		size += r;
> +	} while (r);

Hmm, this probably should be:

	} while (r > 0);

(I know this was broken before this patch)

> +
> +	close(fd);
> +	if (size > 0) {

Hmm, if r is less than zero here, we have a bug. Perhaps this needs to
be:

	if (r == 0 && size > 0) {

> +		buf[size] = '\0';
> +		*buffer = buf;
> +	} else
> +		free(buf);
> +
> +	return size;
> +}


-- Steve

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

* Re: [PATCH v2 2/5] kernel-shark: Use new tracefs library
  2020-01-06 15:40 ` [PATCH v2 2/5] kernel-shark: Use new tracefs library Tzvetomir Stoyanov (VMware)
  2020-01-08 19:05   ` Steven Rostedt
@ 2020-01-08 20:09   ` Steven Rostedt
  1 sibling, 0 replies; 15+ messages in thread
From: Steven Rostedt @ 2020-01-08 20:09 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Mon,  6 Jan 2020 17:40:55 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> diff --git a/kernel-shark/build/FindTraceCmd.cmake b/kernel-shark/build/FindTraceCmd.cmake
> index d3e145c..f27fafe 100644
> --- a/kernel-shark/build/FindTraceCmd.cmake
> +++ b/kernel-shark/build/FindTraceCmd.cmake
> @@ -6,6 +6,8 @@
>  #  TRACEEVENT_FOUND, If false, do not try to use traceevent.
>  #
>  #  TRACECMD_INCLUDE_DIR, where to find trace-cmd header.
> +#  TRACEFS_INCLUDE_DIR, where to find tracefs header.
> +#  TRACEFS_LIBRARY, the tracefs library.
>  #  TRACECMD_LIBRARY, the trace-cmd library.
>  #  TRACECMD_FOUND, If false, do not try to use trace-cmd.
>  
> @@ -31,12 +33,21 @@ find_path(TRACECMD_INCLUDE_DIR  NAMES  trace-cmd/trace-cmd.h
>                                  PATHS  $ENV{TRACE_CMD}/include/
>                                         ${CMAKE_SOURCE_DIR}/../include/
>                                  NO_DEFAULT_PATH)
> +find_path(TRACEFS_INCLUDE_DIR   NAMES  tracefs/tracefs.h
> +                                PATHS  $ENV{TRACE_CMD}/include/
> +                                       ${CMAKE_SOURCE_DIR}/../include/
> +                                NO_DEFAULT_PATH)
>  
>  find_library(TRACECMD_LIBRARY   NAMES  trace-cmd/libtracecmd.a
>                                  PATHS  $ENV{TRACE_CMD}/lib/
>                                         ${CMAKE_SOURCE_DIR}/../lib/
>                                  NO_DEFAULT_PATH)
>  
> +find_library(TRACEFS_LIBRARY    NAMES  trace-cmd/libtracefs.a

I believe the above should be "tracefs/libtracefs.a" (I had to change
this to get it to compile).

-- Steve

> +                                PATHS  $ENV{TRACE_CMD}/lib/
> +                                       ${CMAKE_SOURCE_DIR}/../lib/
> +                                NO_DEFAULT_PATH)
> +
>  find_library(TRACEEVENT_LIBRARY NAMES  traceevent/libtraceevent.a
>                                  PATHS  $ENV{TRACE_CMD}/lib/
>                                         ${CMAKE_SOURCE_DIR}/../lib/

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

* Re: [PATCH v2 4/5] trace-cmd,kernel-shark: New libtracefs APIs for ftrace events and systems
  2020-01-06 15:40 ` [PATCH v2 4/5] trace-cmd,kernel-shark: New libtracefs APIs for ftrace events and systems Tzvetomir Stoyanov (VMware)
@ 2020-01-08 20:22   ` Steven Rostedt
  2020-01-09 13:37     ` Tzvetomir Stoyanov
  2020-01-08 21:09   ` Steven Rostedt
  2020-01-09  2:32   ` Steven Rostedt
  2 siblings, 1 reply; 15+ messages in thread
From: Steven Rostedt @ 2020-01-08 20:22 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Mon,  6 Jan 2020 17:40:57 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> The functionality related to ftrace events and systems
> is moved from trace-cmd application and libtracecmd to libtracefs.
> 
> The following libtracecmd APIs are removed:
>   tracecmd_read_page_record();
>   tracecmd_event_systems();
>   tracecmd_system_events();
>   tracecmd_local_plugins();
>   tracecmd_add_list();
>   tracecmd_free_list();
> 
> The following new library APIs are introduced:
>   tracefs_read_page_record();
>   tracefs_event_systems();
>   tracefs_system_events();
>   tracefs_local_plugins();

Hmm, we need to discuss plugins a bit more. I'm not sure they are
needed for either libtracecmd nor libtracefs. We have plugins for
libtraceevent which is just a better way to present the event, but I'm
imagining that plugins are going to be more specific to trace-cmd
directly. Maybe they can be packaged with libtracecmd, but I'm not
seeing a need for libtracefs.

What do you have in mind?

-- Steve


>   tracefs_iterate_raw_events();
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
> 

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

* Re: [PATCH v2 4/5] trace-cmd,kernel-shark: New libtracefs APIs for ftrace events and systems
  2020-01-06 15:40 ` [PATCH v2 4/5] trace-cmd,kernel-shark: New libtracefs APIs for ftrace events and systems Tzvetomir Stoyanov (VMware)
  2020-01-08 20:22   ` Steven Rostedt
@ 2020-01-08 21:09   ` Steven Rostedt
  2020-01-09 14:14     ` Tzvetomir Stoyanov
  2020-01-09  2:32   ` Steven Rostedt
  2 siblings, 1 reply; 15+ messages in thread
From: Steven Rostedt @ 2020-01-08 21:09 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Mon,  6 Jan 2020 17:40:57 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:


> --- /dev/null
> +++ b/lib/tracefs/tracefs-events.c
> @@ -0,0 +1,476 @@
> +// SPDX-License-Identifier: LGPL-2.1
> +/*
> + * Copyright (C) 2008, 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
> + *
> + * Updates:
> + * Copyright (C) 2019, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
> + *
> + */
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <dirent.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +
> +#include "kbuffer.h"
> +#include "tracefs.h"
> +#include "tracefs-local.h"
> +
> +/**
> + * tracefs_read_page_record - read a record off of a page
> + * @tep: tep used to parse the page
> + * @page: the page to read
> + * @size: the size of the page
> + * @last_record: last record read from this page
> + *
> + * If a ring buffer page is available, and the need to parse it
> + * without having a handle, then this function can be used

Not having a handle to what? Ah, I wrote this back in 2011. And that
"handle" meant a tracecmd_input_handle. Hmm, I think we need a
tracefs_handle of some sort for this, because it will slow down
tracefs_iterate_raw_events() very much so).


> + *
> + * The @tep needs to be initialized to have the page header information
> + * already available.
> + *
> + * The @last_record is used to know where to read the next record from
> + * If @last_record is NULL, the first record on the page will be read
> + *
> + * Returns:
> + *  A newly allocated record that must be freed with free_record() if

Hmm, free_record() needs a prefix ("tracefs_") ?

> + *  a record is found. Otherwise NULL is returned if the record is bad
> + *  or no more records exist
> + */
> +struct tep_record *
> +tracefs_read_page_record(struct tep_handle *tep, void *page, int size,
> +			  struct tep_record *last_record)
> +{
> +	unsigned long long ts;
> +	struct kbuffer *kbuf;
> +	struct tep_record *record = NULL;
> +	enum kbuffer_long_size long_size;
> +	enum kbuffer_endian endian;
> +	void *ptr;
> +
> +	if (tep_is_file_bigendian(tep))
> +		endian = KBUFFER_ENDIAN_BIG;
> +	else
> +		endian = KBUFFER_ENDIAN_LITTLE;
> +
> +	if (tep_get_header_page_size(tep) == 8)
> +		long_size = KBUFFER_LSIZE_8;
> +	else
> +		long_size = KBUFFER_LSIZE_4;
> +
> +	kbuf = kbuffer_alloc(long_size, endian);
> +	if (!kbuf)
> +		return NULL;
> +
> +	kbuffer_load_subbuffer(kbuf, page);
> +	if (kbuffer_subbuffer_size(kbuf) > size) {
> +		warning("%s: page_size > size", __func__);
> +		goto out_free;
> +	}
> +
> +	if (last_record) {
> +		if (last_record->data < page || last_record->data >= (page + size)) {
> +			warning("%s: bad last record (size=%u)",
> +				__func__, last_record->size);
> +			goto out_free;
> +		}
> +
> +		ptr = kbuffer_read_event(kbuf, &ts);
> +		while (ptr < last_record->data) {

Here we are doing a linear search of last_record.

I could probably add a quicker solution to this by finding the next
record by passing in last_record->data and last_record->ts to a kbuffer
function, at the bottom.


> +			ptr = kbuffer_next_event(kbuf, NULL);
> +			if (!ptr)
> +				break;
> +			if (ptr == last_record->data)
> +				break;
> +		}
> +		if (ptr != last_record->data) {
> +			warning("$s: could not find last_record", __func__);
> +			goto out_free;
> +		}
> +		ptr = kbuffer_next_event(kbuf, &ts);
> +	} else
> +		ptr = kbuffer_read_event(kbuf, &ts);
> +
> +	if (!ptr)
> +		goto out_free;
> +
> +	record = malloc(sizeof(*record));
> +	if (!record)
> +		return NULL;
> +	memset(record, 0, sizeof(*record));
> +
> +	record->ts = ts;
> +	record->size = kbuffer_event_size(kbuf);
> +	record->record_size = kbuffer_curr_size(kbuf);
> +	record->cpu = 0;
> +	record->data = ptr;
> +	record->ref_count = 1;
> +
> + out_free:
> +	kbuffer_free(kbuf);
> +	return record;
> +}
> +
> +static void free_record(struct tep_record *record)
> +{
> +	if (!record)
> +		return;
> +
> +	if (record->ref_count > 0)
> +		record->ref_count--;
> +	if (record->ref_count)
> +		return;
> +
> +	free(record);
> +}
> +
> +static int
> +get_events_in_page(struct tep_handle *tep, void *page,
> +		   int size, int cpu,
> +		   int (*callback)(struct tep_event *,
> +				   struct tep_record *,
> +				   int, void *),
> +		   void *callback_context)
> +{
> +	struct tep_record *last_record = NULL;
> +	struct tep_event *event = NULL;
> +	struct tep_record *record;
> +	int id, cnt = 0;
> +
> +	if (size <= 0)
> +		return 0;
> +
> +	while (true) {
> +		event = NULL;
> +		record = tracefs_read_page_record(tep, page, size, last_record);

This is very slow because the above function does a linear search to
find the next record each time! It would be much better to keep a kbuf
reference and use that.


> +		if (!record)
> +			break;
> +		free_record(last_record);
> +		id = tep_data_type(tep, record);
> +		event = tep_find_event(tep, id);
> +		if (event && callback) {
> +			if (callback(event, record, cpu, callback_context))
> +				break;
> +		}
> +		last_record = record;
> +	}
> +	free_record(last_record);
> +
> +	return cnt;
> +}
> +

Here's a patch (untested ;-) that should help speed up the reading by
using the last record from before. This may even be able to help speed
up other parts of the code.

-- Steve

diff --git a/lib/traceevent/kbuffer-parse.c b/lib/traceevent/kbuffer-parse.c
index b18dedc4..ecbb64e9 100644
--- a/lib/traceevent/kbuffer-parse.c
+++ b/lib/traceevent/kbuffer-parse.c
@@ -649,6 +649,40 @@ void *kbuffer_read_at_offset(struct kbuffer *kbuf, int offset,
 	return data;
 }
 
+/**
+ * kbuffer_set_position - set kbuf to index to a current offset and timestamp
+ * @kbuf:	The kbuffer to read from
+ * @offset:	The offset to set the current postion to
+ * @ts:		The timestamp to set the kbuffer to.
+ *
+ * This routine is used to set the current position saved in @kbuf.
+ * This can be used in conjunction with using kbuffer_curr_offset() to
+ * get the offset of an event retrieved from the @kbuf subbuffer, and
+ * also using the time stamp that was retrived from that same event.
+ * This will set the position of @kbuf back to the state it was when
+ * it returned that event.
+ *
+ * Returns zero on success, -1 otherwise.
+ */
+int kbuffer_set_position(struct kbuffer *kbuf, int offset,
+			   unsigned long long *ts)
+{
+	int ret;
+
+	offset -= kbuf->start;
+
+	if (offset < 0 || offset >= kbuf->size || !ts)
+		return -1;	/* Bad offset */
+
+	kbuf->next = offset;
+	ret = next_event(kbuf);
+	if (ret < 0)
+		return -1;
+
+	kbuf->timestamp = *ts;
+	return 0;
+}
+
 /**
  * kbuffer_subbuffer_size - the size of the loaded subbuffer
  * @kbuf:	The kbuffer to read from

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

* Re: [PATCH v2 4/5] trace-cmd,kernel-shark: New libtracefs APIs for ftrace events and systems
  2020-01-06 15:40 ` [PATCH v2 4/5] trace-cmd,kernel-shark: New libtracefs APIs for ftrace events and systems Tzvetomir Stoyanov (VMware)
  2020-01-08 20:22   ` Steven Rostedt
  2020-01-08 21:09   ` Steven Rostedt
@ 2020-01-09  2:32   ` Steven Rostedt
  2 siblings, 0 replies; 15+ messages in thread
From: Steven Rostedt @ 2020-01-09  2:32 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Mon,  6 Jan 2020 17:40:57 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> +static char **add_list_string(char **list, const char *name, int len)
> +{
> +	if (!list)
> +		list = malloc(sizeof(*list) * 2);
> +	else
> +		list = realloc(list, sizeof(*list) * (len + 2));

I know this was copied from my old code, but this is to document that
we need to add another patch (separate patch, to keep this patch as
more of a move), but the above really should just be:

	list = realloc(list, sizeof(*list) * (len + 2));

as realloc is malloc when the first parameter is NULL.


> +	if (!list)
> +		return NULL;
> +
> +	list[len] = strdup(name);
> +	if (!list[len])
> +		return NULL;
> +
> +	list[len + 1] = NULL;
> +
> +	return list;
> +}
> +
> +static char *append_file(const char *dir, const char *name)
> +{
> +	char *file;
> +	int ret;
> +
> +	ret = asprintf(&file, "%s/%s", dir, name);
> +
> +	return ret < 0 ? NULL : file;
> +}
> +
> +/**
> + * tracefs_event_systems - return list of systems for tracing
> + * @tracing_dir: directory holding the "events" directory
> + *		 if NULL, top tracing directory is used
> + *
> + * Returns an allocated list of system names. Both the names and
> + * the list must be freed with free()
> + * The list returned ends with a "NULL" pointer

We really should have a helper function to free this list. Perhaps we
should call it: tracefs_list_free()?

> + */
> +char **tracefs_event_systems(const char *tracing_dir)
> +{
> +	struct dirent *dent;
> +	char **systems = NULL;
> +	char *events_dir;
> +	struct stat st;
> +	DIR *dir;
> +	int len = 0;
> +	int ret;
> +
> +	if (!tracing_dir)
> +		tracing_dir = tracefs_get_tracing_dir();
> +
> +	if (!tracing_dir)
> +		return NULL;
> +
> +	events_dir = append_file(tracing_dir, "events");
> +	if (!events_dir)
> +		return NULL;
> +
> +	/*
> +	 * Search all the directories in the events directory,
> +	 * and collect the ones that have the "enable" file.
> +	 */
> +	ret = stat(events_dir, &st);
> +	if (ret < 0 || !S_ISDIR(st.st_mode))
> +		goto out_free;
> +
> +	dir = opendir(events_dir);
> +	if (!dir)
> +		goto out_free;
> +
> +	while ((dent = readdir(dir))) {
> +		const char *name = dent->d_name;
> +		char *enable;
> +		char *sys;
> +
> +		if (strcmp(name, ".") == 0 ||
> +		    strcmp(name, "..") == 0)
> +			continue;
> +
> +		sys = append_file(events_dir, name);
> +		ret = stat(sys, &st);
> +		if (ret < 0 || !S_ISDIR(st.st_mode)) {
> +			free(sys);
> +			continue;
> +		}
> +
> +		enable = append_file(sys, "enable");
> +
> +		ret = stat(enable, &st);
> +		if (ret >= 0)
> +			systems = add_list_string(systems, name, len++);
> +
> +		free(enable);
> +		free(sys);
> +	}
> +
> +	closedir(dir);
> +
> + out_free:
> +	free(events_dir);
> +	return systems;
> +}
> +
> +/**
> + * tracefs_system_events - return list of events for system
> + * @tracing_dir: directory holding the "events" directory
> + * @system: the system to return the events for
> + *
> + * Returns an allocated list of event names. Both the names and
> + * the list must be freed with free()
> + * The list returned ends with a "NULL" pointer

And have this free with tracefs_list_free() too.

> + */
> +char **tracefs_system_events(const char *tracing_dir, const char *system)
> +{
> +	struct dirent *dent;
> +	char **events = NULL;
> +	char *system_dir = NULL;
> +	struct stat st;
> +	DIR *dir;
> +	int len = 0;
> +	int ret;
> +
> +	if (!tracing_dir)
> +		tracing_dir = tracefs_get_tracing_dir();
> +
> +	if (!tracing_dir || !system)
> +		return NULL;
> +
> +	asprintf(&system_dir, "%s/events/%s", tracing_dir, system);
> +	if (!system_dir)
> +		return NULL;
> +
> +	ret = stat(system_dir, &st);
> +	if (ret < 0 || !S_ISDIR(st.st_mode))
> +		goto out_free;
> +
> +	dir = opendir(system_dir);
> +	if (!dir)
> +		goto out_free;
> +
> +	while ((dent = readdir(dir))) {
> +		const char *name = dent->d_name;
> +		char *enable;
> +		char *event;
> +
> +		if (strcmp(name, ".") == 0 ||
> +		    strcmp(name, "..") == 0)
> +			continue;
> +
> +		event = append_file(system_dir, name);
> +		ret = stat(event, &st);
> +		if (ret < 0 || !S_ISDIR(st.st_mode)) {
> +			free(event);
> +			continue;
> +		}
> +
> +		enable = append_file(event, "enable");
> +
> +		ret = stat(enable, &st);
> +		if (ret >= 0)
> +			events = add_list_string(events, name, len++);
> +
> +		free(enable);
> +		free(event);
> +	}
> +
> +	closedir(dir);
> +
> + out_free:
> +	free(system_dir);
> +
> +	return events;
> +}
> +
> +/**
> + * tracefs_local_plugins - returns an array of available tracer plugins

Let's not call this "local_plugins" as the name is a misnomer (what
they were called back in 2010, but are no longer called that). Let's
change this to their real name:

	tracefs_tracers()


-- Steve

> + * @tracing_dir: The directory that contains the tracing directory
> + *
> + * Returns an allocate list of plugins. The array ends with NULL
> + * Both the plugin names and array must be freed with free()
> + */
> +char **tracefs_local_plugins(const char *tracing_dir)
> +{
> +	char *available_tracers;
> +	struct stat st;
> +	char **plugins = NULL;
> +	char *buf;
> +	char *str, *saveptr;
> +	char *plugin;
> +	int slen;
> +	int len;
> +	int ret;
> +
> +	if (!tracing_dir)
> +		return NULL;
> +
> +	available_tracers = append_file(tracing_dir, "available_tracers");
> +	if (!available_tracers)
> +		return NULL;
> +
> +	ret = stat(available_tracers, &st);
> +	if (ret < 0)
> +		goto out_free;
> +
> +	len = str_read_file(available_tracers, &buf);
> +	if (len < 0)
> +		goto out_free;
> +
> +	len = 0;
> +	for (str = buf; ; str = NULL) {
> +		plugin = strtok_r(str, " ", &saveptr);
> +		if (!plugin)
> +			break;
> +		slen = strlen(plugin);
> +		if (!slen)
> +			continue;
> +
> +		/* chop off any newlines */
> +		if (plugin[slen - 1] == '\n')
> +			plugin[slen - 1] = '\0';
> +
> +		/* Skip the non tracers */
> +		if (strcmp(plugin, "nop") == 0 ||
> +		    strcmp(plugin, "none") == 0)
> +			continue;
> +
> +		plugins = add_list_string(plugins, plugin, len++);
> +	}
> +	free(buf);
> +
> + out_free:
> +	free(available_tracers);
> +
> +	return plugins;
> +}
> diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
> index 62f67d5..1d91e87 100644
> --- a/tracecmd/trace-record.c
> +++ b/tracecmd/trace-record.c
> @@ -4178,7 +4178,7 @@ find_ts_in_page(struct tep_handle *pevent, void *page, int size)
>  		return 0;
>  
>  	while (!ts) {
> -		record = tracecmd_read_page_record(pevent, page, size,
> +		record = tracefs_read_page_record(pevent, page, size,
>  						   last_record);
>  		if (!record)
>  			break;


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

* Re: [PATCH v2 3/5] trace-cmd: New libtracefs APIs for ftrace instances
  2020-01-08 19:37   ` Steven Rostedt
@ 2020-01-09 13:29     ` Tzvetomir Stoyanov
  0 siblings, 0 replies; 15+ messages in thread
From: Tzvetomir Stoyanov @ 2020-01-09 13:29 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-trace-devel

On Wed, Jan 8, 2020 at 9:37 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Mon,  6 Jan 2020 17:40:56 +0200
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > The functionality related to ftrace instances is moved from
> > trace-cmd application to libtracefs. The following new
> > library APIs are introduced:
> >     struct tracefs_instance;
> >     struct tracefs_instance *tracefs_alloc_instance(const char *name);
> >     int tracefs_free_instance(struct tracefs_instance *instance);
> >     int tracefs_make_instance(struct tracefs_instance *instance);
> >     int tracefs_remove_instance(struct tracefs_instance *instance);
>
> I'm thinking that "make" should be "create" and "remove" should be
> "destroy". The "make" and "remove" does match "mkdir" and "rmdir" but
> that's just the behind-the-scenes of what the OS does. But from an
> application point of view, "create" and "destroy" seem more
> appropriate.
>
> What do you think?
>
>
> >     char *tracefs_get_instance_name(struct tracefs_instance *instance);
> >     char *tracefs_get_instance_file(struct tracefs_instance *instance, const char *file);
> >     char *tracefs_get_instance_dir(struct tracefs_instance *instance);
> >     int tracefs_write_instance_file(struct tracefs_instance *instance,
> >                                const char *file, const char *str, const char *type);
> >     char *tracefs_read_instance_file(struct tracefs_instance *instance, char *file, int *psize);
>
> Also, another naming convention used commonly, is when you have a set
> of functions working on the same thing, is to use the name of what it
> is working on first. That is, instead of the above, we should have:
>
>  tracefs_instance_alloc()
>  tracefs_instance_free()
>  tracefs_instance_create() (instead of "make")
>  tracefs_instance_destroy() (instead of "remove")
>  tracefs_instance_get_name()
>  tracefs_instance_get_file()
>  tracefs_instance_file_read()
>  tracefs_instance_file_write()
>
> That way it is easier to find these functions with just searching for
> "tracefs_instance"
>
> The "get_*" is ok to keep, but as I showed above, I think "_file_read"
> and "_file_write" is a better name.
>
> [ more below ]
>
> >
> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> > ---
> >  include/tracefs/tracefs.h           |  17 ++
> >  lib/tracefs/Makefile                |   1 +
> >  lib/tracefs/include/tracefs-local.h |   1 +
> >  lib/tracefs/tracefs-instance.c      | 267 ++++++++++++++++++++++++++++
> >  lib/tracefs/tracefs-utils.c         |  43 +++++
> >  tracecmd/include/trace-local.h      |   5 +-
> >  tracecmd/trace-list.c               |   2 +-
> >  tracecmd/trace-record.c             | 264 +++++++++------------------
> >  tracecmd/trace-show.c               |   2 +
> >  tracecmd/trace-stat.c               |  21 ++-
> >  10 files changed, 431 insertions(+), 192 deletions(-)
> >  create mode 100644 lib/tracefs/tracefs-instance.c
> >
> > diff --git a/include/tracefs/tracefs.h b/include/tracefs/tracefs.h
> > index e844c75..6b27ff7 100644
> > --- a/include/tracefs/tracefs.h
> > +++ b/include/tracefs/tracefs.h
> > @@ -17,4 +17,21 @@ const char *tracefs_get_tracing_dir(void);
> >  /* tracefs_find_tracing_dir must be freed */
> >  char *tracefs_find_tracing_dir(void);
> >
> > +/* ftarce instances */
> > +struct tracefs_instance;
> > +
> > +struct tracefs_instance *tracefs_alloc_instance(const char *name);
> > +int tracefs_free_instance(struct tracefs_instance *instance);
> > +int tracefs_make_instance(struct tracefs_instance *instance);
> > +int tracefs_remove_instance(struct tracefs_instance *instance);
> > +char *tracefs_get_instance_name(struct tracefs_instance *instance);
> > +char *
> > +tracefs_get_instance_file(struct tracefs_instance *instance, const char *file);
> > +char *tracefs_get_instance_dir(struct tracefs_instance *instance);
> > +int tracefs_write_instance_file(struct tracefs_instance *instance,
> > +                             const char *file, const char *str,
> > +                             const char *type);
> > +char *tracefs_read_instance_file(struct tracefs_instance *instance,
> > +                              char *file, int *psize);
> > +
> >  #endif /* _TRACE_FS_H */
> > diff --git a/lib/tracefs/Makefile b/lib/tracefs/Makefile
> > index 86d7845..4030272 100644
> > --- a/lib/tracefs/Makefile
> > +++ b/lib/tracefs/Makefile
> > @@ -8,6 +8,7 @@ DEFAULT_TARGET = $(bdir)/libtracefs.a
> >
> >  OBJS =
> >  OBJS += tracefs-utils.o
> > +OBJS += tracefs-instance.o
> >
> >  OBJS := $(OBJS:%.o=$(bdir)/%.o)
> >  DEPS := $(OBJS:$(bdir)/%.o=$(bdir)/.%.d)
> > diff --git a/lib/tracefs/include/tracefs-local.h b/lib/tracefs/include/tracefs-local.h
> > index 231edd1..fe327a0 100644
> > --- a/lib/tracefs/include/tracefs-local.h
> > +++ b/lib/tracefs/include/tracefs-local.h
> > @@ -8,5 +8,6 @@
> >
> >  /* Can be overridden */
> >  void warning(const char *fmt, ...);
> > +int str_read_file(const char *file, char **buffer);
> >
> >  #endif /* _TRACE_FS_LOCAL_H */
> > diff --git a/lib/tracefs/tracefs-instance.c b/lib/tracefs/tracefs-instance.c
> > new file mode 100644
> > index 0000000..14e8eed
> > --- /dev/null
> > +++ b/lib/tracefs/tracefs-instance.c
> > @@ -0,0 +1,267 @@
> > +// SPDX-License-Identifier: LGPL-2.1
> > +/*
> > + * Copyright (C) 2008, 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
> > + *
> > + * Updates:
> > + * Copyright (C) 2019, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
> > + *
> > + */
> > +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <errno.h>
> > +#include <sys/stat.h>
> > +#include <fcntl.h>
> > +
> > +#include "tracefs.h"
> > +#include "tracefs-local.h"
> > +
> > +struct tracefs_instance {
> > +     char *name;
> > +};
> > +
> > +/**
> > + * tracefs_alloc_instance - allocate a new ftrace instance
> > + * @name: The name of the instance (instance will point to this)
> > + *
> > + * Returns a newly allocated instance, or NULL in case of an error.
> > + */
> > +struct tracefs_instance *tracefs_alloc_instance(const char *name)
> > +{
> > +     struct tracefs_instance *instance;
> > +
> > +     instance = calloc(1, sizeof(*instance));
> > +     if (!instance)
> > +             return NULL;
> > +     if (name)
> > +             instance->name = strdup(name);
>
> We can remove make this a little simpler (and with the name check):
>
>         instance = calloc(1, sizeof(*instance));
>         if (instance && name) {
>                 instance->name = strdup(name);
>                 if (!instance->name) {
>                         free(instance);
>                         instance = NULL;
>                 }
>         }
>
>         return instance;
>
> > +
> > +     return instance;
> > +}
> > +
> > +/**
> > + * tracefs_free_instance - Free an instance, previously allocated by
> > +                        tracefs_alloc_instance()
> > + * @instance: Pointer to the instance to be freed
> > + *
> > + * Returns -1 in case of an error, or 0 otherwise.
>
> I don't think the free should return a value. Make it void.
>
> > + */
> > +int tracefs_free_instance(struct tracefs_instance *instance)
> > +{
> > +     if (!instance)
> > +             return -1;
> > +
> > +     free(instance->name);
> > +     free(instance);
> > +     return 0;
> > +}
> > +
> > +/**
> > + * tracefs_make_instance - Create a new ftrace instance
> > + * @instance: Pointer to the instance to be created
> > + *
> > + * Returns 1 if the instance already exist, 0 if the instance
> > + * is created successful or -1 in case of an error
> > + */
> > +int tracefs_make_instance(struct tracefs_instance *instance)
> > +{
> > +     struct stat st;
> > +     char *path;
> > +     int ret;
> > +
> > +     path = tracefs_get_instance_dir(instance);
> > +     ret = stat(path, &st);
> > +     if (ret < 0) {
> > +             ret = mkdir(path, 0777);
> > +             if (ret < 0)
> > +                     return ret;
>
> Remove the check of ret here, as the return skips the freeing of path.
>
> Should be just:
>
>         if (ret < 0)
>                 ret = mkdir(path, 0777);
>         else
>                 ret = 1;
>
> > +
> > +     } else
> > +             ret = 1;
> > +     tracefs_put_tracing_file(path);
> > +     return ret;
> > +}
> > +
> > +/**
> > + * tracefs_remove_instance - Remove a ftrace instance
> > + * @instance: Pointer to the instance to be removed
> > + *
> > + * Returns -1 in case of an error, or 0 otherwise.
> > + */
> > +int tracefs_remove_instance(struct tracefs_instance *instance)
> > +{
> > +     char *path;
> > +     int ret;
> > +
> > +     if (!instance || !instance->name) {
> > +             warning("Cannot remove top instance");
> > +             return -1;
> > +     }
> > +
> > +     path = tracefs_get_instance_dir(instance);
>
> path could be NULL here due to failed allocation. Need to check that
> and return an error.
>
> > +     ret = rmdir(path);
> > +     tracefs_put_tracing_file(path);
> > +
> > +     return ret;
> > +}
> > +
> > +/**
> > + * tracefs_get_instance_file - return the path to an instance file.
> > + * @instance: ftrace instance, can be NULL for the top instance
> > + * @file: name of file to return
> > + *
> > + * Returns the path of the @file for the given @instance, or NULL in
> > + * case of an error.
> > + *
> > + * Must use tracefs_put_tracing_file() to free the returned string.
> > + */
> > +char *
> > +tracefs_get_instance_file(struct tracefs_instance *instance, const char *file)
> > +{
> > +     char *path;
> > +     char *buf;
> > +     int ret;
> > +
> > +     if (instance && instance->name) {
> > +             ret = asprintf(&buf, "instances/%s/%s", instance->name, file);
> > +             if (ret < 0) {
> > +                     warning("Failed to allocate name for %s/%s",
> > +                              instance->name, file);
>
> For this being in a library, we should probably remove warnings.
>
> > +                     return NULL;
> > +             }
> > +             path = tracefs_get_tracing_file(buf);
> > +             free(buf);
> > +     } else
> > +             path = tracefs_get_tracing_file(file);
> > +
> > +     return path;
> > +}
> > +
> > +/**
> > + * tracefs_get_instance_dir - return the path to the instance directory.
> > + * @instance: ftrace instance, can be NULL for the top instance
> > + *
> > + * Returns the full path to the instance directory
> > + *
> > + * Must use tracefs_put_tracing_file() to free the returned string.
> > + */
> > +char *tracefs_get_instance_dir(struct tracefs_instance *instance)
> > +{
> > +     char *buf;
> > +     char *path;
> > +     int ret;
> > +
> > +     if (instance && instance->name) {
> > +             ret = asprintf(&buf, "instances/%s", instance->name);
> > +             if (ret < 0) {
> > +                     warning("Failed to allocate path for instance %s",
> > +                              instance->name);
> > +                     return NULL;
> > +             }
> > +             path = tracefs_get_tracing_file(buf);
> > +             free(buf);
> > +     } else
> > +             path = tracefs_find_tracing_dir();
> > +
> > +     return path;
> > +}
> > +
> > +/**
> > + * tracefs_get_instance_name - return the name of an instance
> > + * @instance: ftrace instance
> > + *
> > + * Returns the name of the given @instance.
> > + * The returned string must *not* be freed.
> > + */
> > +char *tracefs_get_instance_name(struct tracefs_instance *instance)
> > +{
> > +     if (instance)
> > +             return instance->name;
> > +     return NULL;
> > +}
> > +
> > +static int write_file(const char *file, const char *str, const char *type)
> > +{
> > +     char buf[BUFSIZ];
> > +     int ret;
> > +     int fd;
> > +
> > +     fd = open(file, O_WRONLY | O_TRUNC);
> > +     if (fd < 0) {
> > +             warning("Failed to open '%s'", file);
> > +             return -1;
> > +     }
> > +     ret = write(fd, str, strlen(str));
> > +     close(fd);
> > +     if (ret < 0 && type) {
> > +             /* write failed */
> > +             fd = open(file, O_RDONLY);
> > +             if (fd < 0) {
> > +                     warning("Failed to write in '%s'", file);
> > +                     return -1;
> > +             }
> > +
> > +             while ((ret = read(fd, buf, BUFSIZ)) > 0)
> > +                     fprintf(stderr, "%.*s", ret, buf);
> > +             warning("Failed %s of %s\n", type, file);
>
> The library function should not bother reading the file on error. This
> is only applicable for some (one or two) files in the instance
> directory. Thus remove this code.
>
> > +             close(fd);
> > +     }
> > +     return ret;
> > +}
> > +
> > +
> > +/**
> > + * tracefs_write_instance_file - Write in trace file of specific instance.
> > + * @instance: ftrace instance, can be NULL for the top instance
> > + * @file: name of the file
> > + * @str: nul terminated string, that will be written in the file.
> > + * @type: nul terminated string, describing the current write operation.
> > + *     Used for logging purposes.
> > + *
> > + * Returns the number of written bytes, or -1 in case of an error
> > + */
> > +int tracefs_write_instance_file(struct tracefs_instance *instance,
> > +                              const char *file, const char *str,
> > +                              const char *type)
>
> Let's get rid of the "type", it's confusing for a library function. If
> the output should be read, that should be for a different operation.
>
>
> > +{
> > +     struct stat st;
> > +     char *path;
> > +     int ret;
> > +
> > +     path = tracefs_get_instance_file(instance, file);
>
> Need to check for path == NULL.
>
> > +     ret = stat(path, &st);
> > +     if (ret == 0)
> > +             ret = write_file(path, str, type);
> > +     tracefs_put_tracing_file(path);
> > +
> > +     return ret;
> > +}
> > +
> > +/**
> > + * tracefs_read_instance_file - Read from a trace file of specific instance.
> > + * @instance: ftrace instance, can be NULL for the top instance
> > + * @file: name of the file
> > + * @psize: returns the number of bytes read
> > + *
> > + * Returns a pointer to a nul terminated string, read from the file, or NULL in
> > + * case of an error.
> > + * The return string must be freed by free()
> > + */
> > +char *tracefs_read_instance_file(struct tracefs_instance *instance,
> > +                               char *file, int *psize)
> > +{
> > +     char *buf = NULL;
> > +     int size = 0;
> > +     char *path;
> > +
> > +     path = tracefs_get_instance_file(instance, file);
>
> Need to check for path == NULL.
>
> > +
> > +     size = str_read_file(path, &buf);
> > +
> > +     tracefs_put_tracing_file(path);
> > +     if (buf && psize)
> > +             *psize = size;
> > +
> > +     return buf;
> > +}
> > diff --git a/lib/tracefs/tracefs-utils.c b/lib/tracefs/tracefs-utils.c
> > index 0ef16fc..bdab130 100644
> > --- a/lib/tracefs/tracefs-utils.c
> > +++ b/lib/tracefs/tracefs-utils.c
> > @@ -10,6 +10,9 @@
> >  #include <sys/mount.h>
> >  #include <sys/stat.h>
> >  #include <linux/limits.h>
> > +#include <sys/stat.h>
> > +#include <fcntl.h>
> > +#include <unistd.h>
> >
> >  #include "tracefs.h"
> >
> > @@ -181,3 +184,43 @@ void tracefs_put_tracing_file(char *name)
> >  {
> >       free(name);
> >  }
> > +
> > +int str_read_file(const char *file, char **buffer)
> > +{
> > +     char stbuf[BUFSIZ];
> > +     char *buf = NULL;
> > +     int size = 0;
> > +     char *nbuf;
> > +     int fd;
> > +     int r;
> > +
> > +     fd = open(file, O_RDONLY);
> > +     if (fd < 0) {
> > +             warning("File %s not found", file);
> > +             return -1;
> > +     }
> > +
> > +     do {
> > +             r = read(fd, stbuf, BUFSIZ);
> > +             if (r <= 0)
> > +                     continue;
> > +             nbuf = realloc(buf, size+r+1);
> > +             if (!nbuf) {
> > +                     warning("Failed to allocate file buffer");
>
> Again, we should remove warnings from the library function. But this
> can be done later. I just wanted to document that we need to do that.
>
> > +                     size = -1;
> > +                     break;
> > +             }
> > +             buf = nbuf;
> > +             memcpy(buf+size, stbuf, r);
> > +             size += r;
> > +     } while (r);
>
> Hmm, this probably should be:
>
>         } while (r > 0);
>
> (I know this was broken before this patch)
>
> > +
> > +     close(fd);
> > +     if (size > 0) {
>
> Hmm, if r is less than zero here, we have a bug. Perhaps this needs to
> be:
>
>         if (r == 0 && size > 0) {
>
> > +             buf[size] = '\0';
> > +             *buffer = buf;
> > +     } else
> > +             free(buf);
> > +
> > +     return size;
> > +}
>
>
Thanks Steve! It makes sense, will include these changes in the next version.


> -- Steve



-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

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

* Re: [PATCH v2 4/5] trace-cmd,kernel-shark: New libtracefs APIs for ftrace events and systems
  2020-01-08 20:22   ` Steven Rostedt
@ 2020-01-09 13:37     ` Tzvetomir Stoyanov
  0 siblings, 0 replies; 15+ messages in thread
From: Tzvetomir Stoyanov @ 2020-01-09 13:37 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-trace-devel

On Wed, Jan 8, 2020 at 10:22 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Mon,  6 Jan 2020 17:40:57 +0200
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > The functionality related to ftrace events and systems
> > is moved from trace-cmd application and libtracecmd to libtracefs.
> >
> > The following libtracecmd APIs are removed:
> >   tracecmd_read_page_record();
> >   tracecmd_event_systems();
> >   tracecmd_system_events();
> >   tracecmd_local_plugins();
> >   tracecmd_add_list();
> >   tracecmd_free_list();
> >
> > The following new library APIs are introduced:
> >   tracefs_read_page_record();
> >   tracefs_event_systems();
> >   tracefs_system_events();
> >   tracefs_local_plugins();
>
> Hmm, we need to discuss plugins a bit more. I'm not sure they are
> needed for either libtracecmd nor libtracefs. We have plugins for
> libtraceevent which is just a better way to present the event, but I'm
> imagining that plugins are going to be more specific to trace-cmd
> directly. Maybe they can be packaged with libtracecmd, but I'm not
> seeing a need for libtracefs.
>
> What do you have in mind?
>
May be the API name is confusing, it is not about the library plugins.
It returns the list of
plugins from "available_tracers" file. We can rename the API to
tracefs_local_tracers() ?
Tracers are referred as "plugins" in ftrace docs, that's why the API
is named this way.

> -- Steve
>
>
> >   tracefs_iterate_raw_events();
> >
> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> > ---
> >



-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

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

* Re: [PATCH v2 4/5] trace-cmd,kernel-shark: New libtracefs APIs for ftrace events and systems
  2020-01-08 21:09   ` Steven Rostedt
@ 2020-01-09 14:14     ` Tzvetomir Stoyanov
  0 siblings, 0 replies; 15+ messages in thread
From: Tzvetomir Stoyanov @ 2020-01-09 14:14 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-trace-devel

On Wed, Jan 8, 2020 at 11:09 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Mon,  6 Jan 2020 17:40:57 +0200
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
>
> > --- /dev/null
> > +++ b/lib/tracefs/tracefs-events.c
> > @@ -0,0 +1,476 @@
> > +// SPDX-License-Identifier: LGPL-2.1
> > +/*
> > + * Copyright (C) 2008, 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
> > + *
> > + * Updates:
> > + * Copyright (C) 2019, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
> > + *
> > + */
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <dirent.h>
> > +#include <unistd.h>
> > +#include <errno.h>
> > +#include <sys/stat.h>
> > +#include <fcntl.h>
> > +
> > +#include "kbuffer.h"
> > +#include "tracefs.h"
> > +#include "tracefs-local.h"
> > +
> > +/**
> > + * tracefs_read_page_record - read a record off of a page
> > + * @tep: tep used to parse the page
> > + * @page: the page to read
> > + * @size: the size of the page
> > + * @last_record: last record read from this page
> > + *
> > + * If a ring buffer page is available, and the need to parse it
> > + * without having a handle, then this function can be used
>
> Not having a handle to what? Ah, I wrote this back in 2011. And that
> "handle" meant a tracecmd_input_handle. Hmm, I think we need a
> tracefs_handle of some sort for this, because it will slow down
> tracefs_iterate_raw_events() very much so).
>
I'm not sure how tracefs_handle could speed up the events iteration,
but I can think about it in a context of a different patch. The
current patch set
is just a skeleton of the new library, most of the code is moved from
the application
or libtracecmd with almost no modifications.

>
> > + *
> > + * The @tep needs to be initialized to have the page header information
> > + * already available.
> > + *
> > + * The @last_record is used to know where to read the next record from
> > + * If @last_record is NULL, the first record on the page will be read
> > + *
> > + * Returns:
> > + *  A newly allocated record that must be freed with free_record() if
>
> Hmm, free_record() needs a prefix ("tracefs_") ?
Actually free_record() is not exported as libtracefs API, which is
confusing. There is
free_record() libtracecmd API, which is used in the trace-cmd context. May be it
is better to have tracefs_free_record() , or to use the one from libtracecmd ?

>
> > + *  a record is found. Otherwise NULL is returned if the record is bad
> > + *  or no more records exist
> > + */
> > +struct tep_record *
> > +tracefs_read_page_record(struct tep_handle *tep, void *page, int size,
> > +                       struct tep_record *last_record)
> > +{
> > +     unsigned long long ts;
> > +     struct kbuffer *kbuf;
> > +     struct tep_record *record = NULL;
> > +     enum kbuffer_long_size long_size;
> > +     enum kbuffer_endian endian;
> > +     void *ptr;
> > +
> > +     if (tep_is_file_bigendian(tep))
> > +             endian = KBUFFER_ENDIAN_BIG;
> > +     else
> > +             endian = KBUFFER_ENDIAN_LITTLE;
> > +
> > +     if (tep_get_header_page_size(tep) == 8)
> > +             long_size = KBUFFER_LSIZE_8;
> > +     else
> > +             long_size = KBUFFER_LSIZE_4;
> > +
> > +     kbuf = kbuffer_alloc(long_size, endian);
> > +     if (!kbuf)
> > +             return NULL;
> > +
> > +     kbuffer_load_subbuffer(kbuf, page);
> > +     if (kbuffer_subbuffer_size(kbuf) > size) {
> > +             warning("%s: page_size > size", __func__);
> > +             goto out_free;
> > +     }
> > +
> > +     if (last_record) {
> > +             if (last_record->data < page || last_record->data >= (page + size)) {
> > +                     warning("%s: bad last record (size=%u)",
> > +                             __func__, last_record->size);
> > +                     goto out_free;
> > +             }
> > +
> > +             ptr = kbuffer_read_event(kbuf, &ts);
> > +             while (ptr < last_record->data) {
>
> Here we are doing a linear search of last_record.
>
> I could probably add a quicker solution to this by finding the next
> record by passing in last_record->data and last_record->ts to a kbuffer
> function, at the bottom.
>
>
> > +                     ptr = kbuffer_next_event(kbuf, NULL);
> > +                     if (!ptr)
> > +                             break;
> > +                     if (ptr == last_record->data)
> > +                             break;
> > +             }
> > +             if (ptr != last_record->data) {
> > +                     warning("$s: could not find last_record", __func__);
> > +                     goto out_free;
> > +             }
> > +             ptr = kbuffer_next_event(kbuf, &ts);
> > +     } else
> > +             ptr = kbuffer_read_event(kbuf, &ts);
> > +
> > +     if (!ptr)
> > +             goto out_free;
> > +
> > +     record = malloc(sizeof(*record));
> > +     if (!record)
> > +             return NULL;
> > +     memset(record, 0, sizeof(*record));
> > +
> > +     record->ts = ts;
> > +     record->size = kbuffer_event_size(kbuf);
> > +     record->record_size = kbuffer_curr_size(kbuf);
> > +     record->cpu = 0;
> > +     record->data = ptr;
> > +     record->ref_count = 1;
> > +
> > + out_free:
> > +     kbuffer_free(kbuf);
> > +     return record;
> > +}
> > +
> > +static void free_record(struct tep_record *record)
> > +{
> > +     if (!record)
> > +             return;
> > +
> > +     if (record->ref_count > 0)
> > +             record->ref_count--;
> > +     if (record->ref_count)
> > +             return;
> > +
> > +     free(record);
> > +}
> > +
> > +static int
> > +get_events_in_page(struct tep_handle *tep, void *page,
> > +                int size, int cpu,
> > +                int (*callback)(struct tep_event *,
> > +                                struct tep_record *,
> > +                                int, void *),
> > +                void *callback_context)
> > +{
> > +     struct tep_record *last_record = NULL;
> > +     struct tep_event *event = NULL;
> > +     struct tep_record *record;
> > +     int id, cnt = 0;
> > +
> > +     if (size <= 0)
> > +             return 0;
> > +
> > +     while (true) {
> > +             event = NULL;
> > +             record = tracefs_read_page_record(tep, page, size, last_record);
>
> This is very slow because the above function does a linear search to
> find the next record each time! It would be much better to keep a kbuf
> reference and use that.
>
>
> > +             if (!record)
> > +                     break;
> > +             free_record(last_record);
> > +             id = tep_data_type(tep, record);
> > +             event = tep_find_event(tep, id);
> > +             if (event && callback) {
> > +                     if (callback(event, record, cpu, callback_context))
> > +                             break;
> > +             }
> > +             last_record = record;
> > +     }
> > +     free_record(last_record);
> > +
> > +     return cnt;
> > +}
> > +
>
> Here's a patch (untested ;-) that should help speed up the reading by
> using the last record from before. This may even be able to help speed
> up other parts of the code.
>
> -- Steve
>
> diff --git a/lib/traceevent/kbuffer-parse.c b/lib/traceevent/kbuffer-parse.c
> index b18dedc4..ecbb64e9 100644
> --- a/lib/traceevent/kbuffer-parse.c
> +++ b/lib/traceevent/kbuffer-parse.c
> @@ -649,6 +649,40 @@ void *kbuffer_read_at_offset(struct kbuffer *kbuf, int offset,
>         return data;
>  }
>
> +/**
> + * kbuffer_set_position - set kbuf to index to a current offset and timestamp
> + * @kbuf:      The kbuffer to read from
> + * @offset:    The offset to set the current postion to
> + * @ts:                The timestamp to set the kbuffer to.
> + *
> + * This routine is used to set the current position saved in @kbuf.
> + * This can be used in conjunction with using kbuffer_curr_offset() to
> + * get the offset of an event retrieved from the @kbuf subbuffer, and
> + * also using the time stamp that was retrived from that same event.
> + * This will set the position of @kbuf back to the state it was when
> + * it returned that event.
> + *
> + * Returns zero on success, -1 otherwise.
> + */
> +int kbuffer_set_position(struct kbuffer *kbuf, int offset,
> +                          unsigned long long *ts)
> +{
> +       int ret;
> +
> +       offset -= kbuf->start;
> +
> +       if (offset < 0 || offset >= kbuf->size || !ts)
> +               return -1;      /* Bad offset */
> +
> +       kbuf->next = offset;
> +       ret = next_event(kbuf);
> +       if (ret < 0)
> +               return -1;
> +
> +       kbuf->timestamp = *ts;
> +       return 0;
> +}
> +
>  /**
>   * kbuffer_subbuffer_size - the size of the loaded subbuffer
>   * @kbuf:      The kbuffer to read from

Thanks Steven! I'll test it and add it in the next version of the patch set.


-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

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

end of thread, other threads:[~2020-01-09 14:14 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-06 15:40 [PATCH v2 0/5] tracefs library Tzvetomir Stoyanov (VMware)
2020-01-06 15:40 ` [PATCH v2 1/5] trace-cmd: Introduce libtracefs library Tzvetomir Stoyanov (VMware)
2020-01-06 15:40 ` [PATCH v2 2/5] kernel-shark: Use new tracefs library Tzvetomir Stoyanov (VMware)
2020-01-08 19:05   ` Steven Rostedt
2020-01-08 20:09   ` Steven Rostedt
2020-01-06 15:40 ` [PATCH v2 3/5] trace-cmd: New libtracefs APIs for ftrace instances Tzvetomir Stoyanov (VMware)
2020-01-08 19:37   ` Steven Rostedt
2020-01-09 13:29     ` Tzvetomir Stoyanov
2020-01-06 15:40 ` [PATCH v2 4/5] trace-cmd,kernel-shark: New libtracefs APIs for ftrace events and systems Tzvetomir Stoyanov (VMware)
2020-01-08 20:22   ` Steven Rostedt
2020-01-09 13:37     ` Tzvetomir Stoyanov
2020-01-08 21:09   ` Steven Rostedt
2020-01-09 14:14     ` Tzvetomir Stoyanov
2020-01-09  2:32   ` Steven Rostedt
2020-01-06 15:40 ` [PATCH v2 5/5] trace-cmd,kernel-shark: New libtracefs APIs for loading ftrace events Tzvetomir Stoyanov (VMware)

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.