[PATCH 0/2] perf_counter: add tracepoint support

[PATCH 0/2] perf_counter: add tracepoint support

[Jason Baron]

hi,

The following two patches build on Peter's initial tracepoint perf counter work.
The first patches enables the kernel side, and the second patch adds support to
the 'perf' tool itself. The debugfs filesystem needs to be mounted to make this
work and it is searched in the default location '/sys/kernel/debug'. We can
add an optional parameter to search for it elsewhere if need be.

Below, I show the output of 'perf list', and 'perf stat' for a tbench run.

thanks,

-Jason


> perf list

List of pre-defined events (to be used in -e):

  cpu-cycles OR cycles                     [Hardware event]
  instructions                             [Hardware event]
  cache-references                         [Hardware event]
  cache-misses                             [Hardware event]
  branch-instructions OR branches          [Hardware event]
  branch-misses                            [Hardware event]
  bus-cycles                               [Hardware event]

  cpu-clock                                [Software event]
  task-clock                               [Software event]
  page-faults OR faults                    [Software event]
  minor-faults                             [Software event]
  major-faults                             [Software event]
  context-switches OR cs                   [Software event]
  cpu-migrations OR migrations             [Software event]

  L1-d$-loads                              [Hardware cache event]
  L1-d$-load-misses                        [Hardware cache event]
  L1-d$-stores                             [Hardware cache event]
  L1-d$-store-misses                       [Hardware cache event]
  L1-d$-prefetches                         [Hardware cache event]
  L1-d$-prefetch-misses                    [Hardware cache event]
  L1-i$-loads                              [Hardware cache event]
  L1-i$-load-misses                        [Hardware cache event]
  L1-i$-prefetches                         [Hardware cache event]
  L1-i$-prefetch-misses                    [Hardware cache event]
  LLC-loads                                [Hardware cache event]
  LLC-load-misses                          [Hardware cache event]
  LLC-stores                               [Hardware cache event]
  LLC-store-misses                         [Hardware cache event]
  LLC-prefetches                           [Hardware cache event]
  LLC-prefetch-misses                      [Hardware cache event]
  dTLB-loads                               [Hardware cache event]
  dTLB-load-misses                         [Hardware cache event]
  dTLB-stores                              [Hardware cache event]
  dTLB-store-misses                        [Hardware cache event]
  dTLB-prefetches                          [Hardware cache event]
  dTLB-prefetch-misses                     [Hardware cache event]
  iTLB-loads                               [Hardware cache event]
  iTLB-load-misses                         [Hardware cache event]
  branch-loads                             [Hardware cache event]
  branch-load-misses                       [Hardware cache event]

  rNNN                                     [raw hardware event descriptor]

  skb:kfree_skb                                [Tracepoint event]
  block:block_rq_abort                           [Tracepoint event]
  block:block_rq_insert                          [Tracepoint event]
  block:block_rq_issue                           [Tracepoint event]
  block:block_rq_requeue                         [Tracepoint event]
  block:block_rq_complete                        [Tracepoint event]
  block:block_bio_bounce                         [Tracepoint event]
  block:block_bio_complete                       [Tracepoint event]
  block:block_bio_backmerge                      [Tracepoint event]
  block:block_bio_frontmerge                     [Tracepoint event]
  block:block_bio_queue                          [Tracepoint event]
  block:block_getrq                              [Tracepoint event]
  block:block_sleeprq                            [Tracepoint event]
  block:block_plug                               [Tracepoint event]
  block:block_unplug_timer                       [Tracepoint event]
  block:block_unplug_io                          [Tracepoint event]
  block:block_split                              [Tracepoint event]
  block:block_remap                              [Tracepoint event]
  kmem:kmalloc                                  [Tracepoint event]
  kmem:kmem_cache_alloc                         [Tracepoint event]
  kmem:kmalloc_node                             [Tracepoint event]
  kmem:kmem_cache_alloc_node                    [Tracepoint event]
  kmem:kfree                                    [Tracepoint event]
  kmem:kmem_cache_free                          [Tracepoint event]
  ftrace:kmem_free                                [Tracepoint event]
  ftrace:kmem_alloc                               [Tracepoint event]
  ftrace:power                                    [Tracepoint event]
  ftrace:hw_branch                                [Tracepoint event]
  ftrace:branch                                   [Tracepoint event]
  ftrace:print                                    [Tracepoint event]
  ftrace:bprint                                   [Tracepoint event]
  ftrace:user_stack                               [Tracepoint event]
  ftrace:kernel_stack                             [Tracepoint event]
  ftrace:special                                  [Tracepoint event]
  ftrace:context_switch                           [Tracepoint event]
  ftrace:wakeup                                   [Tracepoint event]
  ftrace:funcgraph_exit                           [Tracepoint event]
  ftrace:funcgraph_entry                          [Tracepoint event]
  ftrace:function                                 [Tracepoint event]
  workqueue:workqueue_insertion                      [Tracepoint event]
  workqueue:workqueue_execution                      [Tracepoint event]
  workqueue:workqueue_creation                       [Tracepoint event]
  workqueue:workqueue_destruction                    [Tracepoint event]
  irq:irq_handler_entry                        [Tracepoint event]
  irq:irq_handler_exit                         [Tracepoint event]
  irq:softirq_entry                            [Tracepoint event]
  irq:softirq_exit                             [Tracepoint event]
  sched:sched_kthread_stop                       [Tracepoint event]
  sched:sched_kthread_stop_ret                   [Tracepoint event]
  sched:sched_wait_task                          [Tracepoint event]
  sched:sched_wakeup                             [Tracepoint event]
  sched:sched_wakeup_new                         [Tracepoint event]
  sched:sched_switch                             [Tracepoint event]
  sched:sched_migrate_task                       [Tracepoint event]
  sched:sched_process_free                       [Tracepoint event]
  sched:sched_process_exit                       [Tracepoint event]
  sched:sched_process_wait                       [Tracepoint event]
  sched:sched_process_fork                       [Tracepoint event]
  sched:sched_signal_send                        [Tracepoint event]


>  perf stat  -a -e sched:sched_switch -e sched:sched_wakeup -e irq:irq_handler_entry tbench 4

 Performance counter stats for 'tbench 4':

      194658693  sched:sched_switch      
       97479295  sched:sched_wakeup      
           9122  irq:irq_handler_entry   

  720.171439729  seconds time elapsed

  

  -add kernel tracepoint support
  -add support for tracepoints to the perf tool

 init/Kconfig                   |    2 +-
 kernel/perf_counter.c          |    6 +-
 tools/perf/util/parse-events.c |  195 +++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/util.h         |    2 +
 4 files changed, 200 insertions(+), 5 deletions(-)

[PATCH 1/2] perf_counter: cleanup kernel perf counter support for tracepoints

[Jason Baron]

cleanup EVENT_PROFILE support to that it compiles and support tracepoint tracing
from the 'perf' tool. 


Signed-off-by: Jason Baron <jbaron@redhat.com>


---
 init/Kconfig          |    2 +-
 kernel/perf_counter.c |    6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 0e8784e..13473ac 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -955,7 +955,7 @@ config PERF_COUNTERS
 
 config EVENT_PROFILE
 	bool "Tracepoint profile sources"
-	depends on PERF_COUNTERS && EVENT_TRACER
+	depends on PERF_COUNTERS && EVENT_TRACING
 	default y
 
 endmenu
diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
index d55a50d..a6d9759 100644
--- a/kernel/perf_counter.c
+++ b/kernel/perf_counter.c
@@ -3671,7 +3671,7 @@ static const struct pmu perf_ops_task_clock = {
 void perf_tpcounter_event(int event_id)
 {
 	struct perf_sample_data data = {
-		.regs = get_irq_regs();
+		.regs = get_irq_regs(),
 		.addr = 0,
 	};
 
@@ -3687,12 +3687,12 @@ extern void ftrace_profile_disable(int);
 
 static void tp_perf_counter_destroy(struct perf_counter *counter)
 {
-	ftrace_profile_disable(perf_event_id(&counter->attr));
+	ftrace_profile_disable(counter->attr.config);
 }
 
 static const struct pmu *tp_perf_counter_init(struct perf_counter *counter)
 {
-	int event_id = perf_event_id(&counter->attr);
+	int event_id = counter->attr.config;
 	int ret;
 
 	ret = ftrace_profile_enable(event_id);
-- 
1.6.0.6

[PATCH 2/2] perf_counter: add support to the 'perf' tool for tracepoints

[Jason Baron]

Add support to 'perf list' and 'perf stat' for kernel tracepoints. The
implementation creates a 'for_each_subsystem' and 'for_each_event' for
easy iteration over the tracepoints. The debugfs filesystem must be mounted
at '/sys/kernel/debug'. We can add an optional search path in the future.


Signed-off-by: Jason Baron <jbaron@redhat.com>

---
 tools/perf/util/parse-events.c |  195 +++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/util.h         |    2 +
 2 files changed, 196 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 5184959..d454eac 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -12,6 +12,8 @@ int					nr_counters;
 
 struct perf_counter_attr		attrs[MAX_COUNTERS];
 
+static char default_debugfs_path[] = "/sys/kernel/debug/tracing/events";
+
 struct event_symbol {
 	u8	type;
 	u64	config;
@@ -110,6 +112,86 @@ static unsigned long hw_cache_stat[C(MAX)] = {
  [C(BPU)]	= (CACHE_READ),
 };
 
+#define for_each_subsystem(sys_dir, sys_dirent, sys_next, file, st)	       \
+	while (!readdir_r(sys_dir, &sys_dirent, &sys_next) && sys_next)	       \
+	if (sprintf(file, "%s/%s", default_debugfs_path, sys_dirent.d_name) && \
+	   (stat(file, &st) == 0) && (S_ISDIR(st.st_mode)) &&		       \
+	   (strcmp(sys_dirent.d_name, ".") != 0) &&			       \
+	   (strcmp(sys_dirent.d_name, "..") != 0))
+
+#define for_each_event(sys_dirent, evt_dir, evt_dirent, evt_next, file, st)    \
+	while (!readdir_r(evt_dir, &evt_dirent, &evt_next) && evt_next)        \
+	if (sprintf(file, "%s/%s/%s", default_debugfs_path, sys_dirent.d_name, \
+			evt_dirent.d_name) && 				       \
+	   (stat(file, &st) == 0) && (S_ISDIR(st.st_mode)) &&		       \
+	   (strcmp(evt_dirent.d_name, ".") != 0) && 			       \
+	   (strcmp(evt_dirent.d_name, "..") != 0))
+
+#define MAX_PATH_LENGTH (strlen(default_debugfs_path) + 3 + \
+			(3 * (strlen(sys_dirent.d_name))) + 1)
+#define MAX_EVENT_LENGTH 30
+
+static int valid_debugfs_mount(void)
+{
+	struct statfs st_fs;
+
+	if (statfs(default_debugfs_path, &st_fs) < 0)
+		return -ENOENT;
+	else if (st_fs.f_type != (long) DEBUGFS_MAGIC)
+		return -ENOENT;
+	return 0;
+}
+
+static char *tracepoint_id_to_name(u64 config)
+{
+	static char tracepoint_name[2 * MAX_EVENT_LENGTH];
+	DIR *sys_dir, *evt_dir;
+	struct dirent *sys_next, *evt_next, sys_dirent, evt_dirent;
+	struct stat st;
+	char id_buf[4];
+	int fd;
+	long long id;
+	char evt_path[MAX_PATH_LENGTH];
+
+	if (valid_debugfs_mount())
+		return "unkown";
+
+	sys_dir = opendir(default_debugfs_path);
+	if (!sys_dir)
+		goto cleanup;
+	for_each_subsystem(sys_dir, sys_dirent, sys_next, evt_path, st) {
+		evt_dir = opendir(evt_path);
+		if (!evt_dir)
+			goto cleanup;
+		for_each_event(sys_dirent, evt_dir, evt_dirent, evt_next,
+								evt_path, st) {
+			sprintf(evt_path, "%s/%s/%s/id", default_debugfs_path,
+				sys_dirent.d_name, evt_dirent.d_name);
+			fd = open(evt_path, O_RDONLY);
+			if (fd < 0)
+				continue;
+			if (read(fd, id_buf, sizeof(id_buf)) < 0) {
+				close(fd);
+				continue;
+			}
+			close(fd);
+			id = atoll(id_buf);
+			if ((u64)id == config) {
+				closedir(evt_dir);
+				closedir(sys_dir);
+				snprintf(tracepoint_name, 2 * MAX_EVENT_LENGTH,
+					"%s:%s", sys_dirent.d_name,
+					evt_dirent.d_name);
+				return tracepoint_name;
+			}
+		}
+		closedir(evt_dir);
+	}
+cleanup:
+	closedir(sys_dir);
+	return "unkown";
+}
+
 static int is_cache_op_valid(u8 cache_type, u8 cache_op)
 {
 	if (hw_cache_stat[cache_type] & COP(cache_op))
@@ -177,6 +259,9 @@ char *event_name(int counter)
 			return sw_event_names[config];
 		return "unknown-software";
 
+	case PERF_TYPE_TRACEPOINT:
+		return tracepoint_id_to_name(config);
+
 	default:
 		break;
 	}
@@ -265,6 +350,78 @@ parse_generic_hw_event(const char **str, struct perf_counter_attr *attr)
 	return 1;
 }
 
+static int parse_tracepoint_event(const char **strp,
+				    struct perf_counter_attr *attr)
+{
+	const char *evt_name;
+	char sys_name[MAX_EVENT_LENGTH];
+	DIR *sys_dir, *evt_dir;
+	struct dirent *sys_next, *evt_next, sys_dirent, evt_dirent;
+	struct stat st;
+	char id_buf[4];
+	int fd;
+	unsigned int sys_length, evt_length;
+	u64 id;
+	char evt_path[MAX_PATH_LENGTH];
+
+	if (valid_debugfs_mount())
+		return 0;
+
+	evt_name = strchr(*strp, ':');
+	if (evt_name) {
+		sys_length = evt_name - *strp;
+		if (sys_length >= MAX_EVENT_LENGTH)
+			return 0;
+		strncpy(sys_name, *strp, sys_length);
+		sys_name[sys_length] = '\0';
+		evt_name = evt_name + 1;
+		evt_length = strlen(evt_name);
+		if (evt_length >= MAX_EVENT_LENGTH)
+			return 0;
+	} else
+		return 0;
+
+	sys_dir = opendir(default_debugfs_path);
+	if (!sys_dir)
+		goto cleanup;
+	for_each_subsystem(sys_dir, sys_dirent, sys_next, evt_path, st) {
+		if (strcmp(sys_dirent.d_name, sys_name) == 0) {
+			evt_dir = opendir(evt_path);
+			if (!evt_dir)
+				goto cleanup;
+			for_each_event(sys_dirent, evt_dir, evt_dirent,
+						       evt_next, evt_path, st) {
+				if (strcmp(evt_dirent.d_name, evt_name) == 0) {
+					sprintf(evt_path, "%s/%s/%s/id",
+						default_debugfs_path,
+						sys_dirent.d_name,
+						evt_dirent.d_name);
+					fd = open(evt_path, O_RDONLY);
+					if (fd < 0)
+						continue;
+					if (read(fd, id_buf,
+						 sizeof(id_buf)) < 0) {
+						close(fd);
+						continue;
+					}
+					close(fd);
+					id = atoll(id_buf);
+					attr->config = id;
+					attr->type = PERF_TYPE_TRACEPOINT;
+					closedir(evt_dir);
+					closedir(sys_dir);
+					*strp = evt_name + evt_length;
+					return 1;
+				}
+			}
+			closedir(evt_dir);
+		}
+	}
+cleanup:
+	closedir(sys_dir);
+	return 0;
+}
+
 static int check_events(const char *str, unsigned int i)
 {
 	int n;
@@ -374,7 +531,8 @@ parse_event_modifier(const char **strp, struct perf_counter_attr *attr)
  */
 static int parse_event_symbols(const char **str, struct perf_counter_attr *attr)
 {
-	if (!(parse_raw_event(str, attr) ||
+	if (!(parse_tracepoint_event(str, attr) ||
+	      parse_raw_event(str, attr) ||
 	      parse_numeric_event(str, attr) ||
 	      parse_symbolic_event(str, attr) ||
 	      parse_generic_hw_event(str, attr)))
@@ -423,6 +581,39 @@ static const char * const event_type_descriptors[] = {
 };
 
 /*
+ * Print the events from <debugfs_mount_point>/tracing/events
+ */
+
+static void print_tracepoint_events(void)
+{
+	DIR *sys_dir, *evt_dir;
+	struct dirent *sys_next, *evt_next, sys_dirent, evt_dirent;
+	struct stat st;
+	char evt_path[MAX_PATH_LENGTH];
+
+	if (valid_debugfs_mount())
+		return;
+
+	sys_dir = opendir(default_debugfs_path);
+	if (!sys_dir)
+		goto cleanup;
+	for_each_subsystem(sys_dir, sys_dirent, sys_next, evt_path, st) {
+		evt_dir = opendir(evt_path);
+		if (!evt_dir)
+			goto cleanup;
+		for_each_event(sys_dirent, evt_dir, evt_dirent, evt_next,
+								evt_path, st) {
+			fprintf(stderr, "  %s:%-40s [%s]\n", sys_dirent.d_name,
+				evt_dirent.d_name,
+				event_type_descriptors[PERF_TYPE_TRACEPOINT+1]);
+		}
+		closedir(evt_dir);
+	}
+cleanup:
+	closedir(sys_dir);
+}
+
+/*
  * Print the help text for the event symbols:
  */
 void print_events(void)
@@ -472,5 +663,7 @@ void print_events(void)
 		"rNNN");
 	fprintf(stderr, "\n");
 
+	print_tracepoint_events();
+
 	exit(129);
 }
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index b4be607..68fe157 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -50,6 +50,7 @@
 #include <unistd.h>
 #include <stdio.h>
 #include <sys/stat.h>
+#include <sys/statfs.h>
 #include <fcntl.h>
 #include <stddef.h>
 #include <stdlib.h>
@@ -80,6 +81,7 @@
 #include <netdb.h>
 #include <pwd.h>
 #include <inttypes.h>
+#include "../../../include/linux/magic.h"
 
 #ifndef NO_ICONV
 #include <iconv.h>
-- 
1.6.0.6

Re: [PATCH 2/2] perf_counter: add support to the 'perf' tool for tracepoints

[Peter Zijlstra]

On Mon, 2009-07-06 at 17:12 -0400, Jason Baron wrote:
> Add support to 'perf list' and 'perf stat' for kernel tracepoints. The
> implementation creates a 'for_each_subsystem' and 'for_each_event' for
> easy iteration over the tracepoints. The debugfs filesystem must be mounted
> at '/sys/kernel/debug'. We can add an optional search path in the future.

I don't have a single machine that has that mount point.

[PATCH 3/2] perf_counters: add debugfs dir option

[Jason Baron]

On Tue, Jul 07, 2009 at 10:29:20AM +0200, Peter Zijlstra wrote:
> On Mon, 2009-07-06 at 17:12 -0400, Jason Baron wrote:
> > Add support to 'perf list' and 'perf stat' for kernel tracepoints. The
> > implementation creates a 'for_each_subsystem' and 'for_each_event' for
> > easy iteration over the tracepoints. The debugfs filesystem must be mounted
> > at '/sys/kernel/debug'. We can add an optional search path in the future.
> 
> I don't have a single machine that has that mount point.
> 
> 

ok, The patch below adds support for a 'PERF_DIR_DEBUGFS' environment
variable. It also adds a command line for '--debugfs-dir' and
'--debugfs-dir='. By default 'perf' continues to look in
/sys/kernel/debug, which I believe is most common.

thanks,

-Jason

Signed-off-by: Jason Baron <jbaron@redhat.com>


 tools/perf/perf.c              |   16 ++++++++
 tools/perf/util/cache.h        |    1 +
 tools/perf/util/parse-events.c |   78
++++++++++++++++++++++++----------------
 3 files changed, 62 insertions(+), 33 deletions(-)



diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index c565678..29876ee 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -122,6 +122,20 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)
 			setenv(PERF_WORK_TREE_ENVIRONMENT, cmd + 12, 1);
 			if (envchanged)
 				*envchanged = 1;
+		} else if (!strcmp(cmd, "--debugfs-dir")) {
+			if (*argc < 2) {
+				fprintf(stderr, "No directory given for --debugfs-dir.\n");
+				usage(perf_usage_string);
+			}
+			setenv(PERF_DEBUGFS_ENVIRONMENT, (*argv)[1], 1);
+			if (envchanged)
+				*envchanged = 1;
+			(*argv)++;
+			(*argc)--;
+		} else if (!prefixcmp(cmd, "--debugfs-dir=")) {
+			setenv(PERF_DEBUGFS_ENVIRONMENT, cmd + 14, 1);
+			if (envchanged)
+				*envchanged = 1;
 		} else {
 			fprintf(stderr, "Unknown option: %s\n", cmd);
 			usage(perf_usage_string);
@@ -354,7 +368,7 @@ int main(int argc, const char **argv)
 	cmd = perf_extract_argv0_path(argv[0]);
 	if (!cmd)
 		cmd = "perf-help";
-
+	setenv(PERF_DEBUGFS_ENVIRONMENT, "/sys/kernel/debug/", 0);
 	/*
 	 * "perf-xxxx" is the same as "perf xxxx", but we obviously:
 	 *
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index 161d5f4..4ba4b3e 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -18,6 +18,7 @@
 #define PERFATTRIBUTES_FILE ".perfattributes"
 #define INFOATTRIBUTES_FILE "info/attributes"
 #define ATTRIBUTE_MACRO_PREFIX "[attr]"
+#define PERF_DEBUGFS_ENVIRONMENT "PERF_DIR_DEBUGFS"
 
 typedef int (*config_fn_t)(const char *, const char *, void *);
 extern int perf_default_config(const char *, const char *, void *);
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index d454eac..54b6c23 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -5,6 +5,7 @@
 #include "parse-events.h"
 #include "exec_cmd.h"
 #include "string.h"
+#include "cache.h"
 
 extern char *strcasestr(const char *haystack, const char *needle);
 
@@ -12,8 +13,6 @@ int					nr_counters;
 
 struct perf_counter_attr		attrs[MAX_COUNTERS];
 
-static char default_debugfs_path[] = "/sys/kernel/debug/tracing/events";
-
 struct event_symbol {
 	u8	type;
 	u64	config;
@@ -21,6 +20,8 @@ struct event_symbol {
 	char	*alias;
 };
 
+static char debugfs_path[MAXPATHLEN];
+
 #define CHW(x) .type = PERF_TYPE_HARDWARE, .config = PERF_COUNT_HW_##x
 #define CSW(x) .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_##x
 
@@ -112,30 +113,38 @@ static unsigned long hw_cache_stat[C(MAX)] = {
  [C(BPU)]	= (CACHE_READ),
 };
 
-#define for_each_subsystem(sys_dir, sys_dirent, sys_next, file, st)	       \
-	while (!readdir_r(sys_dir, &sys_dirent, &sys_next) && sys_next)	       \
-	if (sprintf(file, "%s/%s", default_debugfs_path, sys_dirent.d_name) && \
-	   (stat(file, &st) == 0) && (S_ISDIR(st.st_mode)) &&		       \
-	   (strcmp(sys_dirent.d_name, ".") != 0) &&			       \
+#define for_each_subsystem(sys_dir, sys_dirent, sys_next, file, st)	     \
+	while (!readdir_r(sys_dir, &sys_dirent, &sys_next) && sys_next)	     \
+	if (snprintf(file, MAXPATHLEN, "%s/%s/%s",			     \
+		     getenv(PERF_DEBUGFS_ENVIRONMENT), "tracing/events",     \
+		     sys_dirent.d_name) &&				     \
+	   (stat(file, &st) == 0) && (S_ISDIR(st.st_mode)) &&		     \
+	   (strcmp(sys_dirent.d_name, ".") != 0) &&			     \
 	   (strcmp(sys_dirent.d_name, "..") != 0))
 
-#define for_each_event(sys_dirent, evt_dir, evt_dirent, evt_next, file, st)    \
-	while (!readdir_r(evt_dir, &evt_dirent, &evt_next) && evt_next)        \
-	if (sprintf(file, "%s/%s/%s", default_debugfs_path, sys_dirent.d_name, \
-			evt_dirent.d_name) && 				       \
-	   (stat(file, &st) == 0) && (S_ISDIR(st.st_mode)) &&		       \
-	   (strcmp(evt_dirent.d_name, ".") != 0) && 			       \
+#define for_each_event(sys_dirent, evt_dir, evt_dirent, evt_next, file, st)  \
+	while (!readdir_r(evt_dir, &evt_dirent, &evt_next) && evt_next)	     \
+	if (snprintf(file, MAXPATHLEN, "%s/%s/%s/%s",			     \
+		     getenv(PERF_DEBUGFS_ENVIRONMENT), "tracing/events",     \
+		     sys_dirent.d_name, evt_dirent.d_name) &&		     \
+	   (stat(file, &st) == 0) && (S_ISDIR(st.st_mode)) &&		     \
+	   (strcmp(evt_dirent.d_name, ".") != 0) &&			     \
 	   (strcmp(evt_dirent.d_name, "..") != 0))
 
-#define MAX_PATH_LENGTH (strlen(default_debugfs_path) + 3 + \
-			(3 * (strlen(sys_dirent.d_name))) + 1)
 #define MAX_EVENT_LENGTH 30
 
-static int valid_debugfs_mount(void)
+static void set_debugfs_path(void)
+{
+	snprintf(debugfs_path, MAXPATHLEN, "%s/%s",
+		 getenv(PERF_DEBUGFS_ENVIRONMENT),
+		 "tracing/events");
+}
+
+static int valid_debugfs_mount(const char *mount)
 {
 	struct statfs st_fs;
 
-	if (statfs(default_debugfs_path, &st_fs) < 0)
+	if (statfs(mount, &st_fs) < 0)
 		return -ENOENT;
 	else if (st_fs.f_type != (long) DEBUGFS_MAGIC)
 		return -ENOENT;
@@ -151,12 +160,13 @@ static char *tracepoint_id_to_name(u64 config)
 	char id_buf[4];
 	int fd;
 	long long id;
-	char evt_path[MAX_PATH_LENGTH];
+	char evt_path[MAXPATHLEN];
 
-	if (valid_debugfs_mount())
+	set_debugfs_path();
+	if (valid_debugfs_mount(debugfs_path))
 		return "unkown";
 
-	sys_dir = opendir(default_debugfs_path);
+	sys_dir = opendir(debugfs_path);
 	if (!sys_dir)
 		goto cleanup;
 	for_each_subsystem(sys_dir, sys_dirent, sys_next, evt_path, st) {
@@ -165,8 +175,9 @@ static char *tracepoint_id_to_name(u64 config)
 			goto cleanup;
 		for_each_event(sys_dirent, evt_dir, evt_dirent, evt_next,
 								evt_path, st) {
-			sprintf(evt_path, "%s/%s/%s/id", default_debugfs_path,
-				sys_dirent.d_name, evt_dirent.d_name);
+			snprintf(evt_path, MAXPATHLEN, "%s/%s/%s/id",
+				 debugfs_path, sys_dirent.d_name,
+				 evt_dirent.d_name);
 			fd = open(evt_path, O_RDONLY);
 			if (fd < 0)
 				continue;
@@ -362,9 +373,10 @@ static int parse_tracepoint_event(const char **strp,
 	int fd;
 	unsigned int sys_length, evt_length;
 	u64 id;
-	char evt_path[MAX_PATH_LENGTH];
+	char evt_path[MAXPATHLEN];
 
-	if (valid_debugfs_mount())
+	set_debugfs_path();
+	if (valid_debugfs_mount(debugfs_path))
 		return 0;
 
 	evt_name = strchr(*strp, ':');
@@ -381,7 +393,7 @@ static int parse_tracepoint_event(const char **strp,
 	} else
 		return 0;
 
-	sys_dir = opendir(default_debugfs_path);
+	sys_dir = opendir(debugfs_path);
 	if (!sys_dir)
 		goto cleanup;
 	for_each_subsystem(sys_dir, sys_dirent, sys_next, evt_path, st) {
@@ -392,10 +404,11 @@ static int parse_tracepoint_event(const char **strp,
 			for_each_event(sys_dirent, evt_dir, evt_dirent,
 						       evt_next, evt_path, st) {
 				if (strcmp(evt_dirent.d_name, evt_name) == 0) {
-					sprintf(evt_path, "%s/%s/%s/id",
-						default_debugfs_path,
-						sys_dirent.d_name,
-						evt_dirent.d_name);
+					snprintf(evt_path, MAXPATHLEN,
+						 "%s/%s/%s/id",
+						 debugfs_path,
+						 sys_dirent.d_name,
+						 evt_dirent.d_name);
 					fd = open(evt_path, O_RDONLY);
 					if (fd < 0)
 						continue;
@@ -589,12 +602,13 @@ static void print_tracepoint_events(void)
 	DIR *sys_dir, *evt_dir;
 	struct dirent *sys_next, *evt_next, sys_dirent, evt_dirent;
 	struct stat st;
-	char evt_path[MAX_PATH_LENGTH];
+	char evt_path[MAXPATHLEN];
 
-	if (valid_debugfs_mount())
+	set_debugfs_path();
+	if (valid_debugfs_mount(debugfs_path))
 		return;
 
-	sys_dir = opendir(default_debugfs_path);
+	sys_dir = opendir(debugfs_path);
 	if (!sys_dir)
 		goto cleanup;
 	for_each_subsystem(sys_dir, sys_dirent, sys_next, evt_path, st) {

Re: [PATCH 3/2] perf_counters: add debugfs dir option

[Frederic Weisbecker]

On Wed, Jul 08, 2009 at 04:17:43PM -0400, Jason Baron wrote:
> On Tue, Jul 07, 2009 at 10:29:20AM +0200, Peter Zijlstra wrote:
> > On Mon, 2009-07-06 at 17:12 -0400, Jason Baron wrote:
> > > Add support to 'perf list' and 'perf stat' for kernel tracepoints. The
> > > implementation creates a 'for_each_subsystem' and 'for_each_event' for
> > > easy iteration over the tracepoints. The debugfs filesystem must be mounted
> > > at '/sys/kernel/debug'. We can add an optional search path in the future.
> > 
> > I don't have a single machine that has that mount point.
> > 
> > 
> 
> ok, The patch below adds support for a 'PERF_DIR_DEBUGFS' environment
> variable. It also adds a command line for '--debugfs-dir' and
> '--debugfs-dir='. By default 'perf' continues to look in
> /sys/kernel/debug, which I believe is most common.
> 
> thanks,
> 
> -Jason
> 
> Signed-off-by: Jason Baron <jbaron@redhat.com>



Or why not parsing /etc/mtab ?

Thanks.



> 
> 
>  tools/perf/perf.c              |   16 ++++++++
>  tools/perf/util/cache.h        |    1 +
>  tools/perf/util/parse-events.c |   78
> ++++++++++++++++++++++++----------------
>  3 files changed, 62 insertions(+), 33 deletions(-)
> 
> 
> 
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index c565678..29876ee 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -122,6 +122,20 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)
>  			setenv(PERF_WORK_TREE_ENVIRONMENT, cmd + 12, 1);
>  			if (envchanged)
>  				*envchanged = 1;
> +		} else if (!strcmp(cmd, "--debugfs-dir")) {
> +			if (*argc < 2) {
> +				fprintf(stderr, "No directory given for --debugfs-dir.\n");
> +				usage(perf_usage_string);
> +			}
> +			setenv(PERF_DEBUGFS_ENVIRONMENT, (*argv)[1], 1);
> +			if (envchanged)
> +				*envchanged = 1;
> +			(*argv)++;
> +			(*argc)--;
> +		} else if (!prefixcmp(cmd, "--debugfs-dir=")) {
> +			setenv(PERF_DEBUGFS_ENVIRONMENT, cmd + 14, 1);
> +			if (envchanged)
> +				*envchanged = 1;
>  		} else {
>  			fprintf(stderr, "Unknown option: %s\n", cmd);
>  			usage(perf_usage_string);
> @@ -354,7 +368,7 @@ int main(int argc, const char **argv)
>  	cmd = perf_extract_argv0_path(argv[0]);
>  	if (!cmd)
>  		cmd = "perf-help";
> -
> +	setenv(PERF_DEBUGFS_ENVIRONMENT, "/sys/kernel/debug/", 0);
>  	/*
>  	 * "perf-xxxx" is the same as "perf xxxx", but we obviously:
>  	 *
> diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
> index 161d5f4..4ba4b3e 100644
> --- a/tools/perf/util/cache.h
> +++ b/tools/perf/util/cache.h
> @@ -18,6 +18,7 @@
>  #define PERFATTRIBUTES_FILE ".perfattributes"
>  #define INFOATTRIBUTES_FILE "info/attributes"
>  #define ATTRIBUTE_MACRO_PREFIX "[attr]"
> +#define PERF_DEBUGFS_ENVIRONMENT "PERF_DIR_DEBUGFS"
>  
>  typedef int (*config_fn_t)(const char *, const char *, void *);
>  extern int perf_default_config(const char *, const char *, void *);
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index d454eac..54b6c23 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -5,6 +5,7 @@
>  #include "parse-events.h"
>  #include "exec_cmd.h"
>  #include "string.h"
> +#include "cache.h"
>  
>  extern char *strcasestr(const char *haystack, const char *needle);
>  
> @@ -12,8 +13,6 @@ int					nr_counters;
>  
>  struct perf_counter_attr		attrs[MAX_COUNTERS];
>  
> -static char default_debugfs_path[] = "/sys/kernel/debug/tracing/events";
> -
>  struct event_symbol {
>  	u8	type;
>  	u64	config;
> @@ -21,6 +20,8 @@ struct event_symbol {
>  	char	*alias;
>  };
>  
> +static char debugfs_path[MAXPATHLEN];
> +
>  #define CHW(x) .type = PERF_TYPE_HARDWARE, .config = PERF_COUNT_HW_##x
>  #define CSW(x) .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_##x
>  
> @@ -112,30 +113,38 @@ static unsigned long hw_cache_stat[C(MAX)] = {
>   [C(BPU)]	= (CACHE_READ),
>  };
>  
> -#define for_each_subsystem(sys_dir, sys_dirent, sys_next, file, st)	       \
> -	while (!readdir_r(sys_dir, &sys_dirent, &sys_next) && sys_next)	       \
> -	if (sprintf(file, "%s/%s", default_debugfs_path, sys_dirent.d_name) && \
> -	   (stat(file, &st) == 0) && (S_ISDIR(st.st_mode)) &&		       \
> -	   (strcmp(sys_dirent.d_name, ".") != 0) &&			       \
> +#define for_each_subsystem(sys_dir, sys_dirent, sys_next, file, st)	     \
> +	while (!readdir_r(sys_dir, &sys_dirent, &sys_next) && sys_next)	     \
> +	if (snprintf(file, MAXPATHLEN, "%s/%s/%s",			     \
> +		     getenv(PERF_DEBUGFS_ENVIRONMENT), "tracing/events",     \
> +		     sys_dirent.d_name) &&				     \
> +	   (stat(file, &st) == 0) && (S_ISDIR(st.st_mode)) &&		     \
> +	   (strcmp(sys_dirent.d_name, ".") != 0) &&			     \
>  	   (strcmp(sys_dirent.d_name, "..") != 0))
>  
> -#define for_each_event(sys_dirent, evt_dir, evt_dirent, evt_next, file, st)    \
> -	while (!readdir_r(evt_dir, &evt_dirent, &evt_next) && evt_next)        \
> -	if (sprintf(file, "%s/%s/%s", default_debugfs_path, sys_dirent.d_name, \
> -			evt_dirent.d_name) && 				       \
> -	   (stat(file, &st) == 0) && (S_ISDIR(st.st_mode)) &&		       \
> -	   (strcmp(evt_dirent.d_name, ".") != 0) && 			       \
> +#define for_each_event(sys_dirent, evt_dir, evt_dirent, evt_next, file, st)  \
> +	while (!readdir_r(evt_dir, &evt_dirent, &evt_next) && evt_next)	     \
> +	if (snprintf(file, MAXPATHLEN, "%s/%s/%s/%s",			     \
> +		     getenv(PERF_DEBUGFS_ENVIRONMENT), "tracing/events",     \
> +		     sys_dirent.d_name, evt_dirent.d_name) &&		     \
> +	   (stat(file, &st) == 0) && (S_ISDIR(st.st_mode)) &&		     \
> +	   (strcmp(evt_dirent.d_name, ".") != 0) &&			     \
>  	   (strcmp(evt_dirent.d_name, "..") != 0))
>  
> -#define MAX_PATH_LENGTH (strlen(default_debugfs_path) + 3 + \
> -			(3 * (strlen(sys_dirent.d_name))) + 1)
>  #define MAX_EVENT_LENGTH 30
>  
> -static int valid_debugfs_mount(void)
> +static void set_debugfs_path(void)
> +{
> +	snprintf(debugfs_path, MAXPATHLEN, "%s/%s",
> +		 getenv(PERF_DEBUGFS_ENVIRONMENT),
> +		 "tracing/events");
> +}
> +
> +static int valid_debugfs_mount(const char *mount)
>  {
>  	struct statfs st_fs;
>  
> -	if (statfs(default_debugfs_path, &st_fs) < 0)
> +	if (statfs(mount, &st_fs) < 0)
>  		return -ENOENT;
>  	else if (st_fs.f_type != (long) DEBUGFS_MAGIC)
>  		return -ENOENT;
> @@ -151,12 +160,13 @@ static char *tracepoint_id_to_name(u64 config)
>  	char id_buf[4];
>  	int fd;
>  	long long id;
> -	char evt_path[MAX_PATH_LENGTH];
> +	char evt_path[MAXPATHLEN];
>  
> -	if (valid_debugfs_mount())
> +	set_debugfs_path();
> +	if (valid_debugfs_mount(debugfs_path))
>  		return "unkown";
>  
> -	sys_dir = opendir(default_debugfs_path);
> +	sys_dir = opendir(debugfs_path);
>  	if (!sys_dir)
>  		goto cleanup;
>  	for_each_subsystem(sys_dir, sys_dirent, sys_next, evt_path, st) {
> @@ -165,8 +175,9 @@ static char *tracepoint_id_to_name(u64 config)
>  			goto cleanup;
>  		for_each_event(sys_dirent, evt_dir, evt_dirent, evt_next,
>  								evt_path, st) {
> -			sprintf(evt_path, "%s/%s/%s/id", default_debugfs_path,
> -				sys_dirent.d_name, evt_dirent.d_name);
> +			snprintf(evt_path, MAXPATHLEN, "%s/%s/%s/id",
> +				 debugfs_path, sys_dirent.d_name,
> +				 evt_dirent.d_name);
>  			fd = open(evt_path, O_RDONLY);
>  			if (fd < 0)
>  				continue;
> @@ -362,9 +373,10 @@ static int parse_tracepoint_event(const char **strp,
>  	int fd;
>  	unsigned int sys_length, evt_length;
>  	u64 id;
> -	char evt_path[MAX_PATH_LENGTH];
> +	char evt_path[MAXPATHLEN];
>  
> -	if (valid_debugfs_mount())
> +	set_debugfs_path();
> +	if (valid_debugfs_mount(debugfs_path))
>  		return 0;
>  
>  	evt_name = strchr(*strp, ':');
> @@ -381,7 +393,7 @@ static int parse_tracepoint_event(const char **strp,
>  	} else
>  		return 0;
>  
> -	sys_dir = opendir(default_debugfs_path);
> +	sys_dir = opendir(debugfs_path);
>  	if (!sys_dir)
>  		goto cleanup;
>  	for_each_subsystem(sys_dir, sys_dirent, sys_next, evt_path, st) {
> @@ -392,10 +404,11 @@ static int parse_tracepoint_event(const char **strp,
>  			for_each_event(sys_dirent, evt_dir, evt_dirent,
>  						       evt_next, evt_path, st) {
>  				if (strcmp(evt_dirent.d_name, evt_name) == 0) {
> -					sprintf(evt_path, "%s/%s/%s/id",
> -						default_debugfs_path,
> -						sys_dirent.d_name,
> -						evt_dirent.d_name);
> +					snprintf(evt_path, MAXPATHLEN,
> +						 "%s/%s/%s/id",
> +						 debugfs_path,
> +						 sys_dirent.d_name,
> +						 evt_dirent.d_name);
>  					fd = open(evt_path, O_RDONLY);
>  					if (fd < 0)
>  						continue;
> @@ -589,12 +602,13 @@ static void print_tracepoint_events(void)
>  	DIR *sys_dir, *evt_dir;
>  	struct dirent *sys_next, *evt_next, sys_dirent, evt_dirent;
>  	struct stat st;
> -	char evt_path[MAX_PATH_LENGTH];
> +	char evt_path[MAXPATHLEN];
>  
> -	if (valid_debugfs_mount())
> +	set_debugfs_path();
> +	if (valid_debugfs_mount(debugfs_path))
>  		return;
>  
> -	sys_dir = opendir(default_debugfs_path);
> +	sys_dir = opendir(debugfs_path);
>  	if (!sys_dir)
>  		goto cleanup;
>  	for_each_subsystem(sys_dir, sys_dirent, sys_next, evt_path, st) {
> 
> 

Re: [PATCH 3/2] perf_counters: add debugfs dir option

[Jason Baron]

On Thu, Jul 09, 2009 at 02:47:17AM +0200, Frederic Weisbecker wrote:
> On Wed, Jul 08, 2009 at 04:17:43PM -0400, Jason Baron wrote:
> > On Tue, Jul 07, 2009 at 10:29:20AM +0200, Peter Zijlstra wrote:
> > > On Mon, 2009-07-06 at 17:12 -0400, Jason Baron wrote:
> > > > Add support to 'perf list' and 'perf stat' for kernel tracepoints. The
> > > > implementation creates a 'for_each_subsystem' and 'for_each_event' for
> > > > easy iteration over the tracepoints. The debugfs filesystem must be mounted
> > > > at '/sys/kernel/debug'. We can add an optional search path in the future.
> > > 
> > > I don't have a single machine that has that mount point.
> > > 
> > > 
> > 
> > ok, The patch below adds support for a 'PERF_DIR_DEBUGFS' environment
> > variable. It also adds a command line for '--debugfs-dir' and
> > '--debugfs-dir='. By default 'perf' continues to look in
> > /sys/kernel/debug, which I believe is most common.
> > 
> > thanks,
> > 
> > -Jason
> > 
> > Signed-off-by: Jason Baron <jbaron@redhat.com>
> 
> 
> 
> Or why not parsing /etc/mtab ?
> 
> Thanks.
> 
> 
> 

good idea. I'll add that.

thanks,

-Jason

Re: [PATCH 3/2] perf_counters: add debugfs dir option

[Ingo Molnar]

* Jason Baron <jbaron@redhat.com> wrote:

> On Thu, Jul 09, 2009 at 02:47:17AM +0200, Frederic Weisbecker wrote:
> > On Wed, Jul 08, 2009 at 04:17:43PM -0400, Jason Baron wrote:
> > > On Tue, Jul 07, 2009 at 10:29:20AM +0200, Peter Zijlstra wrote:
> > > > On Mon, 2009-07-06 at 17:12 -0400, Jason Baron wrote:
> > > > > Add support to 'perf list' and 'perf stat' for kernel tracepoints. The
> > > > > implementation creates a 'for_each_subsystem' and 'for_each_event' for
> > > > > easy iteration over the tracepoints. The debugfs filesystem must be mounted
> > > > > at '/sys/kernel/debug'. We can add an optional search path in the future.
> > > > 
> > > > I don't have a single machine that has that mount point.
> > > > 
> > > > 
> > > 
> > > ok, The patch below adds support for a 'PERF_DIR_DEBUGFS' environment
> > > variable. It also adds a command line for '--debugfs-dir' and
> > > '--debugfs-dir='. By default 'perf' continues to look in
> > > /sys/kernel/debug, which I believe is most common.
> > > 
> > > thanks,
> > > 
> > > -Jason
> > > 
> > > Signed-off-by: Jason Baron <jbaron@redhat.com>
> > 
> > 
> > 
> > Or why not parsing /etc/mtab ?
> > 
> > Thanks.
> > 
> > 
> > 
> 
> good idea. I'll add that.

Have you seen:

  Subject: [PATCH 2/2] perf tool: Add a trace point event parser.

from Chris Wilson? It already includes a /proc/mounts parser.

	Ingo

Re: [PATCH 2/2] perf_counter: add support to the 'perf' tool for tracepoints

[Ingo Molnar]

* Jason Baron <jbaron@redhat.com> wrote:

> +static char *tracepoint_id_to_name(u64 config)
> +{
> +	static char tracepoint_name[2 * MAX_EVENT_LENGTH];
> +	DIR *sys_dir, *evt_dir;
> +	struct dirent *sys_next, *evt_next, sys_dirent, evt_dirent;
> +	struct stat st;
> +	char id_buf[4];
> +	int fd;
> +	long long id;

could this become u64?

> +	char evt_path[MAX_PATH_LENGTH];
> +
> +	if (valid_debugfs_mount())
> +		return "unkown";
> +
> +	sys_dir = opendir(default_debugfs_path);
> +	if (!sys_dir)
> +		goto cleanup;
> +	for_each_subsystem(sys_dir, sys_dirent, sys_next, evt_path, st) {
> +		evt_dir = opendir(evt_path);
> +		if (!evt_dir)
> +			goto cleanup;
> +		for_each_event(sys_dirent, evt_dir, evt_dirent, evt_next,
> +								evt_path, st) {
> +			sprintf(evt_path, "%s/%s/%s/id", default_debugfs_path,
> +				sys_dirent.d_name, evt_dirent.d_name);
> +			fd = open(evt_path, O_RDONLY);
> +			if (fd < 0)
> +				continue;
> +			if (read(fd, id_buf, sizeof(id_buf)) < 0) {
> +				close(fd);
> +				continue;
> +			}
> +			close(fd);
> +			id = atoll(id_buf);
> +			if ((u64)id == config) {

... and thus the cast here could be dropped?

> +				closedir(evt_dir);
> +				closedir(sys_dir);
> +				snprintf(tracepoint_name, 2 * MAX_EVENT_LENGTH,
> +					"%s:%s", sys_dirent.d_name,
> +					evt_dirent.d_name);
> +				return tracepoint_name;
> +			}
> +		}
> +		closedir(evt_dir);
> +	}
> +cleanup:
> +	closedir(sys_dir);
> +	return "unkown";
> +}
> +
>  static int is_cache_op_valid(u8 cache_type, u8 cache_op)
>  {
>  	if (hw_cache_stat[cache_type] & COP(cache_op))
> @@ -177,6 +259,9 @@ char *event_name(int counter)
>  			return sw_event_names[config];
>  		return "unknown-software";
>  
> +	case PERF_TYPE_TRACEPOINT:
> +		return tracepoint_id_to_name(config);
> +
>  	default:
>  		break;
>  	}
> @@ -265,6 +350,78 @@ parse_generic_hw_event(const char **str, struct perf_counter_attr *attr)
>  	return 1;
>  }
>  
> +static int parse_tracepoint_event(const char **strp,
> +				    struct perf_counter_attr *attr)
> +{
> +	const char *evt_name;
> +	char sys_name[MAX_EVENT_LENGTH];
> +	DIR *sys_dir, *evt_dir;
> +	struct dirent *sys_next, *evt_next, sys_dirent, evt_dirent;
> +	struct stat st;
> +	char id_buf[4];
> +	int fd;
> +	unsigned int sys_length, evt_length;
> +	u64 id;
> +	char evt_path[MAX_PATH_LENGTH];
> +
> +	if (valid_debugfs_mount())
> +		return 0;
> +
> +	evt_name = strchr(*strp, ':');
> +	if (evt_name) {
> +		sys_length = evt_name - *strp;
> +		if (sys_length >= MAX_EVENT_LENGTH)
> +			return 0;
> +		strncpy(sys_name, *strp, sys_length);
> +		sys_name[sys_length] = '\0';
> +		evt_name = evt_name + 1;
> +		evt_length = strlen(evt_name);
> +		if (evt_length >= MAX_EVENT_LENGTH)
> +			return 0;
> +	} else
> +		return 0;
> +
> +	sys_dir = opendir(default_debugfs_path);
> +	if (!sys_dir)
> +		goto cleanup;
> +	for_each_subsystem(sys_dir, sys_dirent, sys_next, evt_path, st) {

a minor request. Just like you added a newline after the return 0 
above, please add a newline after separate blocks of code as well - 
i.e after the 'goto cleanup' above. It makes the loop which begins 
stand out on its own, and makes the sys_dir initialization look 
self-sufficient as well. (which it is)

> +		if (strcmp(sys_dirent.d_name, sys_name) == 0) {
> +			evt_dir = opendir(evt_path);
> +			if (!evt_dir)
> +				goto cleanup;
> +			for_each_event(sys_dirent, evt_dir, evt_dirent,
> +						       evt_next, evt_path, st) {
> +				if (strcmp(evt_dirent.d_name, evt_name) == 0) {
> +					sprintf(evt_path, "%s/%s/%s/id",
> +						default_debugfs_path,
> +						sys_dirent.d_name,
> +						evt_dirent.d_name);
> +					fd = open(evt_path, O_RDONLY);
> +					if (fd < 0)
> +						continue;
> +					if (read(fd, id_buf,
> +						 sizeof(id_buf)) < 0) {
> +						close(fd);
> +						continue;
> +					}
> +					close(fd);
> +					id = atoll(id_buf);
> +					attr->config = id;
> +					attr->type = PERF_TYPE_TRACEPOINT;
> +					closedir(evt_dir);
> +					closedir(sys_dir);
> +					*strp = evt_name + evt_length;
> +					return 1;
> +				}
> +			}
> +			closedir(evt_dir);
> +		}
> +	}
> +cleanup:
> +	closedir(sys_dir);
> +	return 0;
> +}

Ok, this whole feature looks nice. A few minor details need fixing: 
the two functions above tracepoint_id_to_name() and 
parse_tracepoint_event() are a bit large and look very crowded.

one trick to win an indentation level would be to turn this:

> +		if (strcmp(sys_dirent.d_name, sys_name) == 0) {
> +			evt_dir = opendir(evt_path);

into:

		if (strcmp(sys_dirent.d_name, sys_name))
			continue;

		evt_dir = opendir(evt_path);

(Also, for logic operations please write '!x' instead of 'x == 0' - 
it's easy to see only the beginning of the line and miss the 
effective negation.)

The other thing to do would be to move the inner core of the loop 
out into a helper inline function.

plus, this:

> +	if (evt_name) {
> +		sys_length = evt_name - *strp;
> +		if (sys_length >= MAX_EVENT_LENGTH)
> +			return 0;
> +		strncpy(sys_name, *strp, sys_length);
> +		sys_name[sys_length] = '\0';
> +		evt_name = evt_name + 1;
> +		evt_length = strlen(evt_name);
> +		if (evt_length >= MAX_EVENT_LENGTH)
> +			return 0;
> +	} else
> +		return 0;

should be written as:

	if (!evt_name)
		return 0;

	sys_length = evt_name - *strp;
	...

i.e. we should try to compress the visual complexity of 
code as much as possible.

Also ... all code flows return 0, regardless of error or not error. 
Is this intentional?

	Ingo

Re: [PATCH 0/2] perf_counter: add tracepoint support

[Frederic Weisbecker]

On Mon, Jul 06, 2009 at 05:11:53PM -0400, Jason Baron wrote:
> 
> hi,
> 
> The following two patches build on Peter's initial tracepoint perf counter work.
> The first patches enables the kernel side, and the second patch adds support to
> the 'perf' tool itself. The debugfs filesystem needs to be mounted to make this
> work and it is searched in the default location '/sys/kernel/debug'. We can
> add an optional parameter to search for it elsewhere if need be.
> 
> Below, I show the output of 'perf list', and 'perf stat' for a tbench run.
> 
[...] 
> >  perf stat  -a -e sched:sched_switch -e sched:sched_wakeup -e irq:irq_handler_entry tbench 4
> 
>  Performance counter stats for 'tbench 4':
> 
>       194658693  sched:sched_switch      
>        97479295  sched:sched_wakeup      
>            9122  irq:irq_handler_entry   
> 
>   720.171439729  seconds time elapsed
> 


Cool, now the next stage that would be nice would be to have the
events themselves and not only the counts so that we can sort the
stats using values from the fields.

I have some ideas about that. We shouldn't interfere with ftrace job
and then use a private buffer for ftrace events used by perf.
But still this private buffer should be a private instance of the
ring buffer implemented in kernel/trace/, which is already well adapted for
the ftrace events (handles recursion).

(RFD parenthesis: why not using the one in kernel/trace for all perf events?
Now that it is lockless, it would fit nicely inside: no fear about NMI
context).

Anyway, unless you are already working on it, I'll try to adapt the content
of these events with perfcounter.

Thanks,
Frederic.


> 
>   -add kernel tracepoint support
>   -add support for tracepoints to the perf tool
> 
>  init/Kconfig                   |    2 +-
>  kernel/perf_counter.c          |    6 +-
>  tools/perf/util/parse-events.c |  195 +++++++++++++++++++++++++++++++++++++++-
>  tools/perf/util/util.h         |    2 +
>  4 files changed, 200 insertions(+), 5 deletions(-)
> 

Re: [PATCH 0/2] perf_counter: add tracepoint support

[Peter Zijlstra]

On Thu, 2009-07-09 at 05:10 +0200, Frederic Weisbecker wrote:
> (RFD parenthesis: why not using the one in kernel/trace for all perf events?
> Now that it is lockless, it would fit nicely inside: no fear about NMI
> context).

because that one doesn't have a single-buffer mode you can mmap().

Re: [PATCH 0/2] perf_counter: add tracepoint support

[Ingo Molnar]

* Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:

> On Thu, 2009-07-09 at 05:10 +0200, Frederic Weisbecker wrote:

> > (RFD parenthesis: why not using the one in kernel/trace for all 
> > perf events? Now that it is lockless, it would fit nicely 
> > inside: no fear about NMI context).
> 
> because that one doesn't have a single-buffer mode you can mmap().

Yeah, the ring-buffer code could certainly be abstracted out better 
to allow this. Once that is done it would allow us to do a couple of 
nice things: such as splice support (which could perhaps be faster 
than mmap for perf record) and the sampling of tracepoints would 
become a whole lot easier a well. But it's certainly not a trivial 
job!

	Ingo