linux-trace-devel.vger.kernel.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).