* [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 ` 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; 11+ 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, Andi Kleen, Adrian Hunter,
Alexander Shishkin, 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>
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] 11+ messages in thread* [PATCH 1/6] perf parse: Refactor 'struct perf_evsel_config_term'
@ 2020-02-01 8:03 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 11+ 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 1/6] perf parse: Refactor 'struct perf_evsel_config_term'
@ 2020-02-01 8:03 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 11+ 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] 11+ 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 ` Arnaldo Carvalho de Melo
2020-02-01 8:03 ` Arnaldo Carvalho de Melo
` (5 subsequent siblings)
6 siblings, 0 replies; 11+ 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] 11+ messages in thread* [PATCH 2/6] perf parse: Copy string to perf_evsel_config_term
@ 2020-02-01 8:03 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 11+ 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>
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 11+ 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 ` Arnaldo Carvalho de Melo
2020-02-01 8:03 ` 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ messages in thread