Messages in this thread | | | From | Ian Rogers <> | Date | Mon, 14 Nov 2022 05:58:06 -0800 | Subject | Re: [PATCH v1 4/9] perf list: Generalize limiting to a PMU name |
| |
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
| |