linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL] perf/core improvements and fixes from Budapest
@ 2020-02-01  8:03 Arnaldo Carvalho de Melo
  2020-02-01  8:03 ` [PATCH 1/6] perf parse: Refactor 'struct perf_evsel_config_term' Arnaldo Carvalho de Melo
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-02-01  8:03 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner
  Cc: Jiri Olsa, Namhyung Kim, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo, Cengiz Can,
	Changbin Du, Leo Yan, Thomas Richter, Arnaldo Carvalho de Melo

Hi Ingo/Thomas,

	Please consider pulling,

Best regards,

- Arnaldo

The following changes since commit 0cc4bd8f70d1ea2940295f1050508c663fe9eff9:

  Merge branch 'core/kprobes' into perf/core, to pick up fixes (2020-01-28 07:59:05 +0100)

are available in the Git repository at:

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

for you to fetch changes up to 85fc95d75970ee7dd8e01904e7fb1197c275ba6b:

  perf maps: Add missing unlock to maps__insert() error case (2020-01-31 09:40:50 +0100)

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

perf maps:

  Cengiz Can:

  - Add missing unlock to maps__insert() error case.

srcline:

  Changbin Du:

  - Make perf able to build with latest libbfd.

perf parse:

  Leo Yan:

  - Keep copy of string in perf_evsel_config_term() to fix sink terms
    processing in ARM CoreSight.

perf test:

  Thomas Richter:

  - Fix test case Merge cpu map, removing extra reference count drop that
    causes a segfault on s/390.

perf probe:

  Thomas Richter:

  - Add ustring support for perf probe command

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

----------------------------------------------------------------
Cengiz Can (1):
      perf maps: Add missing unlock to maps__insert() error case

Changbin Du (1):
      perf: Make perf able to build with latest libbfd

Leo Yan (2):
      perf parse: Refactor 'struct perf_evsel_config_term'
      perf parse: Copy string to perf_evsel_config_term

Thomas Richter (2):
      perf test: Fix test case Merge cpu map
      perf probe: Add ustring support for perf probe command

 tools/perf/arch/arm/util/cs-etm.c |  2 +-
 tools/perf/tests/cpumap.c         |  1 -
 tools/perf/util/evsel.c           |  8 +++--
 tools/perf/util/evsel_config.h    |  5 ++-
 tools/perf/util/map.c             |  1 +
 tools/perf/util/parse-events.c    | 67 ++++++++++++++++++++++++++-------------
 tools/perf/util/probe-finder.c    |  3 +-
 tools/perf/util/srcline.c         | 16 +++++++++-
 8 files changed, 71 insertions(+), 32 deletions(-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/6] perf parse: Refactor 'struct perf_evsel_config_term'
  2020-02-01  8:03 [GIT PULL] perf/core improvements and fixes from Budapest Arnaldo Carvalho de Melo
@ 2020-02-01  8:03 ` Arnaldo Carvalho de Melo
  2020-02-01  8:03 ` [PATCH 2/6] perf parse: Copy string to perf_evsel_config_term Arnaldo Carvalho de Melo
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-02-01  8:03 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner
  Cc: Mark Rutland, Ian Rogers, Andi Kleen, Arnaldo Carvalho de Melo,
	Mathieu Poirier, Suzuki Poulouse, Clark Williams, linux-kernel,
	Adrian Hunter, linux-perf-users, Alexander Shishkin,
	Peter Zijlstra, Jiri Olsa, Leo Yan, Namhyung Kim,
	linux-arm-kernel, Mike Leach

From: Leo Yan <leo.yan@linaro.org>

The struct perf_evsel_config_term::val is a union which contains fields
'callgraph', 'drv_cfg' and 'branch' as string pointers.  This leads to
the complex code logic for handling every type's string separately, and
it's hard to release string as a general way.

This patch refactors the structure to add a common field 'str' in the
'val' union as string pointer and remove the other three fields
'callgraph', 'drv_cfg' and 'branch'.  Without passing field name, the
patch simplifies the string handling with macro ADD_CONFIG_TERM_STR()
for string pointer assignment.

This patch fixes multiple warnings of line over 80 characters detected
by checkpatch tool.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lore.kernel.org/lkml/20200117055251.24058-1-leo.yan@linaro.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/arm/util/cs-etm.c |  2 +-
 tools/perf/util/evsel.c           |  6 +--
 tools/perf/util/evsel_config.h    |  4 +-
 tools/perf/util/parse-events.c    | 62 ++++++++++++++++++++-----------
 4 files changed, 45 insertions(+), 29 deletions(-)

diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index ede040cf82ad..2898cfdf8fe1 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -226,7 +226,7 @@ static int cs_etm_set_sink_attr(struct perf_pmu *pmu,
 		if (term->type != PERF_EVSEL__CONFIG_TERM_DRV_CFG)
 			continue;
 
-		sink = term->val.drv_cfg;
+		sink = term->val.str;
 		snprintf(path, PATH_MAX, "sinks/%s", sink);
 
 		ret = perf_pmu__scan_file(pmu, path, "%x", &hash);
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index a69e64236120..549abd43816f 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -808,12 +808,12 @@ static void apply_config_terms(struct evsel *evsel,
 				perf_evsel__reset_sample_bit(evsel, TIME);
 			break;
 		case PERF_EVSEL__CONFIG_TERM_CALLGRAPH:
-			callgraph_buf = term->val.callgraph;
+			callgraph_buf = term->val.str;
 			break;
 		case PERF_EVSEL__CONFIG_TERM_BRANCH:
-			if (term->val.branch && strcmp(term->val.branch, "no")) {
+			if (term->val.str && strcmp(term->val.str, "no")) {
 				perf_evsel__set_sample_bit(evsel, BRANCH_STACK);
-				parse_branch_str(term->val.branch,
+				parse_branch_str(term->val.str,
 						 &attr->branch_sample_type);
 			} else
 				perf_evsel__reset_sample_bit(evsel, BRANCH_STACK);
diff --git a/tools/perf/util/evsel_config.h b/tools/perf/util/evsel_config.h
index 1f8d2fe0b66e..b4a65201e4f7 100644
--- a/tools/perf/util/evsel_config.h
+++ b/tools/perf/util/evsel_config.h
@@ -36,18 +36,16 @@ struct perf_evsel_config_term {
 		u64	      period;
 		u64	      freq;
 		bool	      time;
-		char	      *callgraph;
-		char	      *drv_cfg;
 		u64	      stack_user;
 		int	      max_stack;
 		bool	      inherit;
 		bool	      overwrite;
-		char	      *branch;
 		unsigned long max_events;
 		bool	      percore;
 		bool	      aux_output;
 		u32	      aux_sample_size;
 		u64	      cfg_chg;
+		char	      *str;
 	} val;
 	bool weak;
 };
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index ed7c008b9c8b..f59f3c8da473 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1219,8 +1219,7 @@ static int config_attr(struct perf_event_attr *attr,
 static int get_config_terms(struct list_head *head_config,
 			    struct list_head *head_terms __maybe_unused)
 {
-#define ADD_CONFIG_TERM(__type, __name, __val)			\
-do {								\
+#define ADD_CONFIG_TERM(__type)					\
 	struct perf_evsel_config_term *__t;			\
 								\
 	__t = zalloc(sizeof(*__t));				\
@@ -1229,9 +1228,19 @@ do {								\
 								\
 	INIT_LIST_HEAD(&__t->list);				\
 	__t->type       = PERF_EVSEL__CONFIG_TERM_ ## __type;	\
-	__t->val.__name = __val;				\
 	__t->weak	= term->weak;				\
-	list_add_tail(&__t->list, head_terms);			\
+	list_add_tail(&__t->list, head_terms)
+
+#define ADD_CONFIG_TERM_VAL(__type, __name, __val)		\
+do {								\
+	ADD_CONFIG_TERM(__type);				\
+	__t->val.__name = __val;				\
+} while (0)
+
+#define ADD_CONFIG_TERM_STR(__type, __val)			\
+do {								\
+	ADD_CONFIG_TERM(__type);				\
+	__t->val.str = __val;					\
 } while (0)
 
 	struct parse_events_term *term;
@@ -1239,53 +1248,62 @@ do {								\
 	list_for_each_entry(term, head_config, list) {
 		switch (term->type_term) {
 		case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
-			ADD_CONFIG_TERM(PERIOD, period, term->val.num);
+			ADD_CONFIG_TERM_VAL(PERIOD, period, term->val.num);
 			break;
 		case PARSE_EVENTS__TERM_TYPE_SAMPLE_FREQ:
-			ADD_CONFIG_TERM(FREQ, freq, term->val.num);
+			ADD_CONFIG_TERM_VAL(FREQ, freq, term->val.num);
 			break;
 		case PARSE_EVENTS__TERM_TYPE_TIME:
-			ADD_CONFIG_TERM(TIME, time, term->val.num);
+			ADD_CONFIG_TERM_VAL(TIME, time, term->val.num);
 			break;
 		case PARSE_EVENTS__TERM_TYPE_CALLGRAPH:
-			ADD_CONFIG_TERM(CALLGRAPH, callgraph, term->val.str);
+			ADD_CONFIG_TERM_STR(CALLGRAPH, term->val.str);
 			break;
 		case PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE:
-			ADD_CONFIG_TERM(BRANCH, branch, term->val.str);
+			ADD_CONFIG_TERM_STR(BRANCH, term->val.str);
 			break;
 		case PARSE_EVENTS__TERM_TYPE_STACKSIZE:
-			ADD_CONFIG_TERM(STACK_USER, stack_user, term->val.num);
+			ADD_CONFIG_TERM_VAL(STACK_USER, stack_user,
+					    term->val.num);
 			break;
 		case PARSE_EVENTS__TERM_TYPE_INHERIT:
-			ADD_CONFIG_TERM(INHERIT, inherit, term->val.num ? 1 : 0);
+			ADD_CONFIG_TERM_VAL(INHERIT, inherit,
+					    term->val.num ? 1 : 0);
 			break;
 		case PARSE_EVENTS__TERM_TYPE_NOINHERIT:
-			ADD_CONFIG_TERM(INHERIT, inherit, term->val.num ? 0 : 1);
+			ADD_CONFIG_TERM_VAL(INHERIT, inherit,
+					    term->val.num ? 0 : 1);
 			break;
 		case PARSE_EVENTS__TERM_TYPE_MAX_STACK:
-			ADD_CONFIG_TERM(MAX_STACK, max_stack, term->val.num);
+			ADD_CONFIG_TERM_VAL(MAX_STACK, max_stack,
+					    term->val.num);
 			break;
 		case PARSE_EVENTS__TERM_TYPE_MAX_EVENTS:
-			ADD_CONFIG_TERM(MAX_EVENTS, max_events, term->val.num);
+			ADD_CONFIG_TERM_VAL(MAX_EVENTS, max_events,
+					    term->val.num);
 			break;
 		case PARSE_EVENTS__TERM_TYPE_OVERWRITE:
-			ADD_CONFIG_TERM(OVERWRITE, overwrite, term->val.num ? 1 : 0);
+			ADD_CONFIG_TERM_VAL(OVERWRITE, overwrite,
+					    term->val.num ? 1 : 0);
 			break;
 		case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:
-			ADD_CONFIG_TERM(OVERWRITE, overwrite, term->val.num ? 0 : 1);
+			ADD_CONFIG_TERM_VAL(OVERWRITE, overwrite,
+					    term->val.num ? 0 : 1);
 			break;
 		case PARSE_EVENTS__TERM_TYPE_DRV_CFG:
-			ADD_CONFIG_TERM(DRV_CFG, drv_cfg, term->val.str);
+			ADD_CONFIG_TERM_STR(DRV_CFG, term->val.str);
 			break;
 		case PARSE_EVENTS__TERM_TYPE_PERCORE:
-			ADD_CONFIG_TERM(PERCORE, percore,
-					term->val.num ? true : false);
+			ADD_CONFIG_TERM_VAL(PERCORE, percore,
+					    term->val.num ? true : false);
 			break;
 		case PARSE_EVENTS__TERM_TYPE_AUX_OUTPUT:
-			ADD_CONFIG_TERM(AUX_OUTPUT, aux_output, term->val.num ? 1 : 0);
+			ADD_CONFIG_TERM_VAL(AUX_OUTPUT, aux_output,
+					    term->val.num ? 1 : 0);
 			break;
 		case PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE:
-			ADD_CONFIG_TERM(AUX_SAMPLE_SIZE, aux_sample_size, term->val.num);
+			ADD_CONFIG_TERM_VAL(AUX_SAMPLE_SIZE, aux_sample_size,
+					    term->val.num);
 			break;
 		default:
 			break;
@@ -1322,7 +1340,7 @@ static int get_config_chgs(struct perf_pmu *pmu, struct list_head *head_config,
 	}
 
 	if (bits)
-		ADD_CONFIG_TERM(CFG_CHG, cfg_chg, bits);
+		ADD_CONFIG_TERM_VAL(CFG_CHG, cfg_chg, bits);
 
 #undef ADD_CONFIG_TERM
 	return 0;
-- 
2.21.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/6] perf parse: Copy string to perf_evsel_config_term
  2020-02-01  8:03 [GIT PULL] perf/core improvements and fixes from Budapest Arnaldo Carvalho de Melo
  2020-02-01  8:03 ` [PATCH 1/6] perf parse: Refactor 'struct perf_evsel_config_term' Arnaldo Carvalho de Melo
@ 2020-02-01  8:03 ` Arnaldo Carvalho de Melo
  2020-02-01  8:03 ` [PATCH 3/6] perf test: Fix test case Merge cpu map Arnaldo Carvalho de Melo
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-02-01  8:03 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner
  Cc: Jiri Olsa, Namhyung Kim, Clark Williams, linux-kernel,
	linux-perf-users, Leo Yan, Adrian Hunter, Alexander Shishkin,
	Andi Kleen, Ian Rogers, Mark Rutland, Mathieu Poirier, Mike Leach,
	Peter Zijlstra, Suzuki Poulouse, linux-arm-kernel,
	Arnaldo Carvalho de Melo

From: Leo Yan <leo.yan@linaro.org>

perf with CoreSight fails to record trace data with command:

  perf record -e cs_etm/@tmc_etr0/u --per-thread ls
  failed to set sink "" on event cs_etm/@tmc_etr0/u with 21 (Is a
  directory)/perf/

This failure is root caused with the commit 1dc925568f01 ("perf
parse: Add a deep delete for parse event terms").

The log shows, cs_etm fails to parse the sink attribution; cs_etm event
relies on the event configuration to pass sink name, but the event
specific configuration data cannot be passed properly with flow:

  get_config_terms()
    ADD_CONFIG_TERM(DRV_CFG, term->val.str);
      __t->val.str = term->val.str;
        `> __t->val.str is assigned to term->val.str;

  parse_events_terms__purge()
    parse_events_term__delete()
      zfree(&term->val.str);
        `> term->val.str is freed and assigned to NULL pointer;

  cs_etm_set_sink_attr()
    sink = __t->val.str;
      `> sink string has been freed.

To fix this issue, in the function get_config_terms(), this patch
changes to use strdup() for allocation a new duplicate string rather
than directly assignment string pointer.

This patch addes a new field 'free_str' in the data structure
perf_evsel_config_term; 'free_str' is set to true when the union is used
as a string pointer; thus it can tell perf_evsel__free_config_terms() to
free the string.

Fixes: 1dc925568f01 ("perf parse: Add a deep delete for parse event terms")
Suggested-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lore.kernel.org/lkml/20200117055251.24058-2-leo.yan@linaro.org
[ Use zfree() in perf_evsel__free_config_terms ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

:#	modified:   tools/perf/util/evsel_config.h

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evsel.c        | 2 ++
 tools/perf/util/evsel_config.h | 1 +
 tools/perf/util/parse-events.c | 7 ++++++-
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 549abd43816f..c8dc4450884c 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1265,6 +1265,8 @@ static void perf_evsel__free_config_terms(struct evsel *evsel)
 
 	list_for_each_entry_safe(term, h, &evsel->config_terms, list) {
 		list_del_init(&term->list);
+		if (term->free_str)
+			zfree(&term->val.str);
 		free(term);
 	}
 }
diff --git a/tools/perf/util/evsel_config.h b/tools/perf/util/evsel_config.h
index b4a65201e4f7..e026ab67b008 100644
--- a/tools/perf/util/evsel_config.h
+++ b/tools/perf/util/evsel_config.h
@@ -32,6 +32,7 @@ enum evsel_term_type {
 struct perf_evsel_config_term {
 	struct list_head      list;
 	enum evsel_term_type  type;
+	bool		      free_str;
 	union {
 		u64	      period;
 		u64	      freq;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index f59f3c8da473..c01ba6f8fdad 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1240,7 +1240,12 @@ do {								\
 #define ADD_CONFIG_TERM_STR(__type, __val)			\
 do {								\
 	ADD_CONFIG_TERM(__type);				\
-	__t->val.str = __val;					\
+	__t->val.str = strdup(__val);				\
+	if (!__t->val.str) {					\
+		zfree(&__t);					\
+		return -ENOMEM;					\
+	}							\
+	__t->free_str = true;					\
 } while (0)
 
 	struct parse_events_term *term;
-- 
2.21.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/6] perf test: Fix test case Merge cpu map
  2020-02-01  8:03 [GIT PULL] perf/core improvements and fixes from Budapest Arnaldo Carvalho de Melo
  2020-02-01  8:03 ` [PATCH 1/6] perf parse: Refactor 'struct perf_evsel_config_term' Arnaldo Carvalho de Melo
  2020-02-01  8:03 ` [PATCH 2/6] perf parse: Copy string to perf_evsel_config_term Arnaldo Carvalho de Melo
@ 2020-02-01  8:03 ` Arnaldo Carvalho de Melo
  2020-02-01  8:03 ` [PATCH 4/6] perf: Make perf able to build with latest libbfd Arnaldo Carvalho de Melo
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-02-01  8:03 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner
  Cc: Jiri Olsa, Namhyung Kim, Clark Williams, linux-kernel,
	linux-perf-users, Thomas Richter, Andi Kleen, Heiko Carstens,
	Vasily Gorbik, sumanthk, Arnaldo Carvalho de Melo

From: Thomas Richter <tmricht@linux.ibm.com>

Commit a2408a70368a ("perf evlist: Maintain evlist->all_cpus")
introduces a test case for cpumap merge operation, see functions
perf_cpu_map__merge() and test__cpu_map_merge().

The test case fails on s390 with this error message:

 [root@m35lp76 perf]# ./perf test -Fvvvvv 52
 52: Merge cpu map                                         :
 --- start ---
 cpumask list: 1-2,4-5,7
 perf: /root/linux/tools/include/linux/refcount.h:131:\
          refcount_sub_and_test: Assertion `!(new > val)' failed.
 Aborted (core dumped)
 [root@m35lp76 perf]#

The root cause is in the function test__cpu_map_merge():
It creates two cpu_maps named 'a' and 'b':

  struct perf_cpu_map *a = perf_cpu_map__new("4,2,1");
  struct perf_cpu_map *b = perf_cpu_map__new("4,5,7");

and creates a third map named 'c' which is the result of
the merge of maps a and b:

  struct perf_cpu_map *c = perf_cpu_map__merge(a, b);

After some verifaction of the merged cpu_map all three
of them are have their reference count reduced and are
freed:

   perf_cpu_map__put(a); (1)
   perf_cpu_map__put(b);
   perf_cpu_map__put(c);

The release of perf_cpu_map__put(a) is wrong. The map
is already released and free'ed as part of the function

  perf_cpu_map__merge(struct perf_cpu_map *orig,
  |	              struct perf_cpu_map *other)
  +--> perf_cpu_map__put(orig);
       |
       +--> cpu_map__delete(orig)

At the end perf_cpu_map_put() is called for map 'orig'
alias 'a' and since the reference count is 1, the map
is deleted, as can be seen by the following gdb trace:

 (gdb) where
 #0  tcache_put (tc_idx=0, chunk=0x156cc30) at malloc.c:2940
 #1  _int_free (av=0x3fffd49ee80 <main_arena>, p=0x156cc30,
		     have_lock=<optimized out>) at malloc.c:4222
 #2  0x00000000012d5e78 in cpu_map__delete (map=0x156cc40) at cpumap.c:31
 #3  0x00000000012d5f7a in perf_cpu_map__put (map=0x156cc40) at cpumap.c:45
 #4  0x00000000012d723a in perf_cpu_map__merge (orig=0x156cc40,
     other=0x156cc60) at cpumap.c:343
 #5  0x000000000110cdd0 in test__cpu_map_merge (
     test=0x14ea6c8 <generic_tests+2856>, subtest=-1) at tests/cpumap.c:128

Thus the perf_cpu_map__put(a) (see (1) above) frees map 'a'
a second time and causes the failure. Fix this be removing that
function call.

Output after:
  [root@m35lp76 perf]# ./perf test -Fvvvvv 52
  52: Merge cpu map                                         :
  --- start ---
  cpumask list: 1-2,4-5,7
  ---- end ----
  Merge cpu map: Ok
  [root@m35lp76 perf]#

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Reviewed-by: Andi Kleen <ak@linux.intel.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: sumanthk@linux.ibm.com
Link: http://lore.kernel.org/lkml/20200120132011.64698-1-tmricht@linux.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/cpumap.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/perf/tests/cpumap.c b/tools/perf/tests/cpumap.c
index 4ac56741ac5f..29c793ac7d10 100644
--- a/tools/perf/tests/cpumap.c
+++ b/tools/perf/tests/cpumap.c
@@ -131,7 +131,6 @@ int test__cpu_map_merge(struct test *test __maybe_unused, int subtest __maybe_un
 	TEST_ASSERT_VAL("failed to merge map: bad nr", c->nr == 5);
 	cpu_map__snprint(c, buf, sizeof(buf));
 	TEST_ASSERT_VAL("failed to merge map: bad result", !strcmp(buf, "1-2,4-5,7"));
-	perf_cpu_map__put(a);
 	perf_cpu_map__put(b);
 	perf_cpu_map__put(c);
 	return 0;
-- 
2.21.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/6] perf: Make perf able to build with latest libbfd
  2020-02-01  8:03 [GIT PULL] perf/core improvements and fixes from Budapest Arnaldo Carvalho de Melo
                   ` (2 preceding siblings ...)
  2020-02-01  8:03 ` [PATCH 3/6] perf test: Fix test case Merge cpu map Arnaldo Carvalho de Melo
@ 2020-02-01  8:03 ` Arnaldo Carvalho de Melo
  2020-02-01  8:03 ` [PATCH 5/6] perf probe: Add ustring support for perf probe command Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-02-01  8:03 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner
  Cc: Jiri Olsa, Namhyung Kim, Clark Williams, linux-kernel,
	linux-perf-users, Changbin Du, Jiri Olsa, Peter Zijlstra,
	Arnaldo Carvalho de Melo

From: Changbin Du <changbin.du@gmail.com>

libbfd has changed the bfd_section_* macros to inline functions
bfd_section_<field> since 2019-09-18. See below two commits:
  o http://www.sourceware.org/ml/gdb-cvs/2019-09/msg00064.html
  o https://www.sourceware.org/ml/gdb-cvs/2019-09/msg00072.html

This fix make perf able to build with both old and new libbfd.

Signed-off-by: Changbin Du <changbin.du@gmail.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lore.kernel.org/lkml/20200128152938.31413-1-changbin.du@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/srcline.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
index 6ccf6f6d09df..5b7d6c16d33f 100644
--- a/tools/perf/util/srcline.c
+++ b/tools/perf/util/srcline.c
@@ -193,16 +193,30 @@ static void find_address_in_section(bfd *abfd, asection *section, void *data)
 	bfd_vma pc, vma;
 	bfd_size_type size;
 	struct a2l_data *a2l = data;
+	flagword flags;
 
 	if (a2l->found)
 		return;
 
-	if ((bfd_get_section_flags(abfd, section) & SEC_ALLOC) == 0)
+#ifdef bfd_get_section_flags
+	flags = bfd_get_section_flags(abfd, section);
+#else
+	flags = bfd_section_flags(section);
+#endif
+	if ((flags & SEC_ALLOC) == 0)
 		return;
 
 	pc = a2l->addr;
+#ifdef bfd_get_section_vma
 	vma = bfd_get_section_vma(abfd, section);
+#else
+	vma = bfd_section_vma(section);
+#endif
+#ifdef bfd_get_section_size
 	size = bfd_get_section_size(section);
+#else
+	size = bfd_section_size(section);
+#endif
 
 	if (pc < vma || pc >= vma + size)
 		return;
-- 
2.21.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 5/6] perf probe: Add ustring support for perf probe command
  2020-02-01  8:03 [GIT PULL] perf/core improvements and fixes from Budapest Arnaldo Carvalho de Melo
                   ` (3 preceding siblings ...)
  2020-02-01  8:03 ` [PATCH 4/6] perf: Make perf able to build with latest libbfd Arnaldo Carvalho de Melo
@ 2020-02-01  8:03 ` Arnaldo Carvalho de Melo
  2020-02-01  8:03 ` [PATCH 6/6] perf maps: Add missing unlock to maps__insert() error case Arnaldo Carvalho de Melo
  2020-02-05  7:45 ` [GIT PULL] perf/core improvements and fixes from Budapest Ingo Molnar
  6 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-02-01  8:03 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner
  Cc: Jiri Olsa, Namhyung Kim, Clark Williams, linux-kernel,
	linux-perf-users, Thomas Richter, Heiko Carstens, Vasily Gorbik,
	sumanthk, Arnaldo Carvalho de Melo

From: Thomas Richter <tmricht@linux.ibm.com>

Kernel commit 88903c464321 ("tracing/probe: Add ustring type for user-space string")
adds support for user-space strings when type 'ustring' is specified.

Here is an example using sysfs command line interface
for kprobes:

Function to probe:
  struct filename *
  getname_flags(const char __user *filename, int flags, int *empty)

Setup:
  # cd /sys/kernel/debug/tracing/
  # echo 'p:tmr1 getname_flags +0(%r2):ustring' > kprobe_events
  # cat events/kprobes/tmr1/format | fgrep print
  print fmt: "(%lx) arg1=\"%s\"", REC->__probe_ip, REC->arg1
  # echo 1 > events/kprobes/tmr1/enable
  # touch /tmp/111
  # echo 0 > events/kprobes/tmr1/enable
  # cat trace|fgrep /tmp/111
  touch-5846  [005] d..2 255520.717960: tmr1:\
	  (getname_flags+0x0/0x400) arg1="/tmp/111"

Doing the same with the perf tool fails.
Using type 'string' succeeds:
 # perf probe "vfs_getname=getname_flags:72 pathname=filename:string"
 Added new event:
   probe:vfs_getname (on getname_flags:72 with pathname=filename:string)
   ....
 # perf probe -d probe:vfs_getname
 Removed event: probe:vfs_getname

However using type 'ustring' fails (output before):
 # perf probe "vfs_getname=getname_flags:72 pathname=filename:ustring"
 Failed to write event: Invalid argument
   Error: Failed to add events.
 #

Fix this by adding type 'ustring' in function
convert_variable_type().

Using ustring succeeds (output after):
 # ./perf probe "vfs_getname=getname_flags:72 pathname=filename:ustring"
 Added new event:
   probe:vfs_getname (on getname_flags:72 with pathname=filename:ustring)

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

	perf record -e probe:vfs_getname -aR sleep 1

 #

Note: This issue also exists on x86, it is not s390 specific.

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: sumanthk@linux.ibm.com
Link: http://lore.kernel.org/lkml/20200120132011.64698-2-tmricht@linux.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-finder.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index c470c49a804f..1c817add6ca4 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -303,7 +303,8 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 	char prefix;
 
 	/* TODO: check all types */
-	if (cast && strcmp(cast, "string") != 0 && strcmp(cast, "x") != 0 &&
+	if (cast && strcmp(cast, "string") != 0 && strcmp(cast, "ustring") &&
+	    strcmp(cast, "x") != 0 &&
 	    strcmp(cast, "s") != 0 && strcmp(cast, "u") != 0) {
 		/* Non string type is OK */
 		/* and respect signedness/hexadecimal cast */
-- 
2.21.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 6/6] perf maps: Add missing unlock to maps__insert() error case
  2020-02-01  8:03 [GIT PULL] perf/core improvements and fixes from Budapest Arnaldo Carvalho de Melo
                   ` (4 preceding siblings ...)
  2020-02-01  8:03 ` [PATCH 5/6] perf probe: Add ustring support for perf probe command Arnaldo Carvalho de Melo
@ 2020-02-01  8:03 ` Arnaldo Carvalho de Melo
  2020-02-05  7:45 ` [GIT PULL] perf/core improvements and fixes from Budapest Ingo Molnar
  6 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-02-01  8:03 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner
  Cc: Jiri Olsa, Namhyung Kim, Clark Williams, linux-kernel,
	linux-perf-users, Cengiz Can, Adrian Hunter,
	Arnaldo Carvalho de Melo

From: Cengiz Can <cengiz@kernel.wtf>

`tools/perf/util/map.c` has a function named `maps__insert` that
acquires a write lock if its in multithread context.

Even though this lock is released when function successfully completes,
there's a branch that is executed when `maps_by_name == NULL` that
returns from this function without releasing the write lock.

Added an `up_write` to release the lock when this happens.

Fixes: a7c2b572e217 ("perf map_groups: Auto sort maps by name, if needed")
Signed-off-by: Cengiz Can <cengiz@kernel.wtf>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lore.kernel.org/lkml/20200120141553.23934-1-cengiz@kernel.wtf
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/map.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index fdd5bddb3075..f67960bedebb 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -549,6 +549,7 @@ void maps__insert(struct maps *maps, struct map *map)
 
 			if (maps_by_name == NULL) {
 				__maps__free_maps_by_name(maps);
+				up_write(&maps->lock);
 				return;
 			}
 
-- 
2.21.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [GIT PULL] perf/core improvements and fixes from Budapest
  2020-02-01  8:03 [GIT PULL] perf/core improvements and fixes from Budapest Arnaldo Carvalho de Melo
                   ` (5 preceding siblings ...)
  2020-02-01  8:03 ` [PATCH 6/6] perf maps: Add missing unlock to maps__insert() error case Arnaldo Carvalho de Melo
@ 2020-02-05  7:45 ` Ingo Molnar
  6 siblings, 0 replies; 8+ messages in thread
From: Ingo Molnar @ 2020-02-05  7:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Thomas Gleixner, Jiri Olsa, Namhyung Kim, Clark Williams,
	linux-kernel, linux-perf-users, Cengiz Can, Changbin Du, Leo Yan,
	Thomas Richter, Arnaldo Carvalho de Melo


* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Hi Ingo/Thomas,
> 
> 	Please consider pulling,
> 
> Best regards,
> 
> - Arnaldo
> 
> The following changes since commit 0cc4bd8f70d1ea2940295f1050508c663fe9eff9:
> 
>   Merge branch 'core/kprobes' into perf/core, to pick up fixes (2020-01-28 07:59:05 +0100)
> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-core-for-mingo-5.6-20200201
> 
> for you to fetch changes up to 85fc95d75970ee7dd8e01904e7fb1197c275ba6b:
> 
>   perf maps: Add missing unlock to maps__insert() error case (2020-01-31 09:40:50 +0100)
> 
> ----------------------------------------------------------------
> perf/core improvements and fixes:
> 
> perf maps:
> 
>   Cengiz Can:
> 
>   - Add missing unlock to maps__insert() error case.
> 
> srcline:
> 
>   Changbin Du:
> 
>   - Make perf able to build with latest libbfd.
> 
> perf parse:
> 
>   Leo Yan:
> 
>   - Keep copy of string in perf_evsel_config_term() to fix sink terms
>     processing in ARM CoreSight.
> 
> perf test:
> 
>   Thomas Richter:
> 
>   - Fix test case Merge cpu map, removing extra reference count drop that
>     causes a segfault on s/390.
> 
> perf probe:
> 
>   Thomas Richter:
> 
>   - Add ustring support for perf probe command
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Cengiz Can (1):
>       perf maps: Add missing unlock to maps__insert() error case
> 
> Changbin Du (1):
>       perf: Make perf able to build with latest libbfd
> 
> Leo Yan (2):
>       perf parse: Refactor 'struct perf_evsel_config_term'
>       perf parse: Copy string to perf_evsel_config_term
> 
> Thomas Richter (2):
>       perf test: Fix test case Merge cpu map
>       perf probe: Add ustring support for perf probe command
> 
>  tools/perf/arch/arm/util/cs-etm.c |  2 +-
>  tools/perf/tests/cpumap.c         |  1 -
>  tools/perf/util/evsel.c           |  8 +++--
>  tools/perf/util/evsel_config.h    |  5 ++-
>  tools/perf/util/map.c             |  1 +
>  tools/perf/util/parse-events.c    | 67 ++++++++++++++++++++++++++-------------
>  tools/perf/util/probe-finder.c    |  3 +-
>  tools/perf/util/srcline.c         | 16 +++++++++-
>  8 files changed, 71 insertions(+), 32 deletions(-)

Pulled, thanks a lot Arnaldo!

	Ingo

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-02-05  7:45 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-01  8:03 [GIT PULL] perf/core improvements and fixes from Budapest Arnaldo Carvalho de Melo
2020-02-01  8:03 ` [PATCH 1/6] perf parse: Refactor 'struct perf_evsel_config_term' Arnaldo Carvalho de Melo
2020-02-01  8:03 ` [PATCH 2/6] perf parse: Copy string to perf_evsel_config_term Arnaldo Carvalho de Melo
2020-02-01  8:03 ` [PATCH 3/6] perf test: Fix test case Merge cpu map Arnaldo Carvalho de Melo
2020-02-01  8:03 ` [PATCH 4/6] perf: Make perf able to build with latest libbfd Arnaldo Carvalho de Melo
2020-02-01  8:03 ` [PATCH 5/6] perf probe: Add ustring support for perf probe command Arnaldo Carvalho de Melo
2020-02-01  8:03 ` [PATCH 6/6] perf maps: Add missing unlock to maps__insert() error case Arnaldo Carvalho de Melo
2020-02-05  7:45 ` [GIT PULL] perf/core improvements and fixes from Budapest Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).