perf tools: Fixes

PATCH New Jiri Olsa 2 participants 9 messages Jan 20, 2016 – Feb 3, 2016

Subsystems (2)

Stable branch perf events

lore

Share & tools

lore

Jiri Olsa

author reviewer

2016-01-20 11:56 UTC #0 lore raw

cover perf tools: Fixes

hi,
sending few assorted fixes, all on top
of my perf/fixes branch

thanks,
jirka

---
Jiri Olsa (4):
perf tools: Do not read symbols/data from device files perf tools: Fix HISTC_MEM_DCACHELINE width setting perf stat: Do not clean event's private stats perf stat: Making several helper functions static

 tools/perf/util/dso.c    |  5 +++++
 tools/perf/util/hist.c   |  2 ++
 tools/perf/util/stat.c   | 15 +++++++--------
 tools/perf/util/stat.h   | 10 ----------
 tools/perf/util/symbol.c |  6 +++++-
 tools/perf/util/util.c   | 10 ++++++++++
 tools/perf/util/util.h   |  1 +
 7 files changed, 30 insertions(+), 19 deletions(-)

Jiri Olsa

author reviewer

2016-01-20 11:56 UTC #1 lore raw

PATCH 1/4 perf tools: Do not read symbols/data from device files

With mem sampling we could get data source within mapped device file. Processing such sample would block during report phase on trying to read the device file.

Chacking for device files and skip the processing if it's detected.

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/util/dso.c    |  5 +++++
 tools/perf/util/symbol.c |  6 +++++-
 tools/perf/util/util.c   | 10 ++++++++++
 tools/perf/util/util.h   |  1 +
 4 files changed, 21 insertions(+), 1 deletion(-)

tools/perf/util/dso.c +5 -0

index e8e9a9dbf5e3..8e6395439ca0 100644
@@ -52,6 +52,11 @@ int dso__read_binary_type_filename(const struct dso *dso,
 			debuglink--;
 		if (*debuglink == '/')
 			debuglink++;
+
+		ret = -1;
+		if (!is_regular_file(filename))
+			break;
+
 		ret = filename__read_debuglink(filename, debuglink,
 					       size - (debuglink - filename));
 		}

tools/perf/util/symbol.c +5 -1

index ab02209a7cf3..90cedfa30e43 100644
@@ -1466,7 +1466,8 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 	 * Read the build id if possible. This is required for
 	 * DSO_BINARY_TYPE__BUILDID_DEBUGINFO to work
 	 */
-	if (filename__read_build_id(dso->long_name, build_id, BUILD_ID_SIZE) > 0)
+	if (is_regular_file(name) &&
+	    filename__read_build_id(dso->long_name, build_id, BUILD_ID_SIZE) > 0)
 		dso__set_build_id(dso, build_id);
 
 	/*

@@ -1487,6 +1488,9 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 						   root_dir, name, PATH_MAX))
 			continue;
 
+		if (!is_regular_file(name))
+			continue;
+
 		/* Name is now the name of the next image to try */
 		if (symsrc__init(ss, dso, name, symtab_type) < 0)
 			continue;

tools/perf/util/util.c +10 -0

index ead9509835d2..7a2da7ef556e 100644
@@ -691,3 +691,13 @@ out:
 
 	return tip;
 }
+
+bool is_regular_file(const char *file)
+{
+	struct stat st;
+
+	if (stat(file, &st))
+		return false;
+
+	return S_ISREG(st.st_mode);
+}

tools/perf/util/util.h +1 -0

index fe915e616f9b..61650f05e5c1 100644
@@ -343,5 +343,6 @@ int fetch_kernel_version(unsigned int *puint,
 #define KVER_PARAM(x)	KVER_VERSION(x), KVER_PATCHLEVEL(x), KVER_SUBLEVEL(x)
 
 const char *perf_tip(const char *dirpath);
+bool is_regular_file(const char *file);
 
 #endif /* GIT_COMPAT_UTIL_H */

Signature · 1 line

2.4.3

tip-bot for Jiri Olsa

merge bot

2016-02-03 10:07 UTC #2 lore raw

[tip:perf/core] perf symbols: Do not read symbols/ data from device files

perf symbols: Do not read symbols/data from device files

Commit-ID:  403567217d3fa5d4801f820317ada52e5c5f0e53
Gitweb:     http://git.kernel.org/tip/403567217d3fa5d4801f820317ada52e5c5f0e53
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 20 Jan 2016 12:56:32 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 26 Jan 2016 11:52:43 -0300

perf symbols: Do not read symbols/data from device files

With mem sampling we could get data source within mapped device file. Processing such sample would block during report phase on trying to read the device file.

Chacking for device files and skip the processing if it's detected.

Signed-off-by Jiri Olsa <[email protected]>

Cc David Ahern <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/dso.c    |  5 +++++
 tools/perf/util/symbol.c |  6 +++++-
 tools/perf/util/util.c   | 10 ++++++++++
 tools/perf/util/util.h   |  1 +
 4 files changed, 21 insertions(+), 1 deletion(-)

tools/perf/util/dso.c +5 -0

index e8e9a9d..8e639543 100644
@@ -52,6 +52,11 @@ int dso__read_binary_type_filename(const struct dso *dso,
 			debuglink--;
 		if (*debuglink == '/')
 			debuglink++;
+
+		ret = -1;
+		if (!is_regular_file(filename))
+			break;
+
 		ret = filename__read_debuglink(filename, debuglink,
 					       size - (debuglink - filename));
 		}

tools/perf/util/symbol.c +5 -1

index ab02209..90cedfa 100644
@@ -1466,7 +1466,8 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 	 * Read the build id if possible. This is required for
 	 * DSO_BINARY_TYPE__BUILDID_DEBUGINFO to work
 	 */
-	if (filename__read_build_id(dso->long_name, build_id, BUILD_ID_SIZE) > 0)
+	if (is_regular_file(name) &&
+	    filename__read_build_id(dso->long_name, build_id, BUILD_ID_SIZE) > 0)
 		dso__set_build_id(dso, build_id);
 
 	/*

@@ -1487,6 +1488,9 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 						   root_dir, name, PATH_MAX))
 			continue;
 
+		if (!is_regular_file(name))
+			continue;
+
 		/* Name is now the name of the next image to try */
 		if (symsrc__init(ss, dso, name, symtab_type) < 0)
 			continue;

tools/perf/util/util.c +10 -0

index ead9509..7a2da7e 100644
@@ -691,3 +691,13 @@ out:
 
 	return tip;
 }
+
+bool is_regular_file(const char *file)
+{
+	struct stat st;
+
+	if (stat(file, &st))
+		return false;
+
+	return S_ISREG(st.st_mode);
+}

tools/perf/util/util.h +1 -0

index fe915e6..61650f0 100644
@@ -343,5 +343,6 @@ int fetch_kernel_version(unsigned int *puint,
 #define KVER_PARAM(x)	KVER_VERSION(x), KVER_PATCHLEVEL(x), KVER_SUBLEVEL(x)
 
 const char *perf_tip(const char *dirpath);
+bool is_regular_file(const char *file);
 
 #endif /* GIT_COMPAT_UTIL_H */

Jiri Olsa

author reviewer

2016-01-20 11:57 UTC #3 lore raw

PATCH 2/4 perf tools: Fix HISTC_MEM_DCACHELINE width setting

In reply to Jiri Olsa

Set correct with for unresolved addr.

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/util/hist.c | 2 ++
 1 file changed, 2 insertions(+)

tools/perf/util/hist.c +2 -0

index c226303e3da0..68a7612019dc 100644
@@ -131,6 +131,8 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 			symlen = unresolved_col_width + 4 + 2;
 			hists__new_col_len(hists, HISTC_MEM_DADDR_SYMBOL,
 					   symlen);
+			hists__new_col_len(hists, HISTC_MEM_DCACHELINE,
+					   symlen);
 		}
 
 		if (h->mem_info->iaddr.sym) {

Signature · 1 line

2.4.3

tip-bot for Jiri Olsa

merge bot

2016-01-30 08:25 UTC #4 lore raw

[tip:perf/urgent] perf hists: Fix HISTC_MEM_DCACHELINE width setting

perf hists: Fix HISTC_MEM_DCACHELINE width setting

Commit-ID:  0805909f59e02036a4e2660159f27dbf8b6084ac
Gitweb:     http://git.kernel.org/tip/0805909f59e02036a4e2660159f27dbf8b6084ac
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 20 Jan 2016 12:56:33 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 26 Jan 2016 11:14:55 -0300

perf hists: Fix HISTC_MEM_DCACHELINE width setting

Set correct width for unresolved mem_dcacheline addr.

Signed-off-by Jiri Olsa <[email protected]>

Cc David Ahern <[email protected]>

Cc Don Zickus <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Fixes 9b32ba71ba90 ("perf tools: Add dcacheline sort")

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/hist.c | 2 ++
 1 file changed, 2 insertions(+)

tools/perf/util/hist.c +2 -0

index c226303..68a7612 100644
@@ -131,6 +131,8 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 			symlen = unresolved_col_width + 4 + 2;
 			hists__new_col_len(hists, HISTC_MEM_DADDR_SYMBOL,
 					   symlen);
+			hists__new_col_len(hists, HISTC_MEM_DCACHELINE,
+					   symlen);
 		}
 
 		if (h->mem_info->iaddr.sym) {

Jiri Olsa

author reviewer

2016-01-20 11:56 UTC #5 lore raw

PATCH 3/4 perf stat: Do not clean event's private stats

In reply to Jiri Olsa

Mel reported stddev reporting was broken due to following commit:
106a94a0f8c2 perf stat: Introduce read_counters function

This commit merged interval and overall counters reading into single read_counters function.

The old interval code cleaned the stddev data for some reason (it's never displayed in interval mode) and the mentioned commit kept on cleaning the stddev data in merged function, which resulted in the stddev not being displayed.

Removing the wrong stddev data cleanup init_stats call.

Reported-by Mel Gorman <[email protected]>

Tested-by Mel Gorman <[email protected]>

Cc [email protected] # v4.2+

Cc Mel Gorman <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/util/stat.c | 1 -
 1 file changed, 1 deletion(-)

tools/perf/util/stat.c +0 -1

index 2f901d15e063..2b58edccd56f 100644
@@ -310,7 +310,6 @@ int perf_stat_process_counter(struct perf_stat_config *config,
 	int i, ret;
 
 	aggr->val = aggr->ena = aggr->run = 0;
-	init_stats(ps->res_stats);
 
 	if (counter->per_pkg)
 		zero_per_pkg(counter);

Signature · 1 line

2.4.3

tip-bot for Jiri Olsa

merge bot

2016-01-30 08:26 UTC #6 lore raw

[tip:perf/urgent] perf stat: Do not clean event's private stats

perf stat: Do not clean event's private stats

Commit-ID:  3f416f22d1e21709a631189ba169f76fd267b374
Gitweb:     http://git.kernel.org/tip/3f416f22d1e21709a631189ba169f76fd267b374
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 20 Jan 2016 12:56:34 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 26 Jan 2016 11:15:11 -0300

perf stat: Do not clean event's private stats

Mel reported stddev reporting was broken due to following commit:

106a94a0f8c2 ("perf stat: Introduce read_counters function")

This commit merged interval and overall counters reading into single read_counters function.

Removing the wrong stddev data cleanup init_stats call.

Reported-and-Tested-by Mel Gorman <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

Cc David Ahern <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc [email protected] # v4.2+

Fixes 106a94a0f8c2 ("perf stat: Introduce read_counters function")

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/stat.c | 1 -
 1 file changed, 1 deletion(-)

tools/perf/util/stat.c +0 -1

index 2f901d1..2b58edc 100644
@@ -310,7 +310,6 @@ int perf_stat_process_counter(struct perf_stat_config *config,
 	int i, ret;
 
 	aggr->val = aggr->ena = aggr->run = 0;
-	init_stats(ps->res_stats);
 
 	if (counter->per_pkg)
 		zero_per_pkg(counter);

Jiri Olsa

author reviewer

2016-01-20 11:57 UTC #7 lore raw

PATCH 4/4 perf stat: Making several helper functions static

In reply to Jiri Olsa

There's no need for following functions to be global:

perf_evsel__reset_stat_priv
perf_evsel__alloc_stat_priv
perf_evsel__free_stat_priv
perf_evsel__alloc_prev_raw_counts
perf_evsel__free_prev_raw_counts
perf_evsel__alloc_stats

They all ended up in util/stat.c, and they no longer need to be called from outside this object.

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/util/stat.c | 14 +++++++-------
 tools/perf/util/stat.h | 10 ----------
 2 files changed, 7 insertions(+), 17 deletions(-)

tools/perf/util/stat.c +7 -7

index 2b58edccd56f..beeed0bd3bee 100644
@@ -97,7 +97,7 @@ void perf_stat_evsel_id_init(struct perf_evsel *evsel)
 	}
 }
 
-void perf_evsel__reset_stat_priv(struct perf_evsel *evsel)
+static void perf_evsel__reset_stat_priv(struct perf_evsel *evsel)
 {
 	int i;
 	struct perf_stat_evsel *ps = evsel->priv;

@@ -108,7 +108,7 @@ void perf_evsel__reset_stat_priv(struct perf_evsel *evsel)
 	perf_stat_evsel_id_init(evsel);
 }
 
-int perf_evsel__alloc_stat_priv(struct perf_evsel *evsel)
+static int perf_evsel__alloc_stat_priv(struct perf_evsel *evsel)
 {
 	evsel->priv = zalloc(sizeof(struct perf_stat_evsel));
 	if (evsel->priv == NULL)

@@ -117,13 +117,13 @@ int perf_evsel__alloc_stat_priv(struct perf_evsel *evsel)
 	return 0;
 }
 
-void perf_evsel__free_stat_priv(struct perf_evsel *evsel)
+static void perf_evsel__free_stat_priv(struct perf_evsel *evsel)
 {
 	zfree(&evsel->priv);
 }
 
-int perf_evsel__alloc_prev_raw_counts(struct perf_evsel *evsel,
-				      int ncpus, int nthreads)
+static int perf_evsel__alloc_prev_raw_counts(struct perf_evsel *evsel,
+					     int ncpus, int nthreads)
 {
 	struct perf_counts *counts;

@@ -134,13 +134,13 @@ int perf_evsel__alloc_prev_raw_counts(struct perf_evsel *evsel,
 	return counts ? 0 : -ENOMEM;
 }
 
-void perf_evsel__free_prev_raw_counts(struct perf_evsel *evsel)
+static void perf_evsel__free_prev_raw_counts(struct perf_evsel *evsel)
 {
 	perf_counts__delete(evsel->prev_raw_counts);
 	evsel->prev_raw_counts = NULL;
 }
 
-int perf_evsel__alloc_stats(struct perf_evsel *evsel, bool alloc_raw)
+static int perf_evsel__alloc_stats(struct perf_evsel *evsel, bool alloc_raw)
 {
 	int ncpus = perf_evsel__nr_cpus(evsel);
 	int nthreads = thread_map__nr(evsel->threads);

tools/perf/util/stat.h +0 ~~-10~~

index 086f4e128d63..2af63c9cb59f 100644
@@ -74,16 +74,6 @@ 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_stat_priv(struct perf_evsel *evsel);
-int perf_evsel__alloc_stat_priv(struct perf_evsel *evsel);
-void perf_evsel__free_stat_priv(struct perf_evsel *evsel);
-
-int perf_evsel__alloc_prev_raw_counts(struct perf_evsel *evsel,
-				      int ncpus, int nthreads);
-void perf_evsel__free_prev_raw_counts(struct perf_evsel *evsel);
-
-int perf_evsel__alloc_stats(struct perf_evsel *evsel, bool alloc_raw);
-
 int perf_evlist__alloc_stats(struct perf_evlist *evlist, bool alloc_raw);
 void perf_evlist__free_stats(struct perf_evlist *evlist);
 void perf_evlist__reset_stats(struct perf_evlist *evlist);

Signature · 1 line

2.4.3

tip-bot for Jiri Olsa

merge bot

2016-02-03 10:07 UTC #8 lore raw

[tip:perf/core] perf stat: Making several helper functions static

perf stat: Making several helper functions static

Commit-ID:  86a2cf3123bfec118bfb98728d88be0668779b2b
Gitweb:     http://git.kernel.org/tip/86a2cf3123bfec118bfb98728d88be0668779b2b
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 20 Jan 2016 12:56:35 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 26 Jan 2016 11:52:43 -0300

perf stat: Making several helper functions static

There's no need for the following functions to be global:

perf_evsel__reset_stat_priv
perf_evsel__alloc_stat_priv
perf_evsel__free_stat_priv
perf_evsel__alloc_prev_raw_counts
perf_evsel__free_prev_raw_counts
perf_evsel__alloc_stats

They all ended up in util/stat.c, and they no longer need to be called from outside this object.

Signed-off-by Jiri Olsa <[email protected]>

Cc David Ahern <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/stat.c | 14 +++++++-------
 tools/perf/util/stat.h | 10 ----------
 2 files changed, 7 insertions(+), 17 deletions(-)

tools/perf/util/stat.c +7 -7

index 2b58edc..beeed0b 100644
@@ -97,7 +97,7 @@ void perf_stat_evsel_id_init(struct perf_evsel *evsel)
 	}
 }
 
-void perf_evsel__reset_stat_priv(struct perf_evsel *evsel)
+static void perf_evsel__reset_stat_priv(struct perf_evsel *evsel)
 {
 	int i;
 	struct perf_stat_evsel *ps = evsel->priv;

@@ -108,7 +108,7 @@ void perf_evsel__reset_stat_priv(struct perf_evsel *evsel)
 	perf_stat_evsel_id_init(evsel);
 }
 
-int perf_evsel__alloc_stat_priv(struct perf_evsel *evsel)
+static int perf_evsel__alloc_stat_priv(struct perf_evsel *evsel)
 {
 	evsel->priv = zalloc(sizeof(struct perf_stat_evsel));
 	if (evsel->priv == NULL)

@@ -117,13 +117,13 @@ int perf_evsel__alloc_stat_priv(struct perf_evsel *evsel)
 	return 0;
 }
 
-void perf_evsel__free_stat_priv(struct perf_evsel *evsel)
+static void perf_evsel__free_stat_priv(struct perf_evsel *evsel)
 {
 	zfree(&evsel->priv);
 }
 
-int perf_evsel__alloc_prev_raw_counts(struct perf_evsel *evsel,
-				      int ncpus, int nthreads)
+static int perf_evsel__alloc_prev_raw_counts(struct perf_evsel *evsel,
+					     int ncpus, int nthreads)
 {
 	struct perf_counts *counts;

@@ -134,13 +134,13 @@ int perf_evsel__alloc_prev_raw_counts(struct perf_evsel *evsel,
 	return counts ? 0 : -ENOMEM;
 }
 
-void perf_evsel__free_prev_raw_counts(struct perf_evsel *evsel)
+static void perf_evsel__free_prev_raw_counts(struct perf_evsel *evsel)
 {
 	perf_counts__delete(evsel->prev_raw_counts);
 	evsel->prev_raw_counts = NULL;
 }
 
-int perf_evsel__alloc_stats(struct perf_evsel *evsel, bool alloc_raw)
+static int perf_evsel__alloc_stats(struct perf_evsel *evsel, bool alloc_raw)
 {
 	int ncpus = perf_evsel__nr_cpus(evsel);
 	int nthreads = thread_map__nr(evsel->threads);

tools/perf/util/stat.h +0 ~~-10~~

index 086f4e1..2af63c9 100644
@@ -74,16 +74,6 @@ 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_stat_priv(struct perf_evsel *evsel);
-int perf_evsel__alloc_stat_priv(struct perf_evsel *evsel);
-void perf_evsel__free_stat_priv(struct perf_evsel *evsel);
-
-int perf_evsel__alloc_prev_raw_counts(struct perf_evsel *evsel,
-				      int ncpus, int nthreads);
-void perf_evsel__free_prev_raw_counts(struct perf_evsel *evsel);
-
-int perf_evsel__alloc_stats(struct perf_evsel *evsel, bool alloc_raw);
-
 int perf_evlist__alloc_stats(struct perf_evlist *evlist, bool alloc_raw);
 void perf_evlist__free_stats(struct perf_evlist *evlist);
 void perf_evlist__reset_stats(struct perf_evlist *evlist);