* [PATCH 2/2] perf, tools, record: Fix -c/-F options for cpu event aliases
2017-10-20 20:27 [PATCH 1/2] perf, tools: Enable type checking for perf_evsel_config_term types Andi Kleen
@ 2017-10-20 20:27 ` Andi Kleen
2017-10-24 7:39 ` Jiri Olsa
` (2 more replies)
2017-10-24 7:38 ` [PATCH 1/2] perf, tools: Enable type checking for perf_evsel_config_term types Jiri Olsa
2017-11-18 8:25 ` [tip:perf/core] perf evsel: " tip-bot for Andi Kleen
2 siblings, 3 replies; 8+ messages in thread
From: Andi Kleen @ 2017-10-20 20:27 UTC (permalink / raw)
To: acme; +Cc: jolsa, linux-kernel, Andi Kleen
From: Andi Kleen <ak@linux.intel.com>
The Intel PMU event aliases have a implicit
period= specifier to set the default period.
Unfortunately this breaks overriding these periods with -c or
-F, because the alias terms look like they are user specified
to the internal parser, and user specified event qualifiers override
the command line options.
Track that they are coming from aliases by adding a
"weak" state to the term. Any weak terms don't override
command line options.
I only did it for -c/-F for now, I think that's the
only case that's broken currently.
Before:
$ perf record -c 1000 -vv -e uops_issued.any
...
{ sample_period, sample_freq } 2000003
After:
$ perf record -c 1000 -vv -e uops_issued.any
...
{ sample_period, sample_freq } 1000
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
tools/perf/util/evsel.c | 12 ++++++++----
tools/perf/util/evsel.h | 1 +
tools/perf/util/parse-events.c | 2 ++
tools/perf/util/parse-events.h | 3 +++
tools/perf/util/pmu.c | 5 +++++
5 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d1f63b93bf69..4376cdfaea49 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -733,12 +733,16 @@ static void apply_config_terms(struct perf_evsel *evsel,
list_for_each_entry(term, config_terms, list) {
switch (term->type) {
case PERF_EVSEL__CONFIG_TERM_PERIOD:
- attr->sample_period = term->val.period;
- attr->freq = 0;
+ if (!(term->weak && opts->user_interval != ULLONG_MAX)) {
+ attr->sample_period = term->val.period;
+ attr->freq = 0;
+ }
break;
case PERF_EVSEL__CONFIG_TERM_FREQ:
- attr->sample_freq = term->val.freq;
- attr->freq = 1;
+ if (!(term->weak && opts->user_freq != UINT_MAX)) {
+ attr->sample_freq = term->val.freq;
+ attr->freq = 1;
+ }
break;
case PERF_EVSEL__CONFIG_TERM_TIME:
if (term->val.time)
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 6728d8d6e513..e315f91739bc 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -65,6 +65,7 @@ struct perf_evsel_config_term {
bool overwrite;
char *branch;
} val;
+ bool weak;
};
/** struct perf_evsel - event selector
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 04f35db063ee..c773cdd9aa4b 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1115,6 +1115,7 @@ 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); \
} while (0)
@@ -2409,6 +2410,7 @@ static int new_term(struct parse_events_term **_term,
*term = *temp;
INIT_LIST_HEAD(&term->list);
+ term->weak = false;
switch (term->type_val) {
case PARSE_EVENTS__TERM_TYPE_NUM:
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 3909ca0639f2..e5638f06b431 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -100,6 +100,9 @@ struct parse_events_term {
/* error string indexes for within parsed string */
int err_term;
int err_val;
+
+ /* Coming from implicit alias */
+ bool weak;
};
struct parse_events_error {
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 94cf2c29fed6..2b12bcb845a8 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -404,6 +404,11 @@ static int pmu_alias_terms(struct perf_pmu_alias *alias,
parse_events_terms__purge(&list);
return ret;
}
+ /*
+ * Weak terms don't override command line options,
+ * which we don't want for implicit terms in aliases.
+ */
+ cloned->weak = true;
list_add_tail(&cloned->list, &list);
}
list_splice(&list, terms);
--
2.13.6
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 2/2] perf, tools, record: Fix -c/-F options for cpu event aliases
2017-10-20 20:27 ` [PATCH 2/2] perf, tools, record: Fix -c/-F options for cpu event aliases Andi Kleen
@ 2017-10-24 7:39 ` Jiri Olsa
2017-11-18 8:25 ` [tip:perf/core] perf " tip-bot for Andi Kleen
2017-11-29 6:28 ` tip-bot for Andi Kleen
2 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2017-10-24 7:39 UTC (permalink / raw)
To: Andi Kleen; +Cc: acme, jolsa, linux-kernel, Andi Kleen
On Fri, Oct 20, 2017 at 01:27:55PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> The Intel PMU event aliases have a implicit
> period= specifier to set the default period.
>
> Unfortunately this breaks overriding these periods with -c or
> -F, because the alias terms look like they are user specified
> to the internal parser, and user specified event qualifiers override
> the command line options.
>
> Track that they are coming from aliases by adding a
> "weak" state to the term. Any weak terms don't override
> command line options.
>
> I only did it for -c/-F for now, I think that's the
> only case that's broken currently.
>
> Before:
>
> $ perf record -c 1000 -vv -e uops_issued.any
> ...
> { sample_period, sample_freq } 2000003
>
> After:
>
> $ perf record -c 1000 -vv -e uops_issued.any
> ...
> { sample_period, sample_freq } 1000
>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
thanks,
jirka
^ permalink raw reply [flat|nested] 8+ messages in thread* [tip:perf/core] perf record: Fix -c/-F options for cpu event aliases
2017-10-20 20:27 ` [PATCH 2/2] perf, tools, record: Fix -c/-F options for cpu event aliases Andi Kleen
2017-10-24 7:39 ` Jiri Olsa
@ 2017-11-18 8:25 ` tip-bot for Andi Kleen
2017-11-29 6:28 ` tip-bot for Andi Kleen
2 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Andi Kleen @ 2017-11-18 8:25 UTC (permalink / raw)
To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, ak, tglx, acme, jolsa
Commit-ID: c2f1cead19b628d7a23d2cfc43e444af669f9eab
Gitweb: https://git.kernel.org/tip/c2f1cead19b628d7a23d2cfc43e444af669f9eab
Author: Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 20 Oct 2017 13:27:55 -0700
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 16 Nov 2017 14:49:51 -0300
perf record: Fix -c/-F options for cpu event aliases
The Intel PMU event aliases have a implicit period= specifier to set the
default period.
Unfortunately this breaks overriding these periods with -c or -F,
because the alias terms look like they are user specified to the
internal parser, and user specified event qualifiers override the
command line options.
Track that they are coming from aliases by adding a "weak" state to the
term. Any weak terms don't override command line options.
I only did it for -c/-F for now, I think that's the only case that's
broken currently.
Before:
$ perf record -c 1000 -vv -e uops_issued.any
...
{ sample_period, sample_freq } 2000003
After:
$ perf record -c 1000 -vv -e uops_issued.any
...
{ sample_period, sample_freq } 1000
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20171020202755.21410-2-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/evsel.c | 12 ++++++++----
tools/perf/util/evsel.h | 1 +
tools/perf/util/parse-events.c | 2 ++
tools/perf/util/parse-events.h | 3 +++
tools/perf/util/pmu.c | 5 +++++
5 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index f894893..bfde6a7 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -733,12 +733,16 @@ static void apply_config_terms(struct perf_evsel *evsel,
list_for_each_entry(term, config_terms, list) {
switch (term->type) {
case PERF_EVSEL__CONFIG_TERM_PERIOD:
- attr->sample_period = term->val.period;
- attr->freq = 0;
+ if (!(term->weak && opts->user_interval != ULLONG_MAX)) {
+ attr->sample_period = term->val.period;
+ attr->freq = 0;
+ }
break;
case PERF_EVSEL__CONFIG_TERM_FREQ:
- attr->sample_freq = term->val.freq;
- attr->freq = 1;
+ if (!(term->weak && opts->user_freq != UINT_MAX)) {
+ attr->sample_freq = term->val.freq;
+ attr->freq = 1;
+ }
break;
case PERF_EVSEL__CONFIG_TERM_TIME:
if (term->val.time)
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 9277df9..157f49e 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -67,6 +67,7 @@ struct perf_evsel_config_term {
bool overwrite;
char *branch;
} val;
+ bool weak;
};
struct perf_stat_evsel;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index a7fcd95..1703167 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1116,6 +1116,7 @@ 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); \
} while (0)
@@ -2410,6 +2411,7 @@ static int new_term(struct parse_events_term **_term,
*term = *temp;
INIT_LIST_HEAD(&term->list);
+ term->weak = false;
switch (term->type_val) {
case PARSE_EVENTS__TERM_TYPE_NUM:
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index be337c2..88108cd 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -101,6 +101,9 @@ struct parse_events_term {
/* error string indexes for within parsed string */
int err_term;
int err_val;
+
+ /* Coming from implicit alias */
+ bool weak;
};
struct parse_events_error {
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 07cb2ac..80fb159 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -405,6 +405,11 @@ static int pmu_alias_terms(struct perf_pmu_alias *alias,
parse_events_terms__purge(&list);
return ret;
}
+ /*
+ * Weak terms don't override command line options,
+ * which we don't want for implicit terms in aliases.
+ */
+ cloned->weak = true;
list_add_tail(&cloned->list, &list);
}
list_splice(&list, terms);
^ permalink raw reply related [flat|nested] 8+ messages in thread* [tip:perf/core] perf record: Fix -c/-F options for cpu event aliases
2017-10-20 20:27 ` [PATCH 2/2] perf, tools, record: Fix -c/-F options for cpu event aliases Andi Kleen
2017-10-24 7:39 ` Jiri Olsa
2017-11-18 8:25 ` [tip:perf/core] perf " tip-bot for Andi Kleen
@ 2017-11-29 6:28 ` tip-bot for Andi Kleen
2 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Andi Kleen @ 2017-11-29 6:28 UTC (permalink / raw)
To: linux-tip-commits; +Cc: jolsa, tglx, mingo, linux-kernel, acme, hpa, ak
Commit-ID: 59622fd496a3175c7bf549046e091d81c303ecff
Gitweb: https://git.kernel.org/tip/59622fd496a3175c7bf549046e091d81c303ecff
Author: Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 20 Oct 2017 13:27:55 -0700
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 28 Nov 2017 14:19:39 -0300
perf record: Fix -c/-F options for cpu event aliases
The Intel PMU event aliases have a implicit period= specifier to set the
default period.
Unfortunately this breaks overriding these periods with -c or -F,
because the alias terms look like they are user specified to the
internal parser, and user specified event qualifiers override the
command line options.
Track that they are coming from aliases by adding a "weak" state to the
term. Any weak terms don't override command line options.
I only did it for -c/-F for now, I think that's the only case that's
broken currently.
Before:
$ perf record -c 1000 -vv -e uops_issued.any
...
{ sample_period, sample_freq } 2000003
After:
$ perf record -c 1000 -vv -e uops_issued.any
...
{ sample_period, sample_freq } 1000
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20171020202755.21410-2-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/evsel.c | 12 ++++++++----
tools/perf/util/evsel.h | 1 +
tools/perf/util/parse-events.c | 2 ++
tools/perf/util/parse-events.h | 3 +++
tools/perf/util/pmu.c | 5 +++++
5 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index f894893..bfde6a7 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -733,12 +733,16 @@ static void apply_config_terms(struct perf_evsel *evsel,
list_for_each_entry(term, config_terms, list) {
switch (term->type) {
case PERF_EVSEL__CONFIG_TERM_PERIOD:
- attr->sample_period = term->val.period;
- attr->freq = 0;
+ if (!(term->weak && opts->user_interval != ULLONG_MAX)) {
+ attr->sample_period = term->val.period;
+ attr->freq = 0;
+ }
break;
case PERF_EVSEL__CONFIG_TERM_FREQ:
- attr->sample_freq = term->val.freq;
- attr->freq = 1;
+ if (!(term->weak && opts->user_freq != UINT_MAX)) {
+ attr->sample_freq = term->val.freq;
+ attr->freq = 1;
+ }
break;
case PERF_EVSEL__CONFIG_TERM_TIME:
if (term->val.time)
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 9277df9..157f49e 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -67,6 +67,7 @@ struct perf_evsel_config_term {
bool overwrite;
char *branch;
} val;
+ bool weak;
};
struct perf_stat_evsel;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index a7fcd95..1703167 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1116,6 +1116,7 @@ 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); \
} while (0)
@@ -2410,6 +2411,7 @@ static int new_term(struct parse_events_term **_term,
*term = *temp;
INIT_LIST_HEAD(&term->list);
+ term->weak = false;
switch (term->type_val) {
case PARSE_EVENTS__TERM_TYPE_NUM:
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index be337c2..88108cd 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -101,6 +101,9 @@ struct parse_events_term {
/* error string indexes for within parsed string */
int err_term;
int err_val;
+
+ /* Coming from implicit alias */
+ bool weak;
};
struct parse_events_error {
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 07cb2ac..80fb159 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -405,6 +405,11 @@ static int pmu_alias_terms(struct perf_pmu_alias *alias,
parse_events_terms__purge(&list);
return ret;
}
+ /*
+ * Weak terms don't override command line options,
+ * which we don't want for implicit terms in aliases.
+ */
+ cloned->weak = true;
list_add_tail(&cloned->list, &list);
}
list_splice(&list, terms);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] perf, tools: Enable type checking for perf_evsel_config_term types
2017-10-20 20:27 [PATCH 1/2] perf, tools: Enable type checking for perf_evsel_config_term types Andi Kleen
2017-10-20 20:27 ` [PATCH 2/2] perf, tools, record: Fix -c/-F options for cpu event aliases Andi Kleen
@ 2017-10-24 7:38 ` Jiri Olsa
2017-11-08 15:40 ` Arnaldo Carvalho de Melo
2017-11-18 8:25 ` [tip:perf/core] perf evsel: " tip-bot for Andi Kleen
2 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2017-10-24 7:38 UTC (permalink / raw)
To: Andi Kleen; +Cc: acme, jolsa, linux-kernel, Andi Kleen
On Fri, Oct 20, 2017 at 01:27:54PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> Use a typed enum for the perf_evsel_config_term type enum.
> This allows gcc to do much stronger type checks, and also
> check for missing case statements.
>
> I removed the unused _MAX member from the number.
>
> It found one missing case. I'm not sure it's a real problem,
> so I just turned it into a BUG_ON for now.
>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
> tools/perf/util/evsel.c | 2 ++
> tools/perf/util/evsel.h | 5 ++---
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index f894893c203d..d1f63b93bf69 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -775,6 +775,8 @@ static void apply_config_terms(struct perf_evsel *evsel,
> case PERF_EVSEL__CONFIG_TERM_OVERWRITE:
> attr->write_backward = term->val.overwrite ? 1 : 0;
> break;
> + case PERF_EVSEL__CONFIG_TERM_DRV_CFG:
> + BUG_ON(1);
> default:
> break;
> }
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index db658785d828..6728d8d6e513 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -37,7 +37,7 @@ struct cgroup_sel;
> * It is allocated within event parsing and attached to
> * perf_evsel::config_terms list head.
> */
> -enum {
> +enum period_type {
seems ok, but I'm puzzled with the 'period_type' name
I think it should be more like 'term_type' or something
jirka
> PERF_EVSEL__CONFIG_TERM_PERIOD,
> PERF_EVSEL__CONFIG_TERM_FREQ,
> PERF_EVSEL__CONFIG_TERM_TIME,
> @@ -48,12 +48,11 @@ enum {
> PERF_EVSEL__CONFIG_TERM_OVERWRITE,
> PERF_EVSEL__CONFIG_TERM_DRV_CFG,
> PERF_EVSEL__CONFIG_TERM_BRANCH,
> - PERF_EVSEL__CONFIG_TERM_MAX,
> };
>
> struct perf_evsel_config_term {
> struct list_head list;
> - int type;
> + enum period_type type;
> union {
> u64 period;
> u64 freq;
> --
> 2.13.6
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 1/2] perf, tools: Enable type checking for perf_evsel_config_term types
2017-10-24 7:38 ` [PATCH 1/2] perf, tools: Enable type checking for perf_evsel_config_term types Jiri Olsa
@ 2017-11-08 15:40 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-11-08 15:40 UTC (permalink / raw)
To: Jiri Olsa; +Cc: Andi Kleen, jolsa, linux-kernel, Andi Kleen
Em Tue, Oct 24, 2017 at 09:38:49AM +0200, Jiri Olsa escreveu:
> On Fri, Oct 20, 2017 at 01:27:54PM -0700, Andi Kleen wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> >
> > Use a typed enum for the perf_evsel_config_term type enum.
> > This allows gcc to do much stronger type checks, and also
> > check for missing case statements.
> >
> > I removed the unused _MAX member from the number.
> >
> > It found one missing case. I'm not sure it's a real problem,
> > so I just turned it into a BUG_ON for now.
> >
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> > ---
> > tools/perf/util/evsel.c | 2 ++
> > tools/perf/util/evsel.h | 5 ++---
> > 2 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > index f894893c203d..d1f63b93bf69 100644
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> > @@ -775,6 +775,8 @@ static void apply_config_terms(struct perf_evsel *evsel,
> > case PERF_EVSEL__CONFIG_TERM_OVERWRITE:
> > attr->write_backward = term->val.overwrite ? 1 : 0;
> > break;
> > + case PERF_EVSEL__CONFIG_TERM_DRV_CFG:
> > + BUG_ON(1);
> > default:
> > break;
> > }
> > diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> > index db658785d828..6728d8d6e513 100644
> > --- a/tools/perf/util/evsel.h
> > +++ b/tools/perf/util/evsel.h
> > @@ -37,7 +37,7 @@ struct cgroup_sel;
> > * It is allocated within event parsing and attached to
> > * perf_evsel::config_terms list head.
> > */
> > -enum {
> > +enum period_type {
>
> seems ok, but I'm puzzled with the 'period_type' name
> I think it should be more like 'term_type' or something
Ok, renamed it to term_type and added your Acked-by,
- ARnaldo
> jirka
>
>
> > PERF_EVSEL__CONFIG_TERM_PERIOD,
> > PERF_EVSEL__CONFIG_TERM_FREQ,
> > PERF_EVSEL__CONFIG_TERM_TIME,
> > @@ -48,12 +48,11 @@ enum {
> > PERF_EVSEL__CONFIG_TERM_OVERWRITE,
> > PERF_EVSEL__CONFIG_TERM_DRV_CFG,
> > PERF_EVSEL__CONFIG_TERM_BRANCH,
> > - PERF_EVSEL__CONFIG_TERM_MAX,
> > };
> >
> > struct perf_evsel_config_term {
> > struct list_head list;
> > - int type;
> > + enum period_type type;
> > union {
> > u64 period;
> > u64 freq;
> > --
> > 2.13.6
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip:perf/core] perf evsel: Enable type checking for perf_evsel_config_term types
2017-10-20 20:27 [PATCH 1/2] perf, tools: Enable type checking for perf_evsel_config_term types Andi Kleen
2017-10-20 20:27 ` [PATCH 2/2] perf, tools, record: Fix -c/-F options for cpu event aliases Andi Kleen
2017-10-24 7:38 ` [PATCH 1/2] perf, tools: Enable type checking for perf_evsel_config_term types Jiri Olsa
@ 2017-11-18 8:25 ` tip-bot for Andi Kleen
2 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Andi Kleen @ 2017-11-18 8:25 UTC (permalink / raw)
To: linux-tip-commits; +Cc: hpa, acme, mingo, tglx, ak, linux-kernel, jolsa
Commit-ID: d0565132605f454179699a1b8a3276fc0f8cc87b
Gitweb: https://git.kernel.org/tip/d0565132605f454179699a1b8a3276fc0f8cc87b
Author: Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 20 Oct 2017 13:27:54 -0700
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 16 Nov 2017 14:49:51 -0300
perf evsel: Enable type checking for perf_evsel_config_term types
Use a typed enum for the perf_evsel_config_term type enum. This allows
gcc to do much stronger type checks, and also check for missing case
statements.
I removed the unused _MAX member from the number.
It found one missing case. I'm not sure it's a real problem, so I just
turned it into a BUG_ON for now.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20171020202755.21410-1-andi@firstfloor.org
[ Renamed the enum name to term_type as per jolsa's request ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/evsel.c | 2 ++
tools/perf/util/evsel.h | 5 ++---
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index bfde6a7..4376cdf 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -779,6 +779,8 @@ static void apply_config_terms(struct perf_evsel *evsel,
case PERF_EVSEL__CONFIG_TERM_OVERWRITE:
attr->write_backward = term->val.overwrite ? 1 : 0;
break;
+ case PERF_EVSEL__CONFIG_TERM_DRV_CFG:
+ BUG_ON(1);
default:
break;
}
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 157f49e..0688880 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -38,7 +38,7 @@ struct cgroup_sel;
* It is allocated within event parsing and attached to
* perf_evsel::config_terms list head.
*/
-enum {
+enum term_type {
PERF_EVSEL__CONFIG_TERM_PERIOD,
PERF_EVSEL__CONFIG_TERM_FREQ,
PERF_EVSEL__CONFIG_TERM_TIME,
@@ -49,12 +49,11 @@ enum {
PERF_EVSEL__CONFIG_TERM_OVERWRITE,
PERF_EVSEL__CONFIG_TERM_DRV_CFG,
PERF_EVSEL__CONFIG_TERM_BRANCH,
- PERF_EVSEL__CONFIG_TERM_MAX,
};
struct perf_evsel_config_term {
struct list_head list;
- int type;
+ enum term_type type;
union {
u64 period;
u64 freq;
^ permalink raw reply related [flat|nested] 8+ messages in thread