Skip to contentSkip to conversation

perf/core improvements and fixes

GIT PULL New Arnaldo Carvalho de Melo 1 participant 11 messages Jun 16, 2015 perf events

lore
Share & tools
lore

Arnaldo Carvalho de Melo

author maintainer
#0 lore raw

Hi Ingo,

Please consider pulling,

- Arnaldo

The following changes since commit 61d67d568445413137995e1bea2746783e3a81e9:

Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2015-06-12 10:08:38 +0200)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-core-for-mingo

for you to fetch changes up to b031220d520238075bd99513a420e65cf37866ad:

perf probe: Fix to return error if no probe is added (2015-06-16 11:39:51 -0300)

----------------------------------------------------------------
perf/core improvements and fixes:

User visible:

- List perf probes to stdout (Masami Hiramatsu)

- Return error when none of the requested probes were installed (Masami Hiramatsu)

- Cut off the gcc optimization postfixes from function name in 'perf probe' (Masami Hiramatsu)

Infrastructure:

- Replace map->referenced & maps->removed_maps with map->refcnt (Arnaldo Carvalho de Melo)

- Introduce xyarray__reset function (Jiri Olsa)

- Add thread_map__(alloc|realloc) helpers (Jiri Olsa)

- Move perf_evsel__(alloc|free|reset)_counts into stat object (Jiri Olsa)

- Introduce perf_counts__(new|delete|reset) functions (Jiri Olsa)

Build Fixes:

- Fix a libunwind related compile error (Hou Pengyang)

- Fix python/perf.so compiling error related to libtraceevent (Wang Nan)

----------------------------------------------------------------
Arnaldo Carvalho de Melo (1):

      perf tools: Replace map->referenced & maps->removed_maps with map->refcnt

Hou Pengyang (1):
perf unwind: Fix a compile error

Jiri Olsa (4):
perf tools: Introduce xyarray__reset function
perf tools: Add thread_map__(alloc|realloc) helpers
perf tools: Move perf_evsel__(alloc|free|reset)_counts into stat object perf stat: Introduce perf_counts__(new|delete|reset) functions

Masami Hiramatsu (3):
perf probe: Cut off the gcc optimization postfixes from function name perf probe: List probes in stdout
perf probe: Fix to return error if no probe is added

Wang Nan (1):
tools lib traceevent: Fix python/perf.so compiling error

 tools/perf/Makefile.perf                   |   2 +-
 tools/perf/builtin-stat.c                  |  19 ++--
 tools/perf/tests/openat-syscall-all-cpus.c |   1 +
 tools/perf/util/evsel.c                    |  19 +---
 tools/perf/util/evsel.h                    |   3 -
 tools/perf/util/hist.c                     |  37 ++++---
 tools/perf/util/map.c                      |  58 +---------
 tools/perf/util/map.h                      |  10 +-
 tools/perf/util/probe-event.c              | 167 +++++++++++++++++++----------
 tools/perf/util/python-ext-sources         |   1 +
 tools/perf/util/stat.c                     |  36 +++++++
 tools/perf/util/stat.h                     |   6 ++
 tools/perf/util/thread_map.c               |  24 +++--
 tools/perf/util/unwind-libunwind.c         |   2 +-
 tools/perf/util/xyarray.c                  |   8 ++
 tools/perf/util/xyarray.h                  |   2 +
 16 files changed, 223 insertions(+), 172 deletions(-)
author maintainer
#1 lore raw

PATCH 1/10 perf probe: Cut off the gcc optimization postfixes from function name

From: Masami Hiramatsu <[email protected]>

Cut off the postfixes which gcc added for optimized routines from the event name automatically generated from symbol name, since *probe-events doesn't accept it. Those symbols will be used if we don't use debuginfo to find target functions.

E.g. without this fix;
-----

  # perf probe -va alloc_buf.isra.23
  probe-definition(0): alloc_buf.isra.23
  symbol:alloc_buf.isra.23 file:(null) line:0 offset:0 return:0 lazy:(null)
  [...]
  Opening /sys/kernel/debug/tracing/kprobe_events write=1
  Added new event:
  Writing event: p:probe/alloc_buf.isra.23 _text+4869328
  Failed to write event: Invalid argument
    Error: Failed to add events. Reason: Invalid argument (Code: -22)
  -----
With this fix;
  -----
  perf probe -va alloc_buf.isra.23
  probe-definition(0): alloc_buf.isra.23
  symbol:alloc_buf.isra.23 file:(null) line:0 offset:0 return:0 lazy:(null)
  [...]
  Opening /sys/kernel/debug/tracing/kprobe_events write=1
  Added new event:
  Writing event: p:probe/alloc_buf _text+4869328
    probe:alloc_buf      (on alloc_buf.isra.23)

You can now use it in all perf tools, such as:

perf record -e probe:alloc_buf -aR sleep 1

-----

---
 tools/perf/util/probe-event.c | 5 +++++
 1 file changed, 5 insertions(+)
tools/perf/util/probe-event.c +5 -0
index d4cf50b91839..daa24a249e05 100644
@@ -2316,6 +2316,7 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
 			      struct strlist *namelist, bool allow_suffix)
 {
 	int i, ret;
+	char *p;
 
 	if (*base == '.')
 		base++;
@@ -2326,6 +2327,10 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
 		pr_debug("snprintf() failed: %d\n", ret);
 		return ret;
 	}
+	/* Cut off the postfixes (e.g. .const, .isra)*/
+	p = strchr(buf, '.');
+	if (p && p != buf)
+		*p = '\0';
 	if (!strlist__has_entry(namelist, buf))
 		return 0;
 
Signature · 1 line
2.1.0
author maintainer
#2 lore raw

PATCH 2/10 perf tools: Replace map->referenced & maps->removed_maps with map->refcnt

In reply to Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <[email protected]>

Use just reference counts, so that when no more hist_entry instances references a map and the thread instance goes away by processing a PERF_RECORD_EXIT, we can delete the maps.

Cc Adrian Hunter <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc David Ahern <[email protected]>
Cc Don Zickus <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/hist.c | 37 +++++++++++++++++++-------------
 tools/perf/util/map.c  | 58 ++------------------------------------------------
 tools/perf/util/map.h  | 10 +++++++--
 3 files changed, 32 insertions(+), 73 deletions(-)
tools/perf/util/hist.c +22 -15
index f53d017c7c22..6f28d53d4e46 100644
@@ -313,8 +313,7 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template,
 				memset(&he->stat, 0, sizeof(he->stat));
 		}
 
-		if (he->ms.map)
-			he->ms.map->referenced = true;
+		map__get(he->ms.map);
 
 		if (he->branch_info) {
 			/*
@@ -324,6 +323,7 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template,
 			 */
 			he->branch_info = malloc(sizeof(*he->branch_info));
 			if (he->branch_info == NULL) {
+				map__zput(he->ms.map);
 				free(he->stat_acc);
 				free(he);
 				return NULL;
@@ -332,17 +332,13 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template,
 			memcpy(he->branch_info, template->branch_info,
 			       sizeof(*he->branch_info));
 
-			if (he->branch_info->from.map)
-				he->branch_info->from.map->referenced = true;
-			if (he->branch_info->to.map)
-				he->branch_info->to.map->referenced = true;
+			map__get(he->branch_info->from.map);
+			map__get(he->branch_info->to.map);
 		}
 
 		if (he->mem_info) {
-			if (he->mem_info->iaddr.map)
-				he->mem_info->iaddr.map->referenced = true;
-			if (he->mem_info->daddr.map)
-				he->mem_info->daddr.map->referenced = true;
+			map__get(he->mem_info->iaddr.map);
+			map__get(he->mem_info->daddr.map);
 		}
 
 		if (symbol_conf.use_callchain)
@@ -407,9 +403,8 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists,
 			 * the history counter to increment.
 			 */
 			if (he->ms.map != entry->ms.map) {
-				he->ms.map = entry->ms.map;
-				if (he->ms.map)
-					he->ms.map->referenced = true;
+				map__put(he->ms.map);
+				he->ms.map = map__get(entry->ms.map);
 			}
 			goto out;
 		}
@@ -933,8 +928,20 @@ hist_entry__collapse(struct hist_entry *left, struct hist_entry *right)
 void hist_entry__delete(struct hist_entry *he)
 {
 	thread__zput(he->thread);
-	zfree(&he->branch_info);
-	zfree(&he->mem_info);
+	map__zput(he->ms.map);
+
+	if (he->branch_info) {
+		map__zput(he->branch_info->from.map);
+		map__zput(he->branch_info->to.map);
+		zfree(&he->branch_info);
+	}
+
+	if (he->mem_info) {
+		map__zput(he->mem_info->iaddr.map);
+		map__zput(he->mem_info->daddr.map);
+		zfree(&he->mem_info);
+	}
+
 	zfree(&he->stat_acc);
 	free_srcline(he->srcline);
 	free_callchain(he->callchain);
tools/perf/util/map.c +2 -56

104 lines not on this page · raw message

tools/perf/util/map.h +8 -2
index b8df09d94aca..d73e687b224e 100644
@@ -34,7 +34,6 @@ struct map {
 	u64			start;
 	u64			end;
 	u8 /* enum map_type */	type;
-	bool			referenced;
 	bool			erange_warned;
 	u32			priv;
 	u32			prot;
@@ -63,7 +62,6 @@ struct kmap {
 struct maps {
 	struct rb_root	 entries;
 	pthread_rwlock_t lock;
-	struct list_head removed_maps;
 };
 
 struct map_groups {
@@ -161,6 +159,14 @@ static inline struct map *map__get(struct map *map)
 
 void map__put(struct map *map);
 
+static inline void __map__zput(struct map **map)
+{
+	map__put(*map);
+	*map = NULL;
+}
+
+#define map__zput(map) __map__zput(&map)
+
 int map__overlap(struct map *l, struct map *r);
 size_t map__fprintf(struct map *map, FILE *fp);
 size_t map__fprintf_dsoname(struct map *map, FILE *fp);
Signature · 1 line
2.1.0
author maintainer
#3 lore raw

PATCH 3/10 tools lib traceevent: Fix python/perf.so compiling error

In reply to Arnaldo Carvalho de Melo

From: Wang Nan <[email protected]>

'make build-test' finds an error that make_python_perf_so fails due to missing of libtraceevent-dynamic-list:

'.../python2' util/setup.py \
--quiet build_ext; \
mkdir -p python && \
cp python_ext_build/lib/perf.so python/
/path/to/ld: cannot open linker script file /path/to/kernel/tools/lib/traceevent/libtraceevent-dynamic-list: No such file or directory
collect2: error: ld returned 1 exit status
error: command 'x86_64-linux-gcc' failed with exit status 1
cp: cannot stat 'python_ext_build/lib/perf.so': No such file or directory
make[3]: *** [python/perf.so] Error 1
make[2]: *** [python/perf.so] Error 2
test: test -f ./python/perf.so
make[1]: *** [make_python_perf_so] Error 1
make: *** [build-test] Error 2
make: Leaving directory `/path/to/kernel/tools/perf'

This is caused by commit e3d09ec8126fe2c9a3ade661e2126e215ca27a80 ("tools lib traceevent: Export dynamic symbols used by traceevent plugins") that, it adds the list file to LDFLAGS but forgot to add it to dependency list of python/perf.so.

This patch fixes this problem.

---
 tools/perf/Makefile.perf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/Makefile.perf +1 -1
index b1dfcd8e93e3..374378322db9 100644
@@ -190,7 +190,7 @@ python-clean := $(call QUIET_CLEAN, python) $(RM) -r $(PYTHON_EXTBUILD) $(OUTPUT
 PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources)
 PYTHON_EXT_DEPS := util/python-ext-sources util/setup.py $(LIBTRACEEVENT) $(LIBAPI)
 
-$(OUTPUT)python/perf.so: $(PYTHON_EXT_SRCS) $(PYTHON_EXT_DEPS)
+$(OUTPUT)python/perf.so: $(PYTHON_EXT_SRCS) $(PYTHON_EXT_DEPS) $(LIBTRACEEVENT_DYNAMIC_LIST)
 	$(QUIET_GEN)CFLAGS='$(CFLAGS)' $(PYTHON_WORD) util/setup.py \
 	  --quiet build_ext; \
 	mkdir -p $(OUTPUT)python && \
Signature · 1 line
2.1.0
author maintainer
#4 lore raw

PATCH 4/10 perf probe: List probes in stdout

In reply to Arnaldo Carvalho de Melo

From: Masami Hiramatsu <[email protected]>

Since commit 5e17b28f1e24 ("perf probe: Add --quiet option to suppress output result message") have replaced printf with pr_info, perf probe -l outputs its result in stderr. However, that is not what the commit expected.

E.g.:

  # perf probe -l > /dev/null
    probe:vfs_read       (on vfs_read@ksrc/linux-3/fs/read_write.c)

With this fix:

  # perf probe -l > list
  # cat list
    probe:vfs_read       (on vfs_read@ksrc/linux-3/fs/read_write.c)

Of course, --quiet(-q) still works on --add/--del.

  # perf probe -q vfs_write
  # perf probe -l
    probe:vfs_read       (on vfs_read@ksrc/linux-3/fs/read_write.c)
    probe:vfs_write      (on vfs_write@ksrc/linux-3/fs/read_write.c)
  -----
---
 tools/perf/util/probe-event.c | 49 ++++++++++++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 14 deletions(-)
tools/perf/util/probe-event.c +35 -14
index daa24a249e05..c4ab58870fcc 100644
@@ -2126,9 +2126,9 @@ kprobe_blacklist__find_by_address(struct list_head *blacklist,
 	return NULL;
 }
 
-/* Show an event */
-static int show_perf_probe_event(struct perf_probe_event *pev,
-				 const char *module)
+static int perf_probe_event__sprintf(struct perf_probe_event *pev,
+				     const char *module,
+				     struct strbuf *result)
 {
 	int i, ret;
 	char buf[128];
@@ -2141,27 +2141,47 @@ static int show_perf_probe_event(struct perf_probe_event *pev,
 
 	ret = e_snprintf(buf, 128, "%s:%s", pev->group, pev->event);
 	if (ret < 0)
-		return ret;
+		goto out;
 
-	pr_info("  %-20s (on %s", buf, place);
+	strbuf_addf(result, "  %-20s (on %s", buf, place);
 	if (module)
-		pr_info(" in %s", module);
+		strbuf_addf(result, " in %s", module);
 
 	if (pev->nargs > 0) {
-		pr_info(" with");
+		strbuf_addstr(result, " with");
 		for (i = 0; i < pev->nargs; i++) {
 			ret = synthesize_perf_probe_arg(&pev->args[i],
 							buf, 128);
 			if (ret < 0)
-				break;
-			pr_info(" %s", buf);
+				goto out;
+			strbuf_addf(result, " %s", buf);
 		}
 	}
-	pr_info(")\n");
+	strbuf_addch(result, ')');
+out:
 	free(place);
 	return ret;
 }
 
+/* Show an event */
+static int show_perf_probe_event(struct perf_probe_event *pev,
+				 const char *module, bool use_stdout)
+{
+	struct strbuf buf = STRBUF_INIT;
+	int ret;
+
+	ret = perf_probe_event__sprintf(pev, module, &buf);
+	if (ret >= 0) {
+		if (use_stdout)
+			printf("%s\n", buf.buf);
+		else
+			pr_info("%s\n", buf.buf);
+	}
+	strbuf_release(&buf);
+
+	return ret;
+}
+
 static bool filter_probe_trace_event(struct probe_trace_event *tev,
 				     struct strfilter *filter)
 {
@@ -2200,9 +2220,10 @@ static int __show_perf_probe_events(int fd, bool is_kprobe,
 				goto next;
 			ret = convert_to_perf_probe_event(&tev, &pev,
 								is_kprobe);
-			if (ret >= 0)
-				ret = show_perf_probe_event(&pev,
-							    tev.point.module);
+			if (ret < 0)
+				goto next;
+			ret = show_perf_probe_event(&pev, tev.point.module,
+						    true);
 		}
 next:
 		clear_perf_probe_event(&pev);
@@ -2468,7 +2489,7 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 		group = pev->group;
 		pev->event = tev->event;
 		pev->group = tev->group;
-		show_perf_probe_event(pev, tev->point.module);
+		show_perf_probe_event(pev, tev->point.module, false);
 		/* Trick here - restore current event/group */
 		pev->event = (char *)event;
 		pev->group = (char *)group;
Signature · 1 line
2.1.0
author maintainer
#5 lore raw

PATCH 5/10 perf tools: Introduce xyarray__reset function

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

To zero all the xyarray contents. It will be used in following patches.

---
 tools/perf/util/xyarray.c | 8 ++++++++
 tools/perf/util/xyarray.h | 2 ++
 2 files changed, 10 insertions(+)
tools/perf/util/xyarray.c +8 -0
index 22afbf6c536a..c10ba41ef3f6 100644
@@ -9,11 +9,19 @@ struct xyarray *xyarray__new(int xlen, int ylen, size_t entry_size)
 	if (xy != NULL) {
 		xy->entry_size = entry_size;
 		xy->row_size   = row_size;
+		xy->entries    = xlen * ylen;
 	}
 
 	return xy;
 }
 
+void xyarray__reset(struct xyarray *xy)
+{
+	size_t n = xy->entries * xy->entry_size;
+
+	memset(xy->contents, 0, n);
+}
+
 void xyarray__delete(struct xyarray *xy)
 {
 	free(xy);
tools/perf/util/xyarray.h +2 -0
index c488a07275dd..7f30af371b7e 100644
@@ -6,11 +6,13 @@
 struct xyarray {
 	size_t row_size;
 	size_t entry_size;
+	size_t entries;
 	char contents[];
 };
 
 struct xyarray *xyarray__new(int xlen, int ylen, size_t entry_size);
 void xyarray__delete(struct xyarray *xy);
+void xyarray__reset(struct xyarray *xy);
 
 static inline void *xyarray__entry(struct xyarray *xy, int x, int y)
 {
Signature · 1 line
2.1.0
author maintainer
#6 lore raw

PATCH 6/10 perf tools: Add thread_map__(alloc|realloc) helpers

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

In order to have 'struct thread_map' allocation on single place and can change it easily in following patch.

Using alloc|realloc for static helpers, because thread_map__new is already used in public interface.

---
 tools/perf/util/thread_map.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)
tools/perf/util/thread_map.c +16 -8
index f93b9734735b..f4822bd03709 100644
@@ -20,6 +20,15 @@ static int filter(const struct dirent *dir)
 		return 1;
 }
 
+static struct thread_map *thread_map__realloc(struct thread_map *map, int nr)
+{
+	size_t size = sizeof(*map) + sizeof(pid_t) * nr;
+
+	return realloc(map, size);
+}
+
+#define thread_map__alloc(__nr) thread_map__realloc(NULL, __nr)
+
 struct thread_map *thread_map__new_by_pid(pid_t pid)
 {
 	struct thread_map *threads;
@@ -33,7 +42,7 @@ struct thread_map *thread_map__new_by_pid(pid_t pid)
 	if (items <= 0)
 		return NULL;
 
-	threads = malloc(sizeof(*threads) + sizeof(pid_t) * items);
+	threads = thread_map__alloc(items);
 	if (threads != NULL) {
 		for (i = 0; i < items; i++)
 			threads->map[i] = atoi(namelist[i]->d_name);
@@ -49,7 +58,7 @@ struct thread_map *thread_map__new_by_pid(pid_t pid)
 
 struct thread_map *thread_map__new_by_tid(pid_t tid)
 {
-	struct thread_map *threads = malloc(sizeof(*threads) + sizeof(pid_t));
+	struct thread_map *threads = thread_map__alloc(1);
 
 	if (threads != NULL) {
 		threads->map[0] = tid;
@@ -65,8 +74,8 @@ struct thread_map *thread_map__new_by_uid(uid_t uid)
 	int max_threads = 32, items, i;
 	char path[256];
 	struct dirent dirent, *next, **namelist = NULL;
-	struct thread_map *threads = malloc(sizeof(*threads) +
-					    max_threads * sizeof(pid_t));
+	struct thread_map *threads = thread_map__alloc(max_threads);
+
 	if (threads == NULL)
 		goto out;
 
@@ -185,8 +194,7 @@ static struct thread_map *thread_map__new_by_pid_str(const char *pid_str)
 			goto out_free_threads;
 
 		total_tasks += items;
-		nt = realloc(threads, (sizeof(*threads) +
-				       sizeof(pid_t) * total_tasks));
+		nt = thread_map__realloc(threads, total_tasks);
 		if (nt == NULL)
 			goto out_free_namelist;
 
@@ -216,7 +224,7 @@ out_free_threads:
 
 struct thread_map *thread_map__new_dummy(void)
 {
-	struct thread_map *threads = malloc(sizeof(*threads) + sizeof(pid_t));
+	struct thread_map *threads = thread_map__alloc(1);
 
 	if (threads != NULL) {
 		threads->map[0]	= -1;
@@ -253,7 +261,7 @@ static struct thread_map *thread_map__new_by_tid_str(const char *tid_str)
 			continue;
 
 		ntasks++;
-		nt = realloc(threads, sizeof(*threads) + sizeof(pid_t) * ntasks);
+		nt = thread_map__realloc(threads, ntasks);
 
 		if (nt == NULL)
 			goto out_free_threads;
Signature · 1 line
2.1.0
author maintainer
#7 lore raw

PATCH 7/10 perf tools: Move perf_evsel__(alloc|free|reset)_counts into stat object

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

It's stat specific. Updating python build objects with stat.c.

---
 tools/perf/tests/openat-syscall-all-cpus.c |  1 +
 tools/perf/util/evsel.c                    | 19 +------------------
 tools/perf/util/evsel.h                    |  3 ---
 tools/perf/util/python-ext-sources         |  1 +
 tools/perf/util/stat.c                     | 18 ++++++++++++++++++
 tools/perf/util/stat.h                     |  3 +++
 6 files changed, 24 insertions(+), 21 deletions(-)
tools/perf/tests/openat-syscall-all-cpus.c +1 -0
index e34dfdf96b5a..9a7a116e09b8 100644
@@ -3,6 +3,7 @@
 #include "thread_map.h"
 #include "cpumap.h"
 #include "debug.h"
+#include "stat.h"
 
 int test__openat_syscall_event_on_all_cpus(void)
 {
tools/perf/util/evsel.c +1 -18
index d4f9994ae47f..33449decf7bd 100644
@@ -26,6 +26,7 @@
 #include "perf_regs.h"
 #include "debug.h"
 #include "trace-event.h"
+#include "stat.h"
 
 static struct {
 	bool sample_id_all;
@@ -851,19 +852,6 @@ int perf_evsel__alloc_id(struct perf_evsel *evsel, int ncpus, int nthreads)
 	return 0;
 }
 
-void perf_evsel__reset_counts(struct perf_evsel *evsel, int ncpus)
-{
-	memset(evsel->counts, 0, (sizeof(*evsel->counts) +
-				 (ncpus * sizeof(struct perf_counts_values))));
-}
-
-int perf_evsel__alloc_counts(struct perf_evsel *evsel, int ncpus)
-{
-	evsel->counts = zalloc((sizeof(*evsel->counts) +
-				(ncpus * sizeof(struct perf_counts_values))));
-	return evsel->counts != NULL ? 0 : -ENOMEM;
-}
-
 static void perf_evsel__free_fd(struct perf_evsel *evsel)
 {
 	xyarray__delete(evsel->fd);
@@ -891,11 +879,6 @@ void perf_evsel__close_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
 		}
 }
 
-void perf_evsel__free_counts(struct perf_evsel *evsel)
-{
-	zfree(&evsel->counts);
-}
-
 void perf_evsel__exit(struct perf_evsel *evsel)
 {
 	assert(list_empty(&evsel->node));
tools/perf/util/evsel.h +0 -3
index 21ec08247d47..bb0579e8a10a 100644
@@ -170,9 +170,6 @@ const char *perf_evsel__group_name(struct perf_evsel *evsel);
 int perf_evsel__group_desc(struct perf_evsel *evsel, char *buf, size_t size);
 
 int perf_evsel__alloc_id(struct perf_evsel *evsel, int ncpus, int nthreads);
-int perf_evsel__alloc_counts(struct perf_evsel *evsel, int ncpus);
-void perf_evsel__reset_counts(struct perf_evsel *evsel, int ncpus);
-void perf_evsel__free_counts(struct perf_evsel *evsel);
 void perf_evsel__close_fd(struct perf_evsel *evsel, int ncpus, int nthreads);
 
 void __perf_evsel__set_sample_bit(struct perf_evsel *evsel,
tools/perf/util/python-ext-sources +1 -0
index 4d28624a1eca..5925fec90562 100644
@@ -16,6 +16,7 @@ util/util.c
 util/xyarray.c
 util/cgroup.c
 util/rblist.c
+util/stat.c
 util/strlist.c
 util/trace-event.c
 ../../lib/rbtree.c
tools/perf/util/stat.c +18 -0
index 60b92822f655..ac589b6b8bce 100644
@@ -94,3 +94,21 @@ void perf_stat_evsel_id_init(struct perf_evsel *evsel)
 		}
 	}
 }
+
+void perf_evsel__reset_counts(struct perf_evsel *evsel, int ncpus)
+{
+	memset(evsel->counts, 0, (sizeof(*evsel->counts) +
+				 (ncpus * sizeof(struct perf_counts_values))));
+}
+
+int perf_evsel__alloc_counts(struct perf_evsel *evsel, int ncpus)
+{
+	evsel->counts = zalloc((sizeof(*evsel->counts) +
+				(ncpus * sizeof(struct perf_counts_values))));
+	return evsel->counts != NULL ? 0 : -ENOMEM;
+}
+
+void perf_evsel__free_counts(struct perf_evsel *evsel)
+{
+	zfree(&evsel->counts);
+}
tools/perf/util/stat.h +3 -0
index 615c779eb42a..6a782601c1c7 100644
@@ -62,4 +62,7 @@ void perf_stat__update_shadow_stats(struct perf_evsel *counter, u64 *count,
 void perf_stat__print_shadow_stats(FILE *out, struct perf_evsel *evsel,
 				   double avg, int cpu, enum aggr_mode aggr);
 
+void perf_evsel__reset_counts(struct perf_evsel *evsel, int ncpus);
+int perf_evsel__alloc_counts(struct perf_evsel *evsel, int ncpus);
+void perf_evsel__free_counts(struct perf_evsel *evsel);
 #endif
Signature · 1 line
2.1.0
author maintainer
#8 lore raw

PATCH 8/10 perf stat: Introduce perf_counts__(new|delete|reset) functions

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Move 'struct perf_counts' allocation|free|reset code into separate functions.

---
 tools/perf/builtin-stat.c | 19 +++++++------------
 tools/perf/util/stat.c    | 28 +++++++++++++++++++++++-----
 tools/perf/util/stat.h    |  3 +++
 3 files changed, 33 insertions(+), 17 deletions(-)
tools/perf/builtin-stat.c +7 -12
index b24ecee95fec..fcf99bdeb19e 100644
@@ -178,24 +178,19 @@ static void perf_evsel__free_stat_priv(struct perf_evsel *evsel)
 
 static int perf_evsel__alloc_prev_raw_counts(struct perf_evsel *evsel)
 {
-	void *addr;
-	size_t sz;
+	struct perf_counts *counts;
 
-	sz = sizeof(*evsel->counts) +
-	     (perf_evsel__nr_cpus(evsel) * sizeof(struct perf_counts_values));
+	counts = perf_counts__new(perf_evsel__nr_cpus(evsel));
+	if (counts)
+		evsel->prev_raw_counts = counts;
 
-	addr = zalloc(sz);
-	if (!addr)
-		return -ENOMEM;
-
-	evsel->prev_raw_counts =  addr;
-
-	return 0;
+	return counts ? 0 : -ENOMEM;
 }
 
 static void perf_evsel__free_prev_raw_counts(struct perf_evsel *evsel)
 {
-	zfree(&evsel->prev_raw_counts);
+	perf_counts__delete(evsel->prev_raw_counts);
+	evsel->prev_raw_counts = NULL;
 }
 
 static void perf_evlist__free_stats(struct perf_evlist *evlist)
tools/perf/util/stat.c +23 -5
index ac589b6b8bce..4014b709f956 100644
@@ -95,20 +95,38 @@ void perf_stat_evsel_id_init(struct perf_evsel *evsel)
 	}
 }
 
+struct perf_counts *perf_counts__new(int ncpus)
+{
+	int size = sizeof(struct perf_counts) +
+		   ncpus * sizeof(struct perf_counts_values);
+
+	return zalloc(size);
+}
+
+void perf_counts__delete(struct perf_counts *counts)
+{
+	free(counts);
+}
+
+static void perf_counts__reset(struct perf_counts *counts, int ncpus)
+{
+	memset(counts, 0, (sizeof(*counts) +
+	       (ncpus * sizeof(struct perf_counts_values))));
+}
+
 void perf_evsel__reset_counts(struct perf_evsel *evsel, int ncpus)
 {
-	memset(evsel->counts, 0, (sizeof(*evsel->counts) +
-				 (ncpus * sizeof(struct perf_counts_values))));
+	perf_counts__reset(evsel->counts, ncpus);
 }
 
 int perf_evsel__alloc_counts(struct perf_evsel *evsel, int ncpus)
 {
-	evsel->counts = zalloc((sizeof(*evsel->counts) +
-				(ncpus * sizeof(struct perf_counts_values))));
+	evsel->counts = perf_counts__new(ncpus);
 	return evsel->counts != NULL ? 0 : -ENOMEM;
 }
 
 void perf_evsel__free_counts(struct perf_evsel *evsel)
 {
-	zfree(&evsel->counts);
+	perf_counts__delete(evsel->counts);
+	evsel->counts = NULL;
 }
tools/perf/util/stat.h +3 -0
index 6a782601c1c7..093dc3cb28dd 100644
@@ -62,6 +62,9 @@ void perf_stat__update_shadow_stats(struct perf_evsel *counter, u64 *count,
 void perf_stat__print_shadow_stats(FILE *out, struct perf_evsel *evsel,
 				   double avg, int cpu, enum aggr_mode aggr);
 
+struct perf_counts *perf_counts__new(int ncpus);
+void perf_counts__delete(struct perf_counts *counts);
+
 void perf_evsel__reset_counts(struct perf_evsel *evsel, int ncpus);
 int perf_evsel__alloc_counts(struct perf_evsel *evsel, int ncpus);
 void perf_evsel__free_counts(struct perf_evsel *evsel);
Signature · 1 line
2.1.0
author maintainer
#9 lore raw

PATCH 9/10 perf unwind: Fix a compile error

In reply to Arnaldo Carvalho de Melo

From: Hou Pengyang <[email protected]>

When libunwind is on, there is a compile error as :

util/unwind-libunwind.c:363:21: error: 'dso' undeclared (first use in this function)

      dso__data_put_fd(dso);

This patch fixes it.

Cc Namhyung Kim <[email protected]>
Cc Wang Nan <[email protected]>
Fixes 4bb11d012ab248d0 ("perf tools: Add dso__data_get/put_fd()")
---
 tools/perf/util/unwind-libunwind.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/unwind-libunwind.c +1 -1
index f079b63f0b7f..4c00507ee3fd 100644
@@ -360,7 +360,7 @@ find_proc_info(unw_addr_space_t as, unw_word_t ip, unw_proc_info_t *pi,
 		unw_word_t base = is_exec ? 0 : map->start;
 
 		if (fd >= 0)
-			dso__data_put_fd(dso);
+			dso__data_put_fd(map->dso);
 
 		memset(&di, 0, sizeof(di));
 		if (dwarf_find_debug_frame(0, &di, ip, base, map->dso->name,
Signature · 1 line
2.1.0
author maintainer
#10 lore raw

PATCH 10/10 perf probe: Fix to return error if no probe is added

In reply to Arnaldo Carvalho de Melo

From: Masami Hiramatsu <[email protected]>

Fix perf probe to return an error if no probe is added due to the given probe point being on the blacklist.

To fix this problem, this moves the blacklist checking to right after finding symbols/probe-points and marks them as skipped.

If all the symbols are skipped, "perf probe" returns an error as it fails to find the corresponding probe address.

E.g. currently if a blacklisted probe is given:

  # perf probe do_trap && echo 'succeed'
  Added new event:
  Warning: Skipped probing on blacklisted function: sync_regs
  succeed

No! It must fail! With this patch, it correctly fails:

  # perf probe do_trap && echo 'succeed'
  do_trap is blacklisted function, skip it.
  Probe point 'do_trap' not found.
    Error: Failed to add events.
---
 tools/perf/util/probe-event.c | 113 ++++++++++++++++++++++++++----------------
 1 file changed, 71 insertions(+), 42 deletions(-)
tools/perf/util/probe-event.c +71 -42
index c4ab58870fcc..85c8207c25cc 100644
@@ -246,6 +246,20 @@ static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
 		clear_probe_trace_event(tevs + i);
 }
 
+static bool kprobe_blacklist__listed(unsigned long address);
+static bool kprobe_warn_out_range(const char *symbol, unsigned long address)
+{
+	/* Get the address of _etext for checking non-probable text symbol */
+	if (kernel_get_symbol_address_by_name("_etext", false) < address)
+		pr_warning("%s is out of .text, skip it.\n", symbol);
+	else if (kprobe_blacklist__listed(address))
+		pr_warning("%s is blacklisted function, skip it.\n", symbol);
+	else
+		return false;
+
+	return true;
+}
+
 #ifdef HAVE_DWARF_SUPPORT
 
 static int kernel_get_module_dso(const char *module, struct dso **pdso)
@@ -559,7 +573,6 @@ static int post_process_probe_trace_events(struct probe_trace_event *tevs,
 					   bool uprobe)
 {
 	struct ref_reloc_sym *reloc_sym;
-	u64 etext_addr;
 	char *tmp;
 	int i, skipped = 0;
 
@@ -575,31 +588,28 @@ static int post_process_probe_trace_events(struct probe_trace_event *tevs,
 		pr_warning("Relocated base symbol is not found!\n");
 		return -EINVAL;
 	}
-	/* Get the address of _etext for checking non-probable text symbol */
-	etext_addr = kernel_get_symbol_address_by_name("_etext", false);
 
 	for (i = 0; i < ntevs; i++) {
-		if (tevs[i].point.address && !tevs[i].point.retprobe) {
-			/* If we found a wrong one, mark it by NULL symbol */
-			if (etext_addr < tevs[i].point.address) {
-				pr_warning("%s+%lu is out of .text, skip it.\n",
-				   tevs[i].point.symbol, tevs[i].point.offset);
-				tmp = NULL;
-				skipped++;
-			} else {
-				tmp = strdup(reloc_sym->name);
-				if (!tmp)
-					return -ENOMEM;
-			}
-			/* If we have no realname, use symbol for it */
-			if (!tevs[i].point.realname)
-				tevs[i].point.realname = tevs[i].point.symbol;
-			else
-				free(tevs[i].point.symbol);
-			tevs[i].point.symbol = tmp;
-			tevs[i].point.offset = tevs[i].point.address -
-					       reloc_sym->unrelocated_addr;
+		if (!tevs[i].point.address || tevs[i].point.retprobe)
+			continue;
+		/* If we found a wrong one, mark it by NULL symbol */
+		if (kprobe_warn_out_range(tevs[i].point.symbol,
+					  tevs[i].point.address)) {
+			tmp = NULL;
+			skipped++;
+		} else {
+			tmp = strdup(reloc_sym->name);
+			if (!tmp)
+				return -ENOMEM;
 		}
+		/* If we have no realname, use symbol for it */
+		if (!tevs[i].point.realname)
+			tevs[i].point.realname = tevs[i].point.symbol;
+		else
+			free(tevs[i].point.symbol);
+		tevs[i].point.symbol = tmp;
+		tevs[i].point.offset = tevs[i].point.address -
+				       reloc_sym->unrelocated_addr;
 	}
 	return skipped;
 }
@@ -2126,6 +2136,27 @@ kprobe_blacklist__find_by_address(struct list_head *blacklist,
 	return NULL;
 }
 
+static LIST_HEAD(kprobe_blacklist);
+
+static void kprobe_blacklist__init(void)
+{
+	if (!list_empty(&kprobe_blacklist))
+		return;
+
+	if (kprobe_blacklist__load(&kprobe_blacklist) < 0)
+		pr_debug("No kprobe blacklist support, ignored\n");
+}
+
+static void kprobe_blacklist__release(void)
+{
+	kprobe_blacklist__delete(&kprobe_blacklist);
+}
+
+static bool kprobe_blacklist__listed(unsigned long address)
+{
+	return !!kprobe_blacklist__find_by_address(&kprobe_blacklist, address);
+}
+
 static int perf_probe_event__sprintf(struct perf_probe_event *pev,
 				     const char *module,
 				     struct strbuf *result)
@@ -2409,8 +2440,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 	char buf[64];
 	const char *event, *group;
 	struct strlist *namelist;
-	LIST_HEAD(blacklist);
-	struct kprobe_blacklist_node *node;
 	bool safename;
 
 	if (pev->uprobes)
@@ -2430,28 +2459,15 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 		ret = -ENOMEM;
 		goto close_out;
 	}
-	/* Get kprobe blacklist if exists */
-	if (!pev->uprobes) {
-		ret = kprobe_blacklist__load(&blacklist);
-		if (ret < 0)
-			pr_debug("No kprobe blacklist support, ignored\n");
-	}
 
 	safename = (pev->point.function && !strisglob(pev->point.function));
 	ret = 0;
 	pr_info("Added new event%s\n", (ntevs > 1) ? "s:" : ":");
 	for (i = 0; i < ntevs; i++) {
 		tev = &tevs[i];
-		/* Skip if the symbol is out of .text (marked previously) */
+		/* Skip if the symbol is out of .text or blacklisted */
 		if (!tev->point.symbol)
 			continue;
-		/* Ensure that the address is NOT blacklisted */
-		node = kprobe_blacklist__find_by_address(&blacklist,
-							 tev->point.address);
-		if (node) {
-			pr_warning("Warning: Skipped probing on blacklisted function: %s\n", node->symbol);
-			continue;
-		}
 
 		if (pev->event)
 			event = pev->event;
@@ -2513,7 +2529,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 			 tev->event);
 	}
 
-	kprobe_blacklist__delete(&blacklist);
 	strlist__delete(namelist);
 close_out:
 	close(fd);
@@ -2563,7 +2578,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 	struct perf_probe_point *pp = &pev->point;
 	struct probe_trace_point *tp;
 	int num_matched_functions;
-	int ret, i, j;
+	int ret, i, j, skipped = 0;
 
 	map = get_target_map(pev->target, pev->uprobes);
 	if (!map) {
@@ -2631,7 +2646,12 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 		}
 		/* Add one probe point */
 		tp->address = map->unmap_ip(map, sym->start) + pp->offset;
-		if (reloc_sym) {
+		/* If we found a wrong one, mark it by NULL symbol */
+		if (!pev->uprobes &&
+		    kprobe_warn_out_range(sym->name, tp->address)) {
+			tp->symbol = NULL;	/* Skip it */
+			skipped++;
+		} else if (reloc_sym) {
 			tp->symbol = strdup_or_goto(reloc_sym->name, nomem_out);
 			tp->offset = tp->address - reloc_sym->addr;
 		} else {
@@ -2667,6 +2687,10 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 		}
 		arch__fix_tev_from_maps(pev, tev, map);
 	}
+	if (ret == skipped) {
+		ret = -ENOENT;
+		goto err_out;
+	}
 
 out:
 	put_target_map(map, pev->uprobes);
@@ -2737,6 +2761,9 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 	/* Loop 1: convert all events */
 	for (i = 0; i < npevs; i++) {
 		pkgs[i].pev = &pevs[i];
+		/* Init kprobe blacklist if needed */
+		if (!pkgs[i].pev->uprobes)
+			kprobe_blacklist__init();
 		/* Convert with or without debuginfo */
 		ret  = convert_to_probe_trace_events(pkgs[i].pev,
 						     &pkgs[i].tevs);
@@ -2744,6 +2771,8 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 			goto end;
 		pkgs[i].ntevs = ret;
 	}
+	/* This just release blacklist only if allocated */
+	kprobe_blacklist__release();
 
 	/* Loop 2: add all events */
 	for (i = 0; i < npevs; i++) {
Signature · 1 line
2.1.0