Message-Id: <20170216101617.4791-1-ravi.bangoria@linux.vnet.ibm.com>
Date:   Thu, 16 Feb 2017 15:46:17 +0530
From:   Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com>
To:     mingo@...hat.com, acme@...nel.org, masami.hiramatsu.pt@...achi.com
Cc:     brendan.d.gregg@...il.com, peterz@...radead.org,
        alexander.shishkin@...ux.intel.com, wangnan0@...wei.com,
        jolsa@...nel.org, ak@...ux.intel.com, treeze.taeung@...il.com,
        mathieu.poirier@...aro.org, hekuang@...wei.com,
        sukadev@...ux.vnet.ibm.com, ananth@...ibm.com,
        naveen.n.rao@...ux.vnet.ibm.com, colin.ing@...onical.com,
        adrian.hunter@...el.com, linux-kernel@...r.kernel.org,
        hemant@...ux.vnet.ibm.com,
        Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com>
Subject: [RFC] perf/sdt: Directly record SDT event with 'perf record'

All events from 'perf list', except SDT events, can be directly recorded
with 'perf record'. But, the flow is little different for SDT events.
Probe point for SDT event needs to be created using 'perf probe' before
recording it using 'perf record'.

As suggested by Ingo[1], it's better to make this process simple by
creating probe points automatically with 'perf record' for SDT events.

This patch disables 'perf probe' on SDT events to simplify usage. It
enables recording SDT event only with 'perf record'.

This removes all those 'multiple events with same name' issues by not
allowing manual probe creation to user. When there are multiple events
with same name, 'perf record' will record all of them (in line with
other tools supporting SDT (systemtap)).

I know 'perf probe' for SDT events has already became interface and
people are using it. But, doing this change will make user interface very
easy and also it will make tool behaviour consistent. Also, it won't
require any changes in uprobe_events structure (suggested by Masami[2]).

After patch:

  $ perf list
      ...
      sdt_libpthread:mutex_entry                         [SDT event]
      ...

  $ perf probe -x /usr/lib64/libpthread-2.24.so %sdt_libpthread:mutex_entry
    SDT events don't need to be put in place using 'perf probe' anymore.
    You can directly record on SDT events using 'perf record'
      Error: Command Parse Error.

  $ perf record -a -e %sdt_libpthread:mutex_entry
    Warning : Recording on 2 occurences of sdt_libpthread:mutex_entry
    ^C[ perf record: Woken up 1 times to write data ]
    [ perf record: Captured and wrote 0.190 MB perf.data (33 samples) ]

  $ perf evlist
    sdt_libpthread:mutex_entry
    sdt_libpthread:mutex_entry_1


Note:
  - This is no way a proper patch. Sending this RFC to get thoughts on the
    idea.
  - This is Hemant's patch[3] rebased to acme/perf/core, with few changes
    to disable 'perf probe' on SDT event.


[1] https://lkml.org/lkml/2017/2/7/59
[2] https://lkml.org/lkml/2016/4/30/50
[3] https://lkml.org/lkml/2016/5/3/810




Signed-off-by: Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com>
---
 tools/perf/builtin-record.c    | 21 +++++++++
 tools/perf/perf.h              |  1 +
 tools/perf/util/parse-events.c | 53 ++++++++++++++++++++++-
 tools/perf/util/parse-events.h |  1 +
 tools/perf/util/probe-event.c  | 43 ++++++++++++++++++-
 tools/perf/util/probe-event.h  |  4 ++
 tools/perf/util/probe-file.c   | 97 ++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/probe-file.h   |  8 ++++
 8 files changed, 225 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 2ddf189..37722f0 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -39,6 +39,7 @@
 #include "util/trigger.h"
 #include "util/perf-hooks.h"
 #include "asm/bug.h"
+#include "util/probe-file.h"
 
 #include <unistd.h>
 #include <sched.h>
@@ -73,6 +74,7 @@ struct record {
 	bool			timestamp_filename;
 	struct switch_output	switch_output;
 	unsigned long long	samples;
+	struct list_head	sdt_event_list;
 };
 
 static volatile int auxtrace_record__snapshot_started;
@@ -1502,6 +1504,23 @@ static struct record record = {
 	},
 };
 
+void sdt_event_list__add(struct list_head *sdt_event_list)
+{
+	if (list_empty(sdt_event_list))
+		return;
+	list_splice(sdt_event_list, &record.sdt_event_list);
+}
+
+bool is_cmd_record(void)
+{
+	return (record.evlist != NULL);
+}
+
+static void sdt_event_list__remove(struct list_head *sdt_event_list)
+{
+	return remove_sdt_event_list(sdt_event_list);
+}
+
 const char record_callchain_help[] = CALLCHAIN_RECORD_HELP
 	"\n\t\t\t\tDefault: fp";
 
@@ -1670,6 +1689,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (rec->evlist == NULL)
 		return -ENOMEM;
 
+	INIT_LIST_HEAD(&rec->sdt_event_list);
 	err = perf_config(perf_record_config, rec);
 	if (err)
 		return err;
@@ -1836,6 +1856,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 	perf_evlist__delete(rec->evlist);
 	symbol__exit();
 	auxtrace_record__free(rec->itr);
+	sdt_event_list__remove(&rec->sdt_event_list);
 	return err;
 }
 
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 1c27d94..9d8e5fe 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -76,4 +76,5 @@ struct record_opts {
 struct option;
 extern const char * const *record_usage;
 extern struct option *record_options;
+bool is_cmd_record(void);
 #endif
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 07be076..3400a5a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1726,13 +1726,64 @@ static void parse_events_print_error(struct parse_events_error *err,
 
 #undef MAX_WIDTH
 
+static int parse_sdt_event(const char *str, struct list_head **sdt_list)
+{
+	char *ptr = NULL;
+	int ret;
+	struct list_head *sdt_evlist;
+
+	ptr = strdup(str);
+	if (ptr == NULL)
+		return -ENOMEM;
+
+	sdt_evlist = zalloc(sizeof(*sdt_evlist));
+	if (!sdt_evlist) {
+		free(ptr);
+		pr_err("Error in sdt_evlist memory allocation\n");
+		return -ENOMEM;
+	}
+	INIT_LIST_HEAD(sdt_evlist);
+
+	/*
+	 * If there is an error in this call, no need to free
+	 * up sdt_evlist, its already free'ed up in the previous
+	 * call. Free up 'ptr' though.
+	 */
+	ret = add_sdt_event(ptr, sdt_evlist);
+
+	free(ptr);
+	if (!ret)
+		*sdt_list = sdt_evlist;
+
+	return ret;
+}
+
 int parse_events_option(const struct option *opt, const char *str,
 			int unset __maybe_unused)
 {
 	struct perf_evlist *evlist = *(struct perf_evlist **)opt->value;
 	struct parse_events_error err = { .idx = 0, };
-	int ret = parse_events(evlist, str, &err);
+	int ret = 0;
+	struct list_head *sdt_list = NULL;
+	struct sdt_event_list *event;
+
+	if (*str == '%' && is_cmd_record()) {
+		ret = parse_sdt_event(str, &sdt_list);
+		if (!ret) {
+			list_for_each_entry(event, sdt_list, list) {
+				ret = parse_events(evlist, event->event_info,
+						   &err);
+				if (ret < 0)
+					goto error;
+			}
+			/* Add it to the record struct */
+			sdt_event_list__add(sdt_list);
+		}
+	} else {
+		ret = parse_events(evlist, str, &err);
+	}
 
+error:
 	if (ret)
 		parse_events_print_error(&err, str);
 
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index da246a3..7dbeb7a 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -194,4 +194,5 @@ int is_valid_tracepoint(const char *event_string);
 int valid_event_mount(const char *eventfs);
 char *parse_events_formats_error_string(char *additional_terms);
 
+void sdt_event_list__add(struct list_head *sdt_event_list);
 #endif /* __PERF_PARSE_EVENTS_H */
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 2c1bca2..5bd5568 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1293,7 +1293,7 @@ int parse_line_range_desc(const char *arg, struct line_range *lr)
 	return err;
 }
 
-static int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev)
+int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev)
 {
 	char *ptr;
 
@@ -1347,7 +1347,8 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 	if (arg[0] == '%' ||
 	    (!strncmp(arg, "sdt_", 4) &&
 	     !!strchr(arg, ':') && !strchr(arg, '='))) {
-		pev->sdt = true;
+		pr_err("SDT events don't need to be put in place using 'perf probe' anymore.\nYou can directly record on SDT events using 'perf record'\n");
+		return -EINVAL;
 		if (arg[0] == '%')
 			arg++;
 	}
@@ -3173,6 +3174,12 @@ static int find_cached_events_all(struct perf_probe_event *pev,
 	return ret;
 }
 
+int find_sdt_events_from_cache(struct perf_probe_event *pev,
+			       struct probe_trace_event **tevs)
+{
+	return find_cached_events_all(pev, tevs);
+}
+
 static int find_probe_trace_events_from_cache(struct perf_probe_event *pev,
 					      struct probe_trace_event **tevs)
 {
@@ -3483,3 +3490,35 @@ int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
 		tvar->name = NULL;
 	return 0;
 }
+
+/*
+ * Record session for SDT events has ended. Delete the SDT events
+ * from uprobe_events file that were created initially.
+ */
+void remove_sdt_event_list(struct list_head *sdt_events)
+{
+	struct sdt_event_list *event;
+	struct strfilter *filter = NULL;
+	const char *err = NULL;
+	int ret = 0;
+
+	if (list_empty(sdt_events))
+		return;
+
+	list_for_each_entry(event, sdt_events, list) {
+		if (!filter) {
+			filter = strfilter__new(event->event_info, &err);
+			if (!filter)
+				goto free_list;
+		} else {
+			ret = strfilter__or(filter, event->event_info, &err);
+		}
+	}
+
+	ret = del_perf_probe_events(filter);
+	if (ret)
+		pr_err("Error in deleting the SDT list\n");
+
+free_list:
+	free_sdt_list(sdt_events);
+}
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 5d4e940..5ec648e 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -182,4 +182,8 @@ struct map *get_target_map(const char *target, bool user);
 void arch__post_process_probe_trace_events(struct perf_probe_event *pev,
 					   int ntevs);
 
+int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev);
+
+int find_sdt_events_from_cache(struct perf_probe_event *pev,
+			       struct probe_trace_event **tevs);
 #endif /*_PROBE_EVENT_H */
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 436b647..2a6ad02 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -27,8 +27,10 @@
 #include "probe-event.h"
 #include "probe-file.h"
 #include "session.h"
+#include "probe-finder.h"
 
 #define MAX_CMDLEN 256
+#define MAX_EVENT_LENGTH 512
 
 static void print_open_warning(int err, bool uprobe)
 {
@@ -933,3 +935,98 @@ bool probe_type_is_available(enum probe_type type)
 
 	return ret;
 }
+
+void free_sdt_list(struct list_head *sdt_events)
+{
+	struct sdt_event_list *tmp, *ptr;
+
+	if (list_empty(sdt_events))
+		return;
+	list_for_each_entry_safe(tmp, ptr, sdt_events, list) {
+		list_del(&tmp->list);
+		free(tmp->event_info);
+		free(tmp);
+	}
+}
+
+/*
+ * Find the SDT event from the cache and if found add it/them
+ * to the uprobe_events file
+ */
+int add_sdt_event(char *event, struct list_head *sdt_events)
+{
+	struct perf_probe_event *pev;
+	int ret, i;
+	char *str = event + 1;
+	struct sdt_event_list *tmp;
+
+	pev = zalloc(sizeof(*pev));
+	if (!pev)
+		return -ENOMEM;
+
+	pev->sdt = true;
+	pev->uprobes = true;
+
+	/*
+	 * Parse str to find the group name and event name of
+	 * the sdt event.
+	 */
+	ret = parse_perf_probe_event_name(&str, pev);
+	if (ret) {
+		pr_err("Error in parsing sdt event %s\n", str);
+		free(pev);
+		return ret;
+	}
+
+	probe_conf.max_probes = MAX_PROBES;
+	probe_conf.force_add = 1;
+
+	/*
+	 * Find the sdt event from the cache, only cached SDT
+	 * events can be directly recorded.
+	 */
+	pev->ntevs = find_sdt_events_from_cache(pev, &pev->tevs);
+	if (pev->ntevs) {
+		if (pev->ntevs > 1) {
+			pr_warning("Warning : Recording on %d occurences of %s:%s\n",
+				   pev->ntevs, pev->group, pev->event);
+		}
+		ret = apply_perf_probe_events(pev, 1);
+		if (ret) {
+			pr_err("Error in adding SDT event : %s\n", event);
+			goto free_pev;
+		}
+	} else {
+		pr_err(" %s:%s not found in the cache\n", pev->group,
+			pev->event);
+		ret = -EINVAL;
+		goto free_pev;
+	}
+
+	/* Add the event name to "sdt_events" list */
+	for (i = 0; i < pev->ntevs; i++) {
+		tmp = zalloc(sizeof(*tmp));
+		if (!tmp) {
+			ret = -ENOMEM;
+			goto free_pev;
+		}
+
+		INIT_LIST_HEAD(&tmp->list);
+		tmp->event_info = zalloc(MAX_EVENT_LENGTH * sizeof(char));
+		if (!tmp->event_info) {
+			free_sdt_list(sdt_events);
+			ret = -ENOMEM;
+			goto free_pev;
+		}
+		snprintf(tmp->event_info, strlen(pev->tevs[i].group) +
+			 strlen(pev->tevs[i].event) + 2, "%s:%s",
+			 pev->tevs[i].group, pev->tevs[i].event);
+		list_add(&tmp->list, sdt_events);
+	}
+
+	ret = 0;
+
+free_pev:
+	cleanup_perf_probe_events(pev, 1);
+	return ret;
+}
diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
index eba44c3..3b39681 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h
@@ -19,6 +19,11 @@ struct probe_cache {
 	struct list_head entries;
 };
 
+struct sdt_event_list {
+	char *event_info;
+	struct list_head list;
+};
+
 enum probe_type {
 	PROBE_TYPE_U = 0,
 	PROBE_TYPE_S,
@@ -64,6 +69,9 @@ struct probe_cache_entry *probe_cache__find_by_name(struct probe_cache *pcache,
 					const char *group, const char *event);
 int probe_cache__show_all_caches(struct strfilter *filter);
 bool probe_type_is_available(enum probe_type type);
+int add_sdt_event(char *event, struct list_head *sdt_event_list);
+void remove_sdt_event_list(struct list_head *sdt_event_list);
+void free_sdt_list(struct list_head *sdt_events);
 #else	/* ! HAVE_LIBELF_SUPPORT */
 static inline struct probe_cache *probe_cache__new(const char *tgt __maybe_unused)
 {
-- 
2.9.3

Powered by blists - more mailing lists