Deprecate the --cputype option and add a --unit option where '--unit
cpu_atom' behaves like '--cputype atom'. The --unit option can be used
with arbitrary PMUs, for example:
```
$ perf list --unit msr pmu
List of pre-defined events (to be used in -e or -M):
msr/aperf/ [Kernel PMU event]
msr/cpu_thermal_margin/ [Kernel PMU event]
msr/mperf/ [Kernel PMU event]
msr/pperf/ [Kernel PMU event]
msr/smi/ [Kernel PMU event]
msr/tsc/ [Kernel PMU event]
```
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/Documentation/perf-list.txt | 6 +++---
tools/perf/builtin-list.c | 18 ++++++++++++------
tools/perf/util/metricgroup.c | 3 ++-
tools/perf/util/pmu.c | 4 +---
4 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index 57384a97c04f..44a819af573d 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -39,9 +39,9 @@ any extra expressions computed by perf stat.
--deprecated::
Print deprecated events. By default the deprecated events are hidden.
---cputype::
-Print events applying cpu with this type for hybrid platform
-(e.g. --cputype core or --cputype atom)
+--unit::
+Print PMU events and metrics limited to the specific PMU name.
+(e.g. --unit cpu, --unit msr, --unit cpu_core, --unit cpu_atom)
[[EVENT_MODIFIERS]]
EVENT MODIFIERS
diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 58e1ec1654ef..cc84ced6da26 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -21,7 +21,6 @@
static bool desc_flag = true;
static bool details_flag;
-static const char *hybrid_type;
int cmd_list(int argc, const char **argv)
{
@@ -30,6 +29,8 @@ int cmd_list(int argc, const char **argv)
bool long_desc_flag = false;
bool deprecated = false;
char *pmu_name = NULL;
+ const char *hybrid_name = NULL;
+ const char *unit_name = NULL;
struct option list_options[] = {
OPT_BOOLEAN(0, "raw-dump", &raw_dump, "Dump raw events"),
OPT_BOOLEAN('d', "desc", &desc_flag,
@@ -40,9 +41,10 @@ int cmd_list(int argc, const char **argv)
"Print information on the perf event names and expressions used internally by events."),
OPT_BOOLEAN(0, "deprecated", &deprecated,
"Print deprecated events."),
- OPT_STRING(0, "cputype", &hybrid_type, "hybrid cpu type",
- "Print events applying cpu with this type for hybrid platform "
- "(e.g. core or atom)"),
+ OPT_STRING(0, "cputype", &hybrid_name, "hybrid cpu type",
+ "Limit PMU or metric printing to the given hybrid PMU (e.g. core or atom)."),
+ OPT_STRING(0, "unit", &unit_name, "PMU name",
+ "Limit PMU or metric printing to the specified PMU."),
OPT_INCR(0, "debug", &verbose,
"Enable debugging output"),
OPT_END()
@@ -53,6 +55,8 @@ int cmd_list(int argc, const char **argv)
};
set_option_flag(list_options, 0, "raw-dump", PARSE_OPT_HIDDEN);
+ /* Hide hybrid flag for the more generic 'unit' flag. */
+ set_option_flag(list_options, 0, "cputype", PARSE_OPT_HIDDEN);
argc = parse_options(argc, argv, list_options, list_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
@@ -62,8 +66,10 @@ int cmd_list(int argc, const char **argv)
if (!raw_dump && pager_in_use())
printf("\nList of pre-defined events (to be used in -e or -M):\n\n");
- if (hybrid_type) {
- pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_type);
+ if (unit_name)
+ pmu_name = strdup(unit_name);
+ else if (hybrid_name) {
+ pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_name);
if (!pmu_name)
pr_warning("WARNING: hybrid cputype is not supported!\n");
}
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 4c98ac29ee13..1943fed9b6d9 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -556,11 +556,12 @@ static int metricgroup__print_callback(const struct pmu_event *pe,
void *vdata)
{
struct metricgroup_print_data *data = vdata;
+ const char *pmu = pe->pmu ?: "cpu";
if (!pe->metric_expr)
return 0;
- if (data->pmu_name && perf_pmu__is_hybrid(pe->pmu) && strcmp(data->pmu_name, pe->pmu))
+ if (data->pmu_name && strcmp(data->pmu_name, pmu))
return 0;
return metricgroup__print_pmu_event(pe, data->metricgroups, data->filter,
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index a8f9f47c6ed9..9c771f136b81 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1694,10 +1694,8 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
pmu = NULL;
j = 0;
while ((pmu = perf_pmu__scan(pmu)) != NULL) {
- if (pmu_name && perf_pmu__is_hybrid(pmu->name) &&
- strcmp(pmu_name, pmu->name)) {
+ if (pmu_name && pmu->name && strcmp(pmu_name, pmu->name))
continue;
- }
list_for_each_entry(alias, &pmu->aliases, list) {
char *name = alias->desc ? alias->name :
--
2.38.1.431.g37b22c650d-goog
On 2022-11-14 2:51 a.m., Ian Rogers wrote:
> Deprecate the --cputype option and add a --unit option where '--unit
> cpu_atom' behaves like '--cputype atom'. The --unit option can be used
> with arbitrary PMUs, for example:
>
> ```
> $ perf list --unit msr pmu
>
> List of pre-defined events (to be used in -e or -M):
>
> msr/aperf/ [Kernel PMU event]
> msr/cpu_thermal_margin/ [Kernel PMU event]
> msr/mperf/ [Kernel PMU event]
> msr/pperf/ [Kernel PMU event]
> msr/smi/ [Kernel PMU event]
> msr/tsc/ [Kernel PMU event]
> ```
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/Documentation/perf-list.txt | 6 +++---
> tools/perf/builtin-list.c | 18 ++++++++++++------
> tools/perf/util/metricgroup.c | 3 ++-
> tools/perf/util/pmu.c | 4 +---
> 4 files changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
> index 57384a97c04f..44a819af573d 100644
> --- a/tools/perf/Documentation/perf-list.txt
> +++ b/tools/perf/Documentation/perf-list.txt
> @@ -39,9 +39,9 @@ any extra expressions computed by perf stat.
> --deprecated::
> Print deprecated events. By default the deprecated events are hidden.
>
> ---cputype::
> -Print events applying cpu with this type for hybrid platform
> -(e.g. --cputype core or --cputype atom)
The "--cputype" is removed from the documentation, but the code is still
available. It sounds weird.
Can we still keep the "--cputype" in the documentation? Just say that
it's a deprecated option, please use the --unit cpu_atom instead. I
think even better if we can throw a warning and point to --unit when the
"--cputype" is used.
Thanks,
Kan
> +--unit::
> +Print PMU events and metrics limited to the specific PMU name.
> +(e.g. --unit cpu, --unit msr, --unit cpu_core, --unit cpu_atom)
>
> [[EVENT_MODIFIERS]]
> EVENT MODIFIERS
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index 58e1ec1654ef..cc84ced6da26 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -21,7 +21,6 @@
>
> static bool desc_flag = true;
> static bool details_flag;
> -static const char *hybrid_type;
>
> int cmd_list(int argc, const char **argv)
> {
> @@ -30,6 +29,8 @@ int cmd_list(int argc, const char **argv)
> bool long_desc_flag = false;
> bool deprecated = false;
> char *pmu_name = NULL;
> + const char *hybrid_name = NULL;
> + const char *unit_name = NULL;
> struct option list_options[] = {
> OPT_BOOLEAN(0, "raw-dump", &raw_dump, "Dump raw events"),
> OPT_BOOLEAN('d', "desc", &desc_flag,
> @@ -40,9 +41,10 @@ int cmd_list(int argc, const char **argv)
> "Print information on the perf event names and expressions used internally by events."),
> OPT_BOOLEAN(0, "deprecated", &deprecated,
> "Print deprecated events."),
> - OPT_STRING(0, "cputype", &hybrid_type, "hybrid cpu type",
> - "Print events applying cpu with this type for hybrid platform "
> - "(e.g. core or atom)"),
> + OPT_STRING(0, "cputype", &hybrid_name, "hybrid cpu type",
> + "Limit PMU or metric printing to the given hybrid PMU (e.g. core or atom)."),
> + OPT_STRING(0, "unit", &unit_name, "PMU name",
> + "Limit PMU or metric printing to the specified PMU."),
> OPT_INCR(0, "debug", &verbose,
> "Enable debugging output"),
> OPT_END()
> @@ -53,6 +55,8 @@ int cmd_list(int argc, const char **argv)
> };
>
> set_option_flag(list_options, 0, "raw-dump", PARSE_OPT_HIDDEN);
> + /* Hide hybrid flag for the more generic 'unit' flag. */
> + set_option_flag(list_options, 0, "cputype", PARSE_OPT_HIDDEN);
>
> argc = parse_options(argc, argv, list_options, list_usage,
> PARSE_OPT_STOP_AT_NON_OPTION);
> @@ -62,8 +66,10 @@ int cmd_list(int argc, const char **argv)
> if (!raw_dump && pager_in_use())
> printf("\nList of pre-defined events (to be used in -e or -M):\n\n");
>
> - if (hybrid_type) {
> - pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_type);
> + if (unit_name)
> + pmu_name = strdup(unit_name);
> + else if (hybrid_name) {
> + pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_name);
> if (!pmu_name)
> pr_warning("WARNING: hybrid cputype is not supported!\n");
> }
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 4c98ac29ee13..1943fed9b6d9 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -556,11 +556,12 @@ static int metricgroup__print_callback(const struct pmu_event *pe,
> void *vdata)
> {
> struct metricgroup_print_data *data = vdata;
> + const char *pmu = pe->pmu ?: "cpu";
>
> if (!pe->metric_expr)
> return 0;
>
> - if (data->pmu_name && perf_pmu__is_hybrid(pe->pmu) && strcmp(data->pmu_name, pe->pmu))
> + if (data->pmu_name && strcmp(data->pmu_name, pmu))
> return 0;
>
> return metricgroup__print_pmu_event(pe, data->metricgroups, data->filter,
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index a8f9f47c6ed9..9c771f136b81 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -1694,10 +1694,8 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
> pmu = NULL;
> j = 0;
> while ((pmu = perf_pmu__scan(pmu)) != NULL) {
> - if (pmu_name && perf_pmu__is_hybrid(pmu->name) &&
> - strcmp(pmu_name, pmu->name)) {
> + if (pmu_name && pmu->name && strcmp(pmu_name, pmu->name))
> continue;
> - }
>
> list_for_each_entry(alias, &pmu->aliases, list) {
> char *name = alias->desc ? alias->name :
On Mon, Nov 14, 2022 at 5:58 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>
>
>
> On 2022-11-14 2:51 a.m., Ian Rogers wrote:
> > Deprecate the --cputype option and add a --unit option where '--unit
> > cpu_atom' behaves like '--cputype atom'. The --unit option can be used
> > with arbitrary PMUs, for example:
> >
> > ```
> > $ perf list --unit msr pmu
> >
> > List of pre-defined events (to be used in -e or -M):
> >
> > msr/aperf/ [Kernel PMU event]
> > msr/cpu_thermal_margin/ [Kernel PMU event]
> > msr/mperf/ [Kernel PMU event]
> > msr/pperf/ [Kernel PMU event]
> > msr/smi/ [Kernel PMU event]
> > msr/tsc/ [Kernel PMU event]
> > ```
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > tools/perf/Documentation/perf-list.txt | 6 +++---
> > tools/perf/builtin-list.c | 18 ++++++++++++------
> > tools/perf/util/metricgroup.c | 3 ++-
> > tools/perf/util/pmu.c | 4 +---
> > 4 files changed, 18 insertions(+), 13 deletions(-)
> >
> > diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
> > index 57384a97c04f..44a819af573d 100644
> > --- a/tools/perf/Documentation/perf-list.txt
> > +++ b/tools/perf/Documentation/perf-list.txt
> > @@ -39,9 +39,9 @@ any extra expressions computed by perf stat.
> > --deprecated::
> > Print deprecated events. By default the deprecated events are hidden.
> >
> > ---cputype::
> > -Print events applying cpu with this type for hybrid platform
> > -(e.g. --cputype core or --cputype atom)
>
> The "--cputype" is removed from the documentation, but the code is still
> available. It sounds weird.
>
> Can we still keep the "--cputype" in the documentation? Just say that
> it's a deprecated option, please use the --unit cpu_atom instead. I
> think even better if we can throw a warning and point to --unit when the
> "--cputype" is used.
I think we want to remove --cputype widely in the code and replace
what it does with specifying a PMU name. Advertising a flag but then
warning seems strange and is a behavioral change from what is
currently done. For raw-dump we don't document it in the man page and
hide the flag, this is the pattern being followed here.
Thanks,
Ian
> Thanks,
> Kan
> > +--unit::
> > +Print PMU events and metrics limited to the specific PMU name.
> > +(e.g. --unit cpu, --unit msr, --unit cpu_core, --unit cpu_atom)
> >
> > [[EVENT_MODIFIERS]]
> > EVENT MODIFIERS
> > diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> > index 58e1ec1654ef..cc84ced6da26 100644
> > --- a/tools/perf/builtin-list.c
> > +++ b/tools/perf/builtin-list.c
> > @@ -21,7 +21,6 @@
> >
> > static bool desc_flag = true;
> > static bool details_flag;
> > -static const char *hybrid_type;
> >
> > int cmd_list(int argc, const char **argv)
> > {
> > @@ -30,6 +29,8 @@ int cmd_list(int argc, const char **argv)
> > bool long_desc_flag = false;
> > bool deprecated = false;
> > char *pmu_name = NULL;
> > + const char *hybrid_name = NULL;
> > + const char *unit_name = NULL;
> > struct option list_options[] = {
> > OPT_BOOLEAN(0, "raw-dump", &raw_dump, "Dump raw events"),
> > OPT_BOOLEAN('d', "desc", &desc_flag,
> > @@ -40,9 +41,10 @@ int cmd_list(int argc, const char **argv)
> > "Print information on the perf event names and expressions used internally by events."),
> > OPT_BOOLEAN(0, "deprecated", &deprecated,
> > "Print deprecated events."),
> > - OPT_STRING(0, "cputype", &hybrid_type, "hybrid cpu type",
> > - "Print events applying cpu with this type for hybrid platform "
> > - "(e.g. core or atom)"),
> > + OPT_STRING(0, "cputype", &hybrid_name, "hybrid cpu type",
> > + "Limit PMU or metric printing to the given hybrid PMU (e.g. core or atom)."),
> > + OPT_STRING(0, "unit", &unit_name, "PMU name",
> > + "Limit PMU or metric printing to the specified PMU."),
> > OPT_INCR(0, "debug", &verbose,
> > "Enable debugging output"),
> > OPT_END()
> > @@ -53,6 +55,8 @@ int cmd_list(int argc, const char **argv)
> > };
> >
> > set_option_flag(list_options, 0, "raw-dump", PARSE_OPT_HIDDEN);
> > + /* Hide hybrid flag for the more generic 'unit' flag. */
> > + set_option_flag(list_options, 0, "cputype", PARSE_OPT_HIDDEN);
> >
> > argc = parse_options(argc, argv, list_options, list_usage,
> > PARSE_OPT_STOP_AT_NON_OPTION);
> > @@ -62,8 +66,10 @@ int cmd_list(int argc, const char **argv)
> > if (!raw_dump && pager_in_use())
> > printf("\nList of pre-defined events (to be used in -e or -M):\n\n");
> >
> > - if (hybrid_type) {
> > - pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_type);
> > + if (unit_name)
> > + pmu_name = strdup(unit_name);
> > + else if (hybrid_name) {
> > + pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_name);
> > if (!pmu_name)
> > pr_warning("WARNING: hybrid cputype is not supported!\n");
> > }
> > diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> > index 4c98ac29ee13..1943fed9b6d9 100644
> > --- a/tools/perf/util/metricgroup.c
> > +++ b/tools/perf/util/metricgroup.c
> > @@ -556,11 +556,12 @@ static int metricgroup__print_callback(const struct pmu_event *pe,
> > void *vdata)
> > {
> > struct metricgroup_print_data *data = vdata;
> > + const char *pmu = pe->pmu ?: "cpu";
> >
> > if (!pe->metric_expr)
> > return 0;
> >
> > - if (data->pmu_name && perf_pmu__is_hybrid(pe->pmu) && strcmp(data->pmu_name, pe->pmu))
> > + if (data->pmu_name && strcmp(data->pmu_name, pmu))
> > return 0;
> >
> > return metricgroup__print_pmu_event(pe, data->metricgroups, data->filter,
> > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> > index a8f9f47c6ed9..9c771f136b81 100644
> > --- a/tools/perf/util/pmu.c
> > +++ b/tools/perf/util/pmu.c
> > @@ -1694,10 +1694,8 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
> > pmu = NULL;
> > j = 0;
> > while ((pmu = perf_pmu__scan(pmu)) != NULL) {
> > - if (pmu_name && perf_pmu__is_hybrid(pmu->name) &&
> > - strcmp(pmu_name, pmu->name)) {
> > + if (pmu_name && pmu->name && strcmp(pmu_name, pmu->name))
> > continue;
> > - }
> >
> > list_for_each_entry(alias, &pmu->aliases, list) {
> > char *name = alias->desc ? alias->name :
On 2022-11-14 9:02 a.m., Ian Rogers wrote:
> On Mon, Nov 14, 2022 at 5:58 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>>
>>
>>
>> On 2022-11-14 2:51 a.m., Ian Rogers wrote:
>>> Deprecate the --cputype option and add a --unit option where '--unit
>>> cpu_atom' behaves like '--cputype atom'. The --unit option can be used
>>> with arbitrary PMUs, for example:
>>>
>>> ```
>>> $ perf list --unit msr pmu
>>>
>>> List of pre-defined events (to be used in -e or -M):
>>>
>>> msr/aperf/ [Kernel PMU event]
>>> msr/cpu_thermal_margin/ [Kernel PMU event]
>>> msr/mperf/ [Kernel PMU event]
>>> msr/pperf/ [Kernel PMU event]
>>> msr/smi/ [Kernel PMU event]
>>> msr/tsc/ [Kernel PMU event]
>>> ```
>>>
>>> Signed-off-by: Ian Rogers <irogers@google.com>
>>> ---
>>> tools/perf/Documentation/perf-list.txt | 6 +++---
>>> tools/perf/builtin-list.c | 18 ++++++++++++------
>>> tools/perf/util/metricgroup.c | 3 ++-
>>> tools/perf/util/pmu.c | 4 +---
>>> 4 files changed, 18 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
>>> index 57384a97c04f..44a819af573d 100644
>>> --- a/tools/perf/Documentation/perf-list.txt
>>> +++ b/tools/perf/Documentation/perf-list.txt
>>> @@ -39,9 +39,9 @@ any extra expressions computed by perf stat.
>>> --deprecated::
>>> Print deprecated events. By default the deprecated events are hidden.
>>>
>>> ---cputype::
>>> -Print events applying cpu with this type for hybrid platform
>>> -(e.g. --cputype core or --cputype atom)
>>
>> The "--cputype" is removed from the documentation, but the code is still
>> available. It sounds weird.
>>
>> Can we still keep the "--cputype" in the documentation? Just say that
>> it's a deprecated option, please use the --unit cpu_atom instead. I
>> think even better if we can throw a warning and point to --unit when the
>> "--cputype" is used.
>
> I think we want to remove --cputype widely in the code and replace
> what it does with specifying a PMU name. Advertising a flag but then
> warning seems strange and is a behavioral change from what is
> currently done. For raw-dump we don't document it in the man page and
> hide the flag, this is the pattern being followed here.
I see. So the --cputype is still supported, but only be hidden in the
default. Sure, we can follow the pattern.
>
> Thanks,
> Ian
>
>> Thanks,
>> Kan
>>> +--unit::
>>> +Print PMU events and metrics limited to the specific PMU name.
>>> +(e.g. --unit cpu, --unit msr, --unit cpu_core, --unit cpu_atom)
>>>
>>> [[EVENT_MODIFIERS]]
>>> EVENT MODIFIERS
>>> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
>>> index 58e1ec1654ef..cc84ced6da26 100644
>>> --- a/tools/perf/builtin-list.c
>>> +++ b/tools/perf/builtin-list.c
>>> @@ -21,7 +21,6 @@
>>>
>>> static bool desc_flag = true;
>>> static bool details_flag;
>>> -static const char *hybrid_type;
>>>
>>> int cmd_list(int argc, const char **argv)
>>> {
>>> @@ -30,6 +29,8 @@ int cmd_list(int argc, const char **argv)
>>> bool long_desc_flag = false;
>>> bool deprecated = false;
>>> char *pmu_name = NULL;
>>> + const char *hybrid_name = NULL;
>>> + const char *unit_name = NULL;
>>> struct option list_options[] = {
>>> OPT_BOOLEAN(0, "raw-dump", &raw_dump, "Dump raw events"),
>>> OPT_BOOLEAN('d', "desc", &desc_flag,
>>> @@ -40,9 +41,10 @@ int cmd_list(int argc, const char **argv)
>>> "Print information on the perf event names and expressions used internally by events."),
>>> OPT_BOOLEAN(0, "deprecated", &deprecated,
>>> "Print deprecated events."),
>>> - OPT_STRING(0, "cputype", &hybrid_type, "hybrid cpu type",
>>> - "Print events applying cpu with this type for hybrid platform "
>>> - "(e.g. core or atom)"),
>>> + OPT_STRING(0, "cputype", &hybrid_name, "hybrid cpu type",
>>> + "Limit PMU or metric printing to the given hybrid PMU (e.g. core or atom)."),
>>> + OPT_STRING(0, "unit", &unit_name, "PMU name",
>>> + "Limit PMU or metric printing to the specified PMU."),
>>> OPT_INCR(0, "debug", &verbose,
>>> "Enable debugging output"),
>>> OPT_END()
>>> @@ -53,6 +55,8 @@ int cmd_list(int argc, const char **argv)
>>> };
>>>
>>> set_option_flag(list_options, 0, "raw-dump", PARSE_OPT_HIDDEN);
>>> + /* Hide hybrid flag for the more generic 'unit' flag. */
>>> + set_option_flag(list_options, 0, "cputype", PARSE_OPT_HIDDEN);
>>>
>>> argc = parse_options(argc, argv, list_options, list_usage,
>>> PARSE_OPT_STOP_AT_NON_OPTION);
>>> @@ -62,8 +66,10 @@ int cmd_list(int argc, const char **argv)
>>> if (!raw_dump && pager_in_use())
>>> printf("\nList of pre-defined events (to be used in -e or -M):\n\n");
>>>
>>> - if (hybrid_type) {
>>> - pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_type);
>>> + if (unit_name)
>>> + pmu_name = strdup(unit_name);
>>> + else if (hybrid_name) {
>>> + pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_name);
>>> if (!pmu_name)
>>> pr_warning("WARNING: hybrid cputype is not supported!\n");
>>> }
Can the tool implicitly convert the --cputype to --unit at the very
beginning? (Just need to append a prefix "cpu_".). Here we only need to
handle the unit_name.
The same logic may be applied for other tools if someone implements the
--unit for the stat or record later.
BTW: we may want to check the existence of a PMU here, just like what we
did for the hybrid. If a user perf list a nonexistence PMU, we can warn
here.
Thanks,
Kan
>>> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
>>> index 4c98ac29ee13..1943fed9b6d9 100644
>>> --- a/tools/perf/util/metricgroup.c
>>> +++ b/tools/perf/util/metricgroup.c
>>> @@ -556,11 +556,12 @@ static int metricgroup__print_callback(const struct pmu_event *pe,
>>> void *vdata)
>>> {
>>> struct metricgroup_print_data *data = vdata;
>>> + const char *pmu = pe->pmu ?: "cpu";
>>>
>>> if (!pe->metric_expr)
>>> return 0;
>>>
>>> - if (data->pmu_name && perf_pmu__is_hybrid(pe->pmu) && strcmp(data->pmu_name, pe->pmu))
>>> + if (data->pmu_name && strcmp(data->pmu_name, pmu))
>>> return 0;
>>>
>>> return metricgroup__print_pmu_event(pe, data->metricgroups, data->filter,
>>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
>>> index a8f9f47c6ed9..9c771f136b81 100644
>>> --- a/tools/perf/util/pmu.c
>>> +++ b/tools/perf/util/pmu.c
>>> @@ -1694,10 +1694,8 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
>>> pmu = NULL;
>>> j = 0;
>>> while ((pmu = perf_pmu__scan(pmu)) != NULL) {
>>> - if (pmu_name && perf_pmu__is_hybrid(pmu->name) &&
>>> - strcmp(pmu_name, pmu->name)) {
>>> + if (pmu_name && pmu->name && strcmp(pmu_name, pmu->name))
>>> continue;
>>> - }
>>>
>>> list_for_each_entry(alias, &pmu->aliases, list) {
>>> char *name = alias->desc ? alias->name :
On Mon, Nov 14, 2022 at 6:55 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>
>
>
> On 2022-11-14 9:02 a.m., Ian Rogers wrote:
> > On Mon, Nov 14, 2022 at 5:58 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
> >>
> >>
> >>
> >> On 2022-11-14 2:51 a.m., Ian Rogers wrote:
> >>> Deprecate the --cputype option and add a --unit option where '--unit
> >>> cpu_atom' behaves like '--cputype atom'. The --unit option can be used
> >>> with arbitrary PMUs, for example:
> >>>
> >>> ```
> >>> $ perf list --unit msr pmu
> >>>
> >>> List of pre-defined events (to be used in -e or -M):
> >>>
> >>> msr/aperf/ [Kernel PMU event]
> >>> msr/cpu_thermal_margin/ [Kernel PMU event]
> >>> msr/mperf/ [Kernel PMU event]
> >>> msr/pperf/ [Kernel PMU event]
> >>> msr/smi/ [Kernel PMU event]
> >>> msr/tsc/ [Kernel PMU event]
> >>> ```
> >>>
> >>> Signed-off-by: Ian Rogers <irogers@google.com>
> >>> ---
> >>> tools/perf/Documentation/perf-list.txt | 6 +++---
> >>> tools/perf/builtin-list.c | 18 ++++++++++++------
> >>> tools/perf/util/metricgroup.c | 3 ++-
> >>> tools/perf/util/pmu.c | 4 +---
> >>> 4 files changed, 18 insertions(+), 13 deletions(-)
> >>>
> >>> diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
> >>> index 57384a97c04f..44a819af573d 100644
> >>> --- a/tools/perf/Documentation/perf-list.txt
> >>> +++ b/tools/perf/Documentation/perf-list.txt
> >>> @@ -39,9 +39,9 @@ any extra expressions computed by perf stat.
> >>> --deprecated::
> >>> Print deprecated events. By default the deprecated events are hidden.
> >>>
> >>> ---cputype::
> >>> -Print events applying cpu with this type for hybrid platform
> >>> -(e.g. --cputype core or --cputype atom)
> >>
> >> The "--cputype" is removed from the documentation, but the code is still
> >> available. It sounds weird.
> >>
> >> Can we still keep the "--cputype" in the documentation? Just say that
> >> it's a deprecated option, please use the --unit cpu_atom instead. I
> >> think even better if we can throw a warning and point to --unit when the
> >> "--cputype" is used.
> >
> > I think we want to remove --cputype widely in the code and replace
> > what it does with specifying a PMU name. Advertising a flag but then
> > warning seems strange and is a behavioral change from what is
> > currently done. For raw-dump we don't document it in the man page and
> > hide the flag, this is the pattern being followed here.
>
> I see. So the --cputype is still supported, but only be hidden in the
> default. Sure, we can follow the pattern.
>
> >
> > Thanks,
> > Ian
> >
> >> Thanks,
> >> Kan
> >>> +--unit::
> >>> +Print PMU events and metrics limited to the specific PMU name.
> >>> +(e.g. --unit cpu, --unit msr, --unit cpu_core, --unit cpu_atom)
> >>>
> >>> [[EVENT_MODIFIERS]]
> >>> EVENT MODIFIERS
> >>> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> >>> index 58e1ec1654ef..cc84ced6da26 100644
> >>> --- a/tools/perf/builtin-list.c
> >>> +++ b/tools/perf/builtin-list.c
> >>> @@ -21,7 +21,6 @@
> >>>
> >>> static bool desc_flag = true;
> >>> static bool details_flag;
> >>> -static const char *hybrid_type;
> >>>
> >>> int cmd_list(int argc, const char **argv)
> >>> {
> >>> @@ -30,6 +29,8 @@ int cmd_list(int argc, const char **argv)
> >>> bool long_desc_flag = false;
> >>> bool deprecated = false;
> >>> char *pmu_name = NULL;
> >>> + const char *hybrid_name = NULL;
> >>> + const char *unit_name = NULL;
> >>> struct option list_options[] = {
> >>> OPT_BOOLEAN(0, "raw-dump", &raw_dump, "Dump raw events"),
> >>> OPT_BOOLEAN('d', "desc", &desc_flag,
> >>> @@ -40,9 +41,10 @@ int cmd_list(int argc, const char **argv)
> >>> "Print information on the perf event names and expressions used internally by events."),
> >>> OPT_BOOLEAN(0, "deprecated", &deprecated,
> >>> "Print deprecated events."),
> >>> - OPT_STRING(0, "cputype", &hybrid_type, "hybrid cpu type",
> >>> - "Print events applying cpu with this type for hybrid platform "
> >>> - "(e.g. core or atom)"),
> >>> + OPT_STRING(0, "cputype", &hybrid_name, "hybrid cpu type",
> >>> + "Limit PMU or metric printing to the given hybrid PMU (e.g. core or atom)."),
> >>> + OPT_STRING(0, "unit", &unit_name, "PMU name",
> >>> + "Limit PMU or metric printing to the specified PMU."),
> >>> OPT_INCR(0, "debug", &verbose,
> >>> "Enable debugging output"),
> >>> OPT_END()
> >>> @@ -53,6 +55,8 @@ int cmd_list(int argc, const char **argv)
> >>> };
> >>>
> >>> set_option_flag(list_options, 0, "raw-dump", PARSE_OPT_HIDDEN);
> >>> + /* Hide hybrid flag for the more generic 'unit' flag. */
> >>> + set_option_flag(list_options, 0, "cputype", PARSE_OPT_HIDDEN);
> >>>
> >>> argc = parse_options(argc, argv, list_options, list_usage,
> >>> PARSE_OPT_STOP_AT_NON_OPTION);
> >>> @@ -62,8 +66,10 @@ int cmd_list(int argc, const char **argv)
> >>> if (!raw_dump && pager_in_use())
> >>> printf("\nList of pre-defined events (to be used in -e or -M):\n\n");
> >>>
> >>> - if (hybrid_type) {
> >>> - pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_type);
> >>> + if (unit_name)
> >>> + pmu_name = strdup(unit_name);
> >>> + else if (hybrid_name) {
> >>> + pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_name);
> >>> if (!pmu_name)
> >>> pr_warning("WARNING: hybrid cputype is not supported!\n");
> >>> }
>
> Can the tool implicitly convert the --cputype to --unit at the very
> beginning? (Just need to append a prefix "cpu_".). Here we only need to
> handle the unit_name.
> The same logic may be applied for other tools if someone implements the
> --unit for the stat or record later.
>
> BTW: we may want to check the existence of a PMU here, just like what we
> did for the hybrid. If a user perf list a nonexistence PMU, we can warn
> here.
That'd be a fine follow up. I think it would be problematic as it
would warn because of a lack of permissions.
Thanks,
Ian
> Thanks,
> Kan
>
> >>> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> >>> index 4c98ac29ee13..1943fed9b6d9 100644
> >>> --- a/tools/perf/util/metricgroup.c
> >>> +++ b/tools/perf/util/metricgroup.c
> >>> @@ -556,11 +556,12 @@ static int metricgroup__print_callback(const struct pmu_event *pe,
> >>> void *vdata)
> >>> {
> >>> struct metricgroup_print_data *data = vdata;
> >>> + const char *pmu = pe->pmu ?: "cpu";
> >>>
> >>> if (!pe->metric_expr)
> >>> return 0;
> >>>
> >>> - if (data->pmu_name && perf_pmu__is_hybrid(pe->pmu) && strcmp(data->pmu_name, pe->pmu))
> >>> + if (data->pmu_name && strcmp(data->pmu_name, pmu))
> >>> return 0;
> >>>
> >>> return metricgroup__print_pmu_event(pe, data->metricgroups, data->filter,
> >>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> >>> index a8f9f47c6ed9..9c771f136b81 100644
> >>> --- a/tools/perf/util/pmu.c
> >>> +++ b/tools/perf/util/pmu.c
> >>> @@ -1694,10 +1694,8 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
> >>> pmu = NULL;
> >>> j = 0;
> >>> while ((pmu = perf_pmu__scan(pmu)) != NULL) {
> >>> - if (pmu_name && perf_pmu__is_hybrid(pmu->name) &&
> >>> - strcmp(pmu_name, pmu->name)) {
> >>> + if (pmu_name && pmu->name && strcmp(pmu_name, pmu->name))
> >>> continue;
> >>> - }
> >>>
> >>> list_for_each_entry(alias, &pmu->aliases, list) {
> >>> char *name = alias->desc ? alias->name :
On 2022-11-14 12:10 p.m., Ian Rogers wrote:
> On Mon, Nov 14, 2022 at 6:55 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>>
>>
>>
>> On 2022-11-14 9:02 a.m., Ian Rogers wrote:
>>> On Mon, Nov 14, 2022 at 5:58 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>>>>
>>>>
>>>>
>>>> On 2022-11-14 2:51 a.m., Ian Rogers wrote:
>>>>> Deprecate the --cputype option and add a --unit option where '--unit
>>>>> cpu_atom' behaves like '--cputype atom'. The --unit option can be used
>>>>> with arbitrary PMUs, for example:
>>>>>
>>>>> ```
>>>>> $ perf list --unit msr pmu
>>>>>
>>>>> List of pre-defined events (to be used in -e or -M):
>>>>>
>>>>> msr/aperf/ [Kernel PMU event]
>>>>> msr/cpu_thermal_margin/ [Kernel PMU event]
>>>>> msr/mperf/ [Kernel PMU event]
>>>>> msr/pperf/ [Kernel PMU event]
>>>>> msr/smi/ [Kernel PMU event]
>>>>> msr/tsc/ [Kernel PMU event]
>>>>> ```
>>>>>
>>>>> Signed-off-by: Ian Rogers <irogers@google.com>
>>>>> ---
>>>>> tools/perf/Documentation/perf-list.txt | 6 +++---
>>>>> tools/perf/builtin-list.c | 18 ++++++++++++------
>>>>> tools/perf/util/metricgroup.c | 3 ++-
>>>>> tools/perf/util/pmu.c | 4 +---
>>>>> 4 files changed, 18 insertions(+), 13 deletions(-)
>>>>>
>>>>> diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
>>>>> index 57384a97c04f..44a819af573d 100644
>>>>> --- a/tools/perf/Documentation/perf-list.txt
>>>>> +++ b/tools/perf/Documentation/perf-list.txt
>>>>> @@ -39,9 +39,9 @@ any extra expressions computed by perf stat.
>>>>> --deprecated::
>>>>> Print deprecated events. By default the deprecated events are hidden.
>>>>>
>>>>> ---cputype::
>>>>> -Print events applying cpu with this type for hybrid platform
>>>>> -(e.g. --cputype core or --cputype atom)
>>>>
>>>> The "--cputype" is removed from the documentation, but the code is still
>>>> available. It sounds weird.
>>>>
>>>> Can we still keep the "--cputype" in the documentation? Just say that
>>>> it's a deprecated option, please use the --unit cpu_atom instead. I
>>>> think even better if we can throw a warning and point to --unit when the
>>>> "--cputype" is used.
>>>
>>> I think we want to remove --cputype widely in the code and replace
>>> what it does with specifying a PMU name. Advertising a flag but then
>>> warning seems strange and is a behavioral change from what is
>>> currently done. For raw-dump we don't document it in the man page and
>>> hide the flag, this is the pattern being followed here.
>>
>> I see. So the --cputype is still supported, but only be hidden in the
>> default. Sure, we can follow the pattern.
>>
>>>
>>> Thanks,
>>> Ian
>>>
>>>> Thanks,
>>>> Kan
>>>>> +--unit::
>>>>> +Print PMU events and metrics limited to the specific PMU name.
>>>>> +(e.g. --unit cpu, --unit msr, --unit cpu_core, --unit cpu_atom)
>>>>>
>>>>> [[EVENT_MODIFIERS]]
>>>>> EVENT MODIFIERS
>>>>> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
>>>>> index 58e1ec1654ef..cc84ced6da26 100644
>>>>> --- a/tools/perf/builtin-list.c
>>>>> +++ b/tools/perf/builtin-list.c
>>>>> @@ -21,7 +21,6 @@
>>>>>
>>>>> static bool desc_flag = true;
>>>>> static bool details_flag;
>>>>> -static const char *hybrid_type;
>>>>>
>>>>> int cmd_list(int argc, const char **argv)
>>>>> {
>>>>> @@ -30,6 +29,8 @@ int cmd_list(int argc, const char **argv)
>>>>> bool long_desc_flag = false;
>>>>> bool deprecated = false;
>>>>> char *pmu_name = NULL;
>>>>> + const char *hybrid_name = NULL;
>>>>> + const char *unit_name = NULL;
>>>>> struct option list_options[] = {
>>>>> OPT_BOOLEAN(0, "raw-dump", &raw_dump, "Dump raw events"),
>>>>> OPT_BOOLEAN('d', "desc", &desc_flag,
>>>>> @@ -40,9 +41,10 @@ int cmd_list(int argc, const char **argv)
>>>>> "Print information on the perf event names and expressions used internally by events."),
>>>>> OPT_BOOLEAN(0, "deprecated", &deprecated,
>>>>> "Print deprecated events."),
>>>>> - OPT_STRING(0, "cputype", &hybrid_type, "hybrid cpu type",
>>>>> - "Print events applying cpu with this type for hybrid platform "
>>>>> - "(e.g. core or atom)"),
>>>>> + OPT_STRING(0, "cputype", &hybrid_name, "hybrid cpu type",
>>>>> + "Limit PMU or metric printing to the given hybrid PMU (e.g. core or atom)."),
>>>>> + OPT_STRING(0, "unit", &unit_name, "PMU name",
>>>>> + "Limit PMU or metric printing to the specified PMU."),
>>>>> OPT_INCR(0, "debug", &verbose,
>>>>> "Enable debugging output"),
>>>>> OPT_END()
>>>>> @@ -53,6 +55,8 @@ int cmd_list(int argc, const char **argv)
>>>>> };
>>>>>
>>>>> set_option_flag(list_options, 0, "raw-dump", PARSE_OPT_HIDDEN);
>>>>> + /* Hide hybrid flag for the more generic 'unit' flag. */
>>>>> + set_option_flag(list_options, 0, "cputype", PARSE_OPT_HIDDEN);
>>>>>
>>>>> argc = parse_options(argc, argv, list_options, list_usage,
>>>>> PARSE_OPT_STOP_AT_NON_OPTION);
>>>>> @@ -62,8 +66,10 @@ int cmd_list(int argc, const char **argv)
>>>>> if (!raw_dump && pager_in_use())
>>>>> printf("\nList of pre-defined events (to be used in -e or -M):\n\n");
>>>>>
>>>>> - if (hybrid_type) {
>>>>> - pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_type);
>>>>> + if (unit_name)
>>>>> + pmu_name = strdup(unit_name);
>>>>> + else if (hybrid_name) {
>>>>> + pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_name);
>>>>> if (!pmu_name)
>>>>> pr_warning("WARNING: hybrid cputype is not supported!\n");
>>>>> }
>>
>> Can the tool implicitly convert the --cputype to --unit at the very
>> beginning? (Just need to append a prefix "cpu_".). Here we only need to
>> handle the unit_name.
>> The same logic may be applied for other tools if someone implements the
>> --unit for the stat or record later.
>>
>> BTW: we may want to check the existence of a PMU here, just like what we
>> did for the hybrid. If a user perf list a nonexistence PMU, we can warn
>> here.
>
> That'd be a fine follow up. I think it would be problematic as it
> would warn because of a lack of permissions.
>
Just check whether the PMU folder exists in the sysfs. I don't think it
has a permission issue.
Thanks,
Kan
> Thanks,
> Ian
>
>> Thanks,
>> Kan
>>
>>>>> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
>>>>> index 4c98ac29ee13..1943fed9b6d9 100644
>>>>> --- a/tools/perf/util/metricgroup.c
>>>>> +++ b/tools/perf/util/metricgroup.c
>>>>> @@ -556,11 +556,12 @@ static int metricgroup__print_callback(const struct pmu_event *pe,
>>>>> void *vdata)
>>>>> {
>>>>> struct metricgroup_print_data *data = vdata;
>>>>> + const char *pmu = pe->pmu ?: "cpu";
>>>>>
>>>>> if (!pe->metric_expr)
>>>>> return 0;
>>>>>
>>>>> - if (data->pmu_name && perf_pmu__is_hybrid(pe->pmu) && strcmp(data->pmu_name, pe->pmu))
>>>>> + if (data->pmu_name && strcmp(data->pmu_name, pmu))
>>>>> return 0;
>>>>>
>>>>> return metricgroup__print_pmu_event(pe, data->metricgroups, data->filter,
>>>>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
>>>>> index a8f9f47c6ed9..9c771f136b81 100644
>>>>> --- a/tools/perf/util/pmu.c
>>>>> +++ b/tools/perf/util/pmu.c
>>>>> @@ -1694,10 +1694,8 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
>>>>> pmu = NULL;
>>>>> j = 0;
>>>>> while ((pmu = perf_pmu__scan(pmu)) != NULL) {
>>>>> - if (pmu_name && perf_pmu__is_hybrid(pmu->name) &&
>>>>> - strcmp(pmu_name, pmu->name)) {
>>>>> + if (pmu_name && pmu->name && strcmp(pmu_name, pmu->name))
>>>>> continue;
>>>>> - }
>>>>>
>>>>> list_for_each_entry(alias, &pmu->aliases, list) {
>>>>> char *name = alias->desc ? alias->name :
On 11/14/2022 3:51 PM, Ian Rogers wrote:
> Deprecate the --cputype option and add a --unit option where '--unit
> cpu_atom' behaves like '--cputype atom'. The --unit option can be used
> with arbitrary PMUs, for example:
>
> ```
> $ perf list --unit msr pmu
>
> List of pre-defined events (to be used in -e or -M):
>
> msr/aperf/ [Kernel PMU event]
> msr/cpu_thermal_margin/ [Kernel PMU event]
> msr/mperf/ [Kernel PMU event]
> msr/pperf/ [Kernel PMU event]
> msr/smi/ [Kernel PMU event]
> msr/tsc/ [Kernel PMU event]
> ```
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/Documentation/perf-list.txt | 6 +++---
> tools/perf/builtin-list.c | 18 ++++++++++++------
> tools/perf/util/metricgroup.c | 3 ++-
> tools/perf/util/pmu.c | 4 +---
> 4 files changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
> index 57384a97c04f..44a819af573d 100644
> --- a/tools/perf/Documentation/perf-list.txt
> +++ b/tools/perf/Documentation/perf-list.txt
> @@ -39,9 +39,9 @@ any extra expressions computed by perf stat.
> --deprecated::
> Print deprecated events. By default the deprecated events are hidden.
>
> ---cputype::
> -Print events applying cpu with this type for hybrid platform
> -(e.g. --cputype core or --cputype atom)
> +--unit::
> +Print PMU events and metrics limited to the specific PMU name.
> +(e.g. --unit cpu, --unit msr, --unit cpu_core, --unit cpu_atom)
>
> [[EVENT_MODIFIERS]]
> EVENT MODIFIERS
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index 58e1ec1654ef..cc84ced6da26 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -21,7 +21,6 @@
>
> static bool desc_flag = true;
> static bool details_flag;
> -static const char *hybrid_type;
>
> int cmd_list(int argc, const char **argv)
> {
> @@ -30,6 +29,8 @@ int cmd_list(int argc, const char **argv)
> bool long_desc_flag = false;
> bool deprecated = false;
> char *pmu_name = NULL;
> + const char *hybrid_name = NULL;
> + const char *unit_name = NULL;
> struct option list_options[] = {
> OPT_BOOLEAN(0, "raw-dump", &raw_dump, "Dump raw events"),
> OPT_BOOLEAN('d', "desc", &desc_flag,
> @@ -40,9 +41,10 @@ int cmd_list(int argc, const char **argv)
> "Print information on the perf event names and expressions used internally by events."),
> OPT_BOOLEAN(0, "deprecated", &deprecated,
> "Print deprecated events."),
> - OPT_STRING(0, "cputype", &hybrid_type, "hybrid cpu type",
> - "Print events applying cpu with this type for hybrid platform "
> - "(e.g. core or atom)"),
> + OPT_STRING(0, "cputype", &hybrid_name, "hybrid cpu type",
> + "Limit PMU or metric printing to the given hybrid PMU (e.g. core or atom)."),
> + OPT_STRING(0, "unit", &unit_name, "PMU name",
> + "Limit PMU or metric printing to the specified PMU."),
> OPT_INCR(0, "debug", &verbose,
> "Enable debugging output"),
> OPT_END()
> @@ -53,6 +55,8 @@ int cmd_list(int argc, const char **argv)
> };
>
> set_option_flag(list_options, 0, "raw-dump", PARSE_OPT_HIDDEN);
> + /* Hide hybrid flag for the more generic 'unit' flag. */
> + set_option_flag(list_options, 0, "cputype", PARSE_OPT_HIDDEN);
>
> argc = parse_options(argc, argv, list_options, list_usage,
> PARSE_OPT_STOP_AT_NON_OPTION);
> @@ -62,8 +66,10 @@ int cmd_list(int argc, const char **argv)
> if (!raw_dump && pager_in_use())
> printf("\nList of pre-defined events (to be used in -e or -M):\n\n");
>
> - if (hybrid_type) {
> - pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_type);
> + if (unit_name)
> + pmu_name = strdup(unit_name);
> + else if (hybrid_name) {
> + pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_name);
> if (!pmu_name)
> pr_warning("WARNING: hybrid cputype is not supported!\n");
> }
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 4c98ac29ee13..1943fed9b6d9 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -556,11 +556,12 @@ static int metricgroup__print_callback(const struct pmu_event *pe,
> void *vdata)
> {
> struct metricgroup_print_data *data = vdata;
> + const char *pmu = pe->pmu ?: "cpu";
>
> if (!pe->metric_expr)
> return 0;
>
> - if (data->pmu_name && perf_pmu__is_hybrid(pe->pmu) && strcmp(data->pmu_name, pe->pmu))
> + if (data->pmu_name && strcmp(data->pmu_name, pmu))
> return 0;
>
> return metricgroup__print_pmu_event(pe, data->metricgroups, data->filter,
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index a8f9f47c6ed9..9c771f136b81 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -1694,10 +1694,8 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
> pmu = NULL;
> j = 0;
> while ((pmu = perf_pmu__scan(pmu)) != NULL) {
> - if (pmu_name && perf_pmu__is_hybrid(pmu->name) &&
> - strcmp(pmu_name, pmu->name)) {
> + if (pmu_name && pmu->name && strcmp(pmu_name, pmu->name))
Why remove perf_pmu__is_hybrid check?
> continue;
> - }
>
> list_for_each_entry(alias, &pmu->aliases, list) {
> char *name = alias->desc ? alias->name :
--
Zhengjun Xing
On Mon, Nov 14, 2022 at 12:51 AM Xing Zhengjun
<zhengjun.xing@linux.intel.com> wrote:
>
>
>
> On 11/14/2022 3:51 PM, Ian Rogers wrote:
> > Deprecate the --cputype option and add a --unit option where '--unit
> > cpu_atom' behaves like '--cputype atom'. The --unit option can be used
> > with arbitrary PMUs, for example:
> >
> > ```
> > $ perf list --unit msr pmu
> >
> > List of pre-defined events (to be used in -e or -M):
> >
> > msr/aperf/ [Kernel PMU event]
> > msr/cpu_thermal_margin/ [Kernel PMU event]
> > msr/mperf/ [Kernel PMU event]
> > msr/pperf/ [Kernel PMU event]
> > msr/smi/ [Kernel PMU event]
> > msr/tsc/ [Kernel PMU event]
> > ```
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > tools/perf/Documentation/perf-list.txt | 6 +++---
> > tools/perf/builtin-list.c | 18 ++++++++++++------
> > tools/perf/util/metricgroup.c | 3 ++-
> > tools/perf/util/pmu.c | 4 +---
> > 4 files changed, 18 insertions(+), 13 deletions(-)
> >
> > diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
> > index 57384a97c04f..44a819af573d 100644
> > --- a/tools/perf/Documentation/perf-list.txt
> > +++ b/tools/perf/Documentation/perf-list.txt
> > @@ -39,9 +39,9 @@ any extra expressions computed by perf stat.
> > --deprecated::
> > Print deprecated events. By default the deprecated events are hidden.
> >
> > ---cputype::
> > -Print events applying cpu with this type for hybrid platform
> > -(e.g. --cputype core or --cputype atom)
> > +--unit::
> > +Print PMU events and metrics limited to the specific PMU name.
> > +(e.g. --unit cpu, --unit msr, --unit cpu_core, --unit cpu_atom)
> >
> > [[EVENT_MODIFIERS]]
> > EVENT MODIFIERS
> > diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> > index 58e1ec1654ef..cc84ced6da26 100644
> > --- a/tools/perf/builtin-list.c
> > +++ b/tools/perf/builtin-list.c
> > @@ -21,7 +21,6 @@
> >
> > static bool desc_flag = true;
> > static bool details_flag;
> > -static const char *hybrid_type;
> >
> > int cmd_list(int argc, const char **argv)
> > {
> > @@ -30,6 +29,8 @@ int cmd_list(int argc, const char **argv)
> > bool long_desc_flag = false;
> > bool deprecated = false;
> > char *pmu_name = NULL;
> > + const char *hybrid_name = NULL;
> > + const char *unit_name = NULL;
> > struct option list_options[] = {
> > OPT_BOOLEAN(0, "raw-dump", &raw_dump, "Dump raw events"),
> > OPT_BOOLEAN('d', "desc", &desc_flag,
> > @@ -40,9 +41,10 @@ int cmd_list(int argc, const char **argv)
> > "Print information on the perf event names and expressions used internally by events."),
> > OPT_BOOLEAN(0, "deprecated", &deprecated,
> > "Print deprecated events."),
> > - OPT_STRING(0, "cputype", &hybrid_type, "hybrid cpu type",
> > - "Print events applying cpu with this type for hybrid platform "
> > - "(e.g. core or atom)"),
> > + OPT_STRING(0, "cputype", &hybrid_name, "hybrid cpu type",
> > + "Limit PMU or metric printing to the given hybrid PMU (e.g. core or atom)."),
> > + OPT_STRING(0, "unit", &unit_name, "PMU name",
> > + "Limit PMU or metric printing to the specified PMU."),
> > OPT_INCR(0, "debug", &verbose,
> > "Enable debugging output"),
> > OPT_END()
> > @@ -53,6 +55,8 @@ int cmd_list(int argc, const char **argv)
> > };
> >
> > set_option_flag(list_options, 0, "raw-dump", PARSE_OPT_HIDDEN);
> > + /* Hide hybrid flag for the more generic 'unit' flag. */
> > + set_option_flag(list_options, 0, "cputype", PARSE_OPT_HIDDEN);
> >
> > argc = parse_options(argc, argv, list_options, list_usage,
> > PARSE_OPT_STOP_AT_NON_OPTION);
> > @@ -62,8 +66,10 @@ int cmd_list(int argc, const char **argv)
> > if (!raw_dump && pager_in_use())
> > printf("\nList of pre-defined events (to be used in -e or -M):\n\n");
> >
> > - if (hybrid_type) {
> > - pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_type);
> > + if (unit_name)
> > + pmu_name = strdup(unit_name);
> > + else if (hybrid_name) {
> > + pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_name);
> > if (!pmu_name)
> > pr_warning("WARNING: hybrid cputype is not supported!\n");
> > }
> > diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> > index 4c98ac29ee13..1943fed9b6d9 100644
> > --- a/tools/perf/util/metricgroup.c
> > +++ b/tools/perf/util/metricgroup.c
> > @@ -556,11 +556,12 @@ static int metricgroup__print_callback(const struct pmu_event *pe,
> > void *vdata)
> > {
> > struct metricgroup_print_data *data = vdata;
> > + const char *pmu = pe->pmu ?: "cpu";
> >
> > if (!pe->metric_expr)
> > return 0;
> >
> > - if (data->pmu_name && perf_pmu__is_hybrid(pe->pmu) && strcmp(data->pmu_name, pe->pmu))
> > + if (data->pmu_name && strcmp(data->pmu_name, pmu))
> > return 0;
> >
> > return metricgroup__print_pmu_event(pe, data->metricgroups, data->filter,
> > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> > index a8f9f47c6ed9..9c771f136b81 100644
> > --- a/tools/perf/util/pmu.c
> > +++ b/tools/perf/util/pmu.c
> > @@ -1694,10 +1694,8 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
> > pmu = NULL;
> > j = 0;
> > while ((pmu = perf_pmu__scan(pmu)) != NULL) {
> > - if (pmu_name && perf_pmu__is_hybrid(pmu->name) &&
> > - strcmp(pmu_name, pmu->name)) {
> > + if (pmu_name && pmu->name && strcmp(pmu_name, pmu->name))
>
> Why remove perf_pmu__is_hybrid check?
Thanks for checking! If you have the perf_pmu__is_hybrid check then
the filter only applies PMUs that are hybrid. Firstly, why have
is_hybrid in struct perf_pmu if you use this function that searches a
list? Secondly, the point of this change is to make the PMU name
filtering a generic feature rather than one limited to specifically
PMUs called cpu_core and cpu_atom.
Thanks,
Ian
>
> > continue;
> > - }
> >
> > list_for_each_entry(alias, &pmu->aliases, list) {
> > char *name = alias->desc ? alias->name :
>
> --
> Zhengjun Xing
© 2016 - 2026 Red Hat, Inc.