Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1593477 > unrolled thread

[PATCH v4 0/7] perf/sdt: Directly record SDT events with 'perf record'

Back to article view | Back to linux.kernel

Contents

  [PATCH v4 0/7] perf/sdt: Directly record SDT events with 'perf record' Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2017-03-06 16:30 +0100
    [PATCH v4 4/7] perf/sdt: Clean uprobe_events when event(out of multiple events) parsing fails Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2017-03-06 17:40 +0100
    [PATCH v4 1/7] perf/sdt: Introduce util func is_sdt_event() Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2017-03-06 18:50 +0100
    [PATCH v4 5/7] perf/sdt: Warn when number of events recorded are not equal to cached events Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2017-03-06 19:40 +0100

#1593477 — [PATCH v4 0/7] perf/sdt: Directly record SDT events 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 point automatically with 'perf record' for SDT events.

Features:
  - Allow both 'perf probe' and 'perf record' on sdt events without
    changing current functionality.

  - Event starting with 'sdt_' or '%' will be considered as SDT event.

  - Always prioritize events from uprobe_events by first checking if
    event exists with exact name. If not found and user has used
    pattern, again try to find pattern matching entries from
    uprobe_events. If found use them. If not, lookup into probe-cache.
    If events found from probe-cache, again check if any event exists
    in uprobe_events by matching filepath+address, as it might exists
    in uprobe_events but with different name. Reuse those events which
    exists in uprobe_events and create new entries for missing one.
    Also maintain list for new entries being created and at the end
    of the session, delete them.

  - Show various warnings/hints to help user understand _which_ events
    are being recorded and _why_. For ex,

    When multiple events of same name found and all are being recorded:

      $ sudo ./perf record -a -e sdt_libpthread:mutex_entry
        Warning: Recording on 2 occurrences of sdt_libpthread:mutex_entry

    Events being reused from uprobe_events is listed as 'name addr@file'
    followed by hint on how to delete them:

      $ sudo ./perf record -a -e sdt_libpthread:mutex_entry
        Matching event(s) from uprobe_events:
          sdt_libpthread:mutex_entry  0x9ddb@/usr/lib64/libpthread-2.24.so
        Use 'perf probe -d <event>' to delete event(s).

    If number of events found from cache is not equal to number of events
    being recorded:

      $ sudo ./perf record -a -e sdt_libpthread:mutex_entry
        Warning: Found 2 events from probe-cache with name 'sdt_libpthread:mutex_entry'.
                 Since 1 probe point already exists, recording only it.
        Hint: Please use 'perf probe -d sdt_libpthread:mutex_entry' to allow record on all events.

Changes in v4:
  - Moved util function is_sdt_event() from tools/perf/util/util.c to
    tools/perf/util/parse-events.h. [PATCH 1/7]

  - Split hunk into multiple patches.

  - Changed pr_err() to pr_debug() (only where Masami has suggested).

  - Removed unnecessary wrapper func find_sdt_events_from_cache() by
    exposing find_cached_events_all().

  - Removed 'struct exst_sdt_event_list', instead using array of
    existing data structure 'struct probe_trace_event'.

  - No hint will be shown at 'perf probe' for SDT events.

  - Functionality change. If all events found from probe-cache are not
    present in uprobe_events, v3 was recording all events found from
    cache. That has been changed in v4. If user has used pattern to
    specify event, v4 will record only those events which are present
    in uprobe_events. This is to make perf semantics consistent across
    normal and SDT events. And If user has not used pattern, it will
    record all events found from probe-cache by reusing name for
    existing one and adding entries for missing one. For ex (with v4),
    
      $ sudo ./perf probe sdt_libpthread:mutex_release
        Added new events:
          sdt_libpthread:mutex_release (on %mutex_release in /usr/lib64/libpthread-2.24.so)
          sdt_libpthread:mutex_release_1 (on %mutex_release in /usr/lib64/libpthread-2.24.so)
          sdt_libpthread:mutex_release_2 (on %mutex_release in /usr/lib64/libpthread-2.24.so)
          sdt_libpthread:mutex_release_3 (on %mutex_release in /usr/lib64/libpthread-2.24.so)
      $ sudo ./perf probe -d sdt_libpthread:mutex_release
      $ sudo ./perf probe -d sdt_libpthread:mutex_release_2

      $ sudo ./perf record -a -e sdt_libpthread:mutex_release*
        Warning: Recording on 2 occurrences of sdt_libpthread:mutex_release*

      $ sudo ./perf record -a -e sdt_libpthread:mutex_release
        Warning: Recording on 4 occurrences of sdt_libpthread:mutex_release


This patchset is prepared on top of acme/perf/core.

v3 link: https://lkml.org/lkml/2017/2/24/27

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


Hemant Kumar (1):
  perf/sdt: Directly record SDT events with 'perf record'

Ravi Bangoria (6):
  perf/sdt: Introduce util func is_sdt_event()
  perf/sdt: Allow recording of existing events
  perf/sdt: Clean uprobe_events when event(out of multiple events)
    parsing fails
  perf/sdt: Warn when number of events recorded are not equal to cached
    events
  perf/sdt: List events fetched from uprobe_events
  perf/sdt: Remove stale warning

 tools/lib/api/fs/tracing_path.c |  17 +--
 tools/perf/builtin-record.c     |  23 +++
 tools/perf/perf.h               |   2 +
 tools/perf/util/parse-events.c  |  56 +++++++-
 tools/perf/util/parse-events.h  |  14 ++
 tools/perf/util/probe-event.c   | 100 +++++++++++--
 tools/perf/util/probe-event.h   |   9 ++
 tools/perf/util/probe-file.c    | 310 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/probe-file.h    |   9 ++
 9 files changed, 513 insertions(+), 27 deletions(-)

-- 
2.9.3

[toc] | [next] | [standalone]

#1593517 — [PATCH v4 4/7] perf/sdt: Clean uprobe_events when event(out of multiple events) parsing fails

User may ask for multiple events in the same record command like,

  perf record -a -e sdt_1:* -e sdt_2:*

If sdt_1:* events are already added to uprobe_events and sdt_2:*
event parsing fails, clean sdt_1:* events from uprobe_events.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 tools/perf/builtin-record.c  | 7 +++----
 tools/perf/perf.h            | 1 +
 tools/perf/util/probe-file.c | 4 +++-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index e87b19b..46d447e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1517,11 +1517,10 @@ bool is_cmd_record(void)
 	return (record.evlist != NULL);
 }
 
-static void
-sdt_event_list__remove(struct list_head *sdt_event_list __maybe_unused)
+void sdt_event_list__remove(void)
 {
 #ifdef HAVE_LIBELF_SUPPORT
-	return remove_sdt_event_list(sdt_event_list);
+	return remove_sdt_event_list(&record.sdt_event_list);
 #endif
 }
 
@@ -1864,7 +1863,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);
+	sdt_event_list__remove();
 	return err;
 }
 
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 9d8e5fe..8a411f1 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -77,4 +77,5 @@ struct option;
 extern const char * const *record_usage;
 extern struct option *record_options;
 bool is_cmd_record(void);
+void sdt_event_list__remove(void);
 #endif
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 47b624a..358ca98 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -1192,8 +1192,10 @@ int add_sdt_event(char *event, struct list_head *sdt_evlist)
 	ret = 0;
 
 free_pev:
-	if (ret < 0)
+	if (ret < 0) {
 		free_sdt_list(sdt_evlist);
+		sdt_event_list__remove();
+	}
 	cleanup_perf_probe_events(pev, 1);
 	clear_probe_trace_events(exst_tevs, exst_ntevs);
 	free(pev);
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1593548 — [PATCH v4 1/7] perf/sdt: Introduce util func is_sdt_event()

Factor out the SDT event name checking routine as is_sdt_event().

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 tools/perf/util/parse-events.h | 12 ++++++++++++
 tools/perf/util/probe-event.c  |  9 +--------
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 1af6a26..c6172cd 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -8,6 +8,7 @@
 #include <stdbool.h>
 #include <linux/types.h>
 #include <linux/perf_event.h>
+#include <string.h>
 
 struct list_head;
 struct perf_evsel;
@@ -196,4 +197,15 @@ int is_valid_tracepoint(const char *event_string);
 int valid_event_mount(const char *eventfs);
 char *parse_events_formats_error_string(char *additional_terms);
 
+/*
+ * If the probe point starts with '%',
+ * or starts with "sdt_" and has a ':' but no '=',
+ * then it should be a SDT/cached probe point.
+ */
+static inline bool is_sdt_event(char *str)
+{
+	return (str[0] == '%' ||
+		(!strncmp(str, "sdt_", 4) &&
+		 !!strchr(str, ':') && !strchr(str, '=')));
+}
 #endif /* __PERF_PARSE_EVENTS_H */
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 28fb62c..2b1409f 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1339,14 +1339,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 	if (!arg)
 		return -EINVAL;
 
-	/*
-	 * If the probe point starts with '%',
-	 * or starts with "sdt_" and has a ':' but no '=',
-	 * then it should be a SDT/cached probe point.
-	 */
-	if (arg[0] == '%' ||
-	    (!strncmp(arg, "sdt_", 4) &&
-	     !!strchr(arg, ':') && !strchr(arg, '='))) {
+	if (is_sdt_event(arg)) {
 		pev->sdt = true;
 		if (arg[0] == '%')
 			arg++;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1593579 — [PATCH v4 5/7] perf/sdt: Warn when number of events recorded are not equal to cached events

If number of events found from probe-cache is not equal to number of
existing events(fetched from uprobe_events), and somehow we decides
to record only existing events, we warn user about the same. For ex,

  $ sudo ./perf probe sdt_libpthread:mutex_release
    Added new events:
      sdt_libpthread:mutex_release (on %mutex_release in /usr/lib64/libpthread-2.24.so)
      sdt_libpthread:mutex_release_1 (on %mutex_release in /usr/lib64/libpthread-2.24.so)
      sdt_libpthread:mutex_release_2 (on %mutex_release in /usr/lib64/libpthread-2.24.so)
      sdt_libpthread:mutex_release_3 (on %mutex_release in /usr/lib64/libpthread-2.24.so)

  $ sudo ./perf record -a -e sdt_libpthread:*
    Warning: Recording on 4 occurrences of sdt_libpthread:*
    Warning: Found 35 events from probe-cache with name 'sdt_libpthread:*'.
             Since 4 probe points already exists, recording only them.
    Hint: Please use 'perf probe -d sdt_libpthread:*' to allow record on all events.

  $ sudo ./perf evlist
    sdt_libpthread:mutex_release_3
    sdt_libpthread:mutex_release_2
    sdt_libpthread:mutex_release_1
    sdt_libpthread:mutex_release

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 tools/perf/util/probe-file.c | 59 ++++++++++++++++++++++++++++----------------
 1 file changed, 38 insertions(+), 21 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 358ca98..90444e5 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -951,24 +951,6 @@ void free_sdt_list(struct list_head *sdt_evlist)
 	}
 }
 
-static int get_sdt_events_from_cache(struct perf_probe_event *pev)
-{
-	int ret = 0;
-
-	pev->ntevs = find_cached_events_all(pev, &pev->tevs);
-
-	if (pev->ntevs < 0) {
-		pr_err("Error: Cache lookup failed (code: %d)\n", pev->ntevs);
-		ret = pev->ntevs;
-	} else if (!pev->ntevs) {
-		pr_err("Error: %s:%s not found in the cache\n",
-			pev->group, pev->event);
-		ret = -EINVAL;
-	}
-
-	return ret;
-}
-
 static int add_event_to_sdt_evlist(struct probe_trace_event *tev,
 				   struct list_head *sdt_evlist,
 				   bool exst)
@@ -1076,6 +1058,17 @@ static void shift_sdt_events(struct perf_probe_event *pev, int i)
 	pev->ntevs--;
 }
 
+static void sdt_warn_abt_exist_events(struct perf_probe_event *pev, int ctr)
+{
+	pr_warning("Warning: Found %d events from probe-cache with name '%s:%s'.\n"
+		"\t Since %d probe point%c already exists, recording only %s.\n"
+		"Hint: Please use 'perf probe -d %s:%s' to allow record on all events.\n\n",
+		pev->ntevs, pev->group, pev->event, ctr,
+		ctr > 1 ? 's' : '\0',
+		ctr > 1 ? "them" : "it",
+		pev->group, pev->event);
+}
+
 static int sdt_merge_events(struct perf_probe_event *pev,
 			    struct probe_trace_event *exst_tevs,
 			    int exst_ntevs,
@@ -1155,6 +1148,15 @@ int add_sdt_event(char *event, struct list_head *sdt_evlist)
 	probe_conf.max_probes = MAX_PROBES;
 	probe_conf.force_add = 1;
 
+	/*
+	 * This call is intentionally placed before fetching events
+	 * from uprobe_events file. If number of events found from probe-
+	 * cache is not equal to number of existing events, and somehow
+	 * we decides to record only existing events, we warn user about
+	 * the same (sdt_warn_abt_exist_events()).
+	 */
+	pev->ntevs = find_cached_events_all(pev, &pev->tevs);
+
 	/* Fetch all sdt events from uprobe_events */
 	exst_ntevs = get_exist_sdt_events(&exst_tevs);
 	if (exst_ntevs < 0) {
@@ -1166,14 +1168,29 @@ int add_sdt_event(char *event, struct list_head *sdt_evlist)
 	ret = sdt_event_probepoint_exists(pev, exst_tevs,
 					 exst_ntevs, sdt_evlist);
 	if (ret) {
+		if (ret > 0 && pev->ntevs > 0 && ret != pev->ntevs)
+			sdt_warn_abt_exist_events(pev, ret);
 		ret = ret > 0 ? 0 : ret;
 		goto free_pev;
 	}
 
-	/* Fetch all matching events from cache. */
-	ret = get_sdt_events_from_cache(pev);
-	if (ret < 0)
+	/*
+	 * Check if find_cached_events_all() failed.
+	 * We deliberately check failure of this function after checking
+	 * entries in uprobe_events. Because, even if this function fails,
+	 * we may find matching entry from uprobe_events and in that case
+	 * we should continue recording that event.
+	 */
+	if (pev->ntevs < 0) {
+		pr_err("Error: Cache lookup failed (code: %d)\n", pev->ntevs);
+		ret = pev->ntevs;
 		goto free_pev;
+	} else if (!pev->ntevs) {
+		pr_err("Error: %s:%s not found in the cache\n",
+			pev->group, pev->event);
+		ret = -EINVAL;
+		goto free_pev;
+	}
 
 	/*
 	 * Merge events found from uprobe_events with events found
-- 
2.9.3

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web