* [PATCH] perf record: Support "--cputype" option for hybrid events
@ 2022-06-15 15:08 zhengjun.xing
2022-06-15 15:16 ` Ian Rogers
0 siblings, 1 reply; 6+ messages in thread
From: zhengjun.xing @ 2022-06-15 15:08 UTC (permalink / raw)
To: acme, peterz, mingo, alexander.shishkin, jolsa, namhyung
Cc: linux-kernel, linux-perf-users, irogers, ak, kan.liang,
zhengjun.xing
From: Zhengjun Xing <zhengjun.xing@linux.intel.com>
perf stat already has the "--cputype" option to enable events only on the
specified PMU for the hybrid platform, this commit extends the "--cputype"
support to perf record.
Without "--cputype", it reports events for both cpu_core and cpu_atom.
# ./perf record -e cycles -a sleep 1 | ./perf report
# To display the perf.data header info, please use --header/--header-only options.
#
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.000 MB (null) ]
#
# Total Lost Samples: 0
#
# Samples: 335 of event 'cpu_core/cycles/'
# Event count (approx.): 35855267
#
# Overhead Command Shared Object Symbol
# ........ ............... ................. .........................................
#
10.31% swapper [kernel.kallsyms] [k] poll_idle
9.42% swapper [kernel.kallsyms] [k] menu_select
... ... ... ... ...
# Samples: 61 of event 'cpu_atom/cycles/'
# Event count (approx.): 16453825
#
# Overhead Command Shared Object Symbol
# ........ ............. ................. ......................................
#
26.36% snapd [unknown] [.] 0x0000563cc6d03841
7.43% migration/13 [kernel.kallsyms] [k] update_sd_lb_stats.constprop.0
... ... ... ... ...
With "--cputype", it reports events only for the specified PMU.
# ./perf record --cputype core -e cycles -a sleep 1 | ./perf report
# To display the perf.data header info, please use --header/--header-only options.
#
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.000 MB (null) ]
#
# Total Lost Samples: 0
#
# Samples: 221 of event 'cpu_core/cycles/'
# Event count (approx.): 27121818
#
# Overhead Command Shared Object Symbol
# ........ ............... ................. .........................................
#
11.24% swapper [kernel.kallsyms] [k] e1000_irq_enable
7.77% swapper [kernel.kallsyms] [k] mwait_idle_with_hints.constprop.0
... ... ... ... ...
Signed-off-by: Zhengjun Xing <zhengjun.xing@linux.intel.com>
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
---
tools/perf/Documentation/perf-record.txt | 4 ++++
tools/perf/builtin-record.c | 3 +++
tools/perf/builtin-stat.c | 20 --------------------
tools/perf/util/pmu-hybrid.c | 19 +++++++++++++++++++
tools/perf/util/pmu-hybrid.h | 2 ++
5 files changed, 28 insertions(+), 20 deletions(-)
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index cf8ad50f3de1..ba8d680da1ac 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -402,6 +402,10 @@ Enable weightened sampling. An additional weight is recorded per sample and can
displayed with the weight and local_weight sort keys. This currently works for TSX
abort events and some memory events in precise mode on modern Intel CPUs.
+--cputype::
+Only enable events on applying cpu with this type for hybrid platform(e.g. core or atom).
+For non-hybrid events, it should be no effect.
+
--namespaces::
Record events of type PERF_RECORD_NAMESPACES. This enables 'cgroup_id' sort key.
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 9a71f0330137..e1edd4e98358 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -3183,6 +3183,9 @@ static struct option __record_options[] = {
OPT_INCR('v', "verbose", &verbose,
"be more verbose (show counter open errors, etc)"),
OPT_BOOLEAN('q', "quiet", &quiet, "don't print any message"),
+ OPT_CALLBACK(0, "cputype", &record.evlist, "hybrid cpu type",
+ "Only enable events on applying cpu with this type for hybrid platform (e.g. core or atom)",
+ parse_hybrid_type),
OPT_BOOLEAN('s', "stat", &record.opts.inherit_stat,
"per thread counts"),
OPT_BOOLEAN('d', "data", &record.opts.sample_address, "Record the sample addresses"),
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 4ce87a8eb7d7..0d95b29273f4 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1184,26 +1184,6 @@ static int parse_stat_cgroups(const struct option *opt,
return parse_cgroups(opt, str, unset);
}
-static int parse_hybrid_type(const struct option *opt,
- const char *str,
- int unset __maybe_unused)
-{
- struct evlist *evlist = *(struct evlist **)opt->value;
-
- if (!list_empty(&evlist->core.entries)) {
- fprintf(stderr, "Must define cputype before events/metrics\n");
- return -1;
- }
-
- evlist->hybrid_pmu_name = perf_pmu__hybrid_type_to_pmu(str);
- if (!evlist->hybrid_pmu_name) {
- fprintf(stderr, "--cputype %s is not supported!\n", str);
- return -1;
- }
-
- return 0;
-}
-
static struct option stat_options[] = {
OPT_BOOLEAN('T', "transaction", &transaction_run,
"hardware transaction statistics"),
diff --git a/tools/perf/util/pmu-hybrid.c b/tools/perf/util/pmu-hybrid.c
index f51ccaac60ee..5c490b5201b7 100644
--- a/tools/perf/util/pmu-hybrid.c
+++ b/tools/perf/util/pmu-hybrid.c
@@ -13,6 +13,7 @@
#include <stdarg.h>
#include <locale.h>
#include <api/fs/fs.h>
+#include "util/evlist.h"
#include "fncache.h"
#include "pmu-hybrid.h"
@@ -87,3 +88,21 @@ char *perf_pmu__hybrid_type_to_pmu(const char *type)
free(pmu_name);
return NULL;
}
+
+int parse_hybrid_type(const struct option *opt, const char *str, int unset __maybe_unused)
+{
+ struct evlist *evlist = *(struct evlist **)opt->value;
+
+ if (!list_empty(&evlist->core.entries)) {
+ fprintf(stderr, "Must define cputype before events/metrics\n");
+ return -1;
+ }
+
+ evlist->hybrid_pmu_name = perf_pmu__hybrid_type_to_pmu(str);
+ if (!evlist->hybrid_pmu_name) {
+ fprintf(stderr, "--cputype %s is not supported!\n", str);
+ return -1;
+ }
+
+ return 0;
+}
diff --git a/tools/perf/util/pmu-hybrid.h b/tools/perf/util/pmu-hybrid.h
index 2b186c26a43e..26101f134a3a 100644
--- a/tools/perf/util/pmu-hybrid.h
+++ b/tools/perf/util/pmu-hybrid.h
@@ -5,6 +5,7 @@
#include <linux/perf_event.h>
#include <linux/compiler.h>
#include <linux/list.h>
+#include <subcmd/parse-options.h>
#include <stdbool.h>
#include "pmu.h"
@@ -18,6 +19,7 @@ bool perf_pmu__hybrid_mounted(const char *name);
struct perf_pmu *perf_pmu__find_hybrid_pmu(const char *name);
bool perf_pmu__is_hybrid(const char *name);
char *perf_pmu__hybrid_type_to_pmu(const char *type);
+int parse_hybrid_type(const struct option *opt, const char *str, int unset __maybe_unused);
static inline int perf_pmu__hybrid_pmu_num(void)
{
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] perf record: Support "--cputype" option for hybrid events 2022-06-15 15:08 [PATCH] perf record: Support "--cputype" option for hybrid events zhengjun.xing @ 2022-06-15 15:16 ` Ian Rogers 2022-06-16 8:31 ` Xing Zhengjun 0 siblings, 1 reply; 6+ messages in thread From: Ian Rogers @ 2022-06-15 15:16 UTC (permalink / raw) To: zhengjun.xing Cc: acme, peterz, mingo, alexander.shishkin, jolsa, namhyung, linux-kernel, linux-perf-users, ak, kan.liang On Wed, Jun 15, 2022 at 8:07 AM <zhengjun.xing@linux.intel.com> wrote: > > From: Zhengjun Xing <zhengjun.xing@linux.intel.com> > > perf stat already has the "--cputype" option to enable events only on the > specified PMU for the hybrid platform, this commit extends the "--cputype" > support to perf record. > > Without "--cputype", it reports events for both cpu_core and cpu_atom. > > # ./perf record -e cycles -a sleep 1 | ./perf report > > # To display the perf.data header info, please use --header/--header-only options. > # > [ perf record: Woken up 1 times to write data ] > [ perf record: Captured and wrote 0.000 MB (null) ] > # > # Total Lost Samples: 0 > # > # Samples: 335 of event 'cpu_core/cycles/' > # Event count (approx.): 35855267 > # > # Overhead Command Shared Object Symbol > # ........ ............... ................. ......................................... > # > 10.31% swapper [kernel.kallsyms] [k] poll_idle > 9.42% swapper [kernel.kallsyms] [k] menu_select > ... ... ... ... ... > > # Samples: 61 of event 'cpu_atom/cycles/' > # Event count (approx.): 16453825 > # > # Overhead Command Shared Object Symbol > # ........ ............. ................. ...................................... > # > 26.36% snapd [unknown] [.] 0x0000563cc6d03841 > 7.43% migration/13 [kernel.kallsyms] [k] update_sd_lb_stats.constprop.0 > ... ... ... ... ... > > With "--cputype", it reports events only for the specified PMU. > > # ./perf record --cputype core -e cycles -a sleep 1 | ./perf report > > # To display the perf.data header info, please use --header/--header-only options. > # > [ perf record: Woken up 1 times to write data ] > [ perf record: Captured and wrote 0.000 MB (null) ] > # > # Total Lost Samples: 0 > # > # Samples: 221 of event 'cpu_core/cycles/' > # Event count (approx.): 27121818 > # > # Overhead Command Shared Object Symbol > # ........ ............... ................. ......................................... > # > 11.24% swapper [kernel.kallsyms] [k] e1000_irq_enable > 7.77% swapper [kernel.kallsyms] [k] mwait_idle_with_hints.constprop.0 > ... ... ... ... ... This is already possible by having the PMU name as part of the event, cpu_atom/cycles/ or cpu_core/cycles/. I don't know why we're adding "hybrid" all over the code base, I wish it would stop. You are asking for an option here for an implied PMU for events that don't specify a PMU. The option should be called --pmutype and consider all PMUs. We should remove the "hybrid" PMU list and make all of the "hybrid" code generic. Thanks, Ian > Signed-off-by: Zhengjun Xing <zhengjun.xing@linux.intel.com> > Reviewed-by: Kan Liang <kan.liang@linux.intel.com> > --- > tools/perf/Documentation/perf-record.txt | 4 ++++ > tools/perf/builtin-record.c | 3 +++ > tools/perf/builtin-stat.c | 20 -------------------- > tools/perf/util/pmu-hybrid.c | 19 +++++++++++++++++++ > tools/perf/util/pmu-hybrid.h | 2 ++ > 5 files changed, 28 insertions(+), 20 deletions(-) > > diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt > index cf8ad50f3de1..ba8d680da1ac 100644 > --- a/tools/perf/Documentation/perf-record.txt > +++ b/tools/perf/Documentation/perf-record.txt > @@ -402,6 +402,10 @@ Enable weightened sampling. An additional weight is recorded per sample and can > displayed with the weight and local_weight sort keys. This currently works for TSX > abort events and some memory events in precise mode on modern Intel CPUs. > > +--cputype:: > +Only enable events on applying cpu with this type for hybrid platform(e.g. core or atom). > +For non-hybrid events, it should be no effect. > + > --namespaces:: > Record events of type PERF_RECORD_NAMESPACES. This enables 'cgroup_id' sort key. > > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c > index 9a71f0330137..e1edd4e98358 100644 > --- a/tools/perf/builtin-record.c > +++ b/tools/perf/builtin-record.c > @@ -3183,6 +3183,9 @@ static struct option __record_options[] = { > OPT_INCR('v', "verbose", &verbose, > "be more verbose (show counter open errors, etc)"), > OPT_BOOLEAN('q', "quiet", &quiet, "don't print any message"), > + OPT_CALLBACK(0, "cputype", &record.evlist, "hybrid cpu type", > + "Only enable events on applying cpu with this type for hybrid platform (e.g. core or atom)", > + parse_hybrid_type), > OPT_BOOLEAN('s', "stat", &record.opts.inherit_stat, > "per thread counts"), > OPT_BOOLEAN('d', "data", &record.opts.sample_address, "Record the sample addresses"), > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c > index 4ce87a8eb7d7..0d95b29273f4 100644 > --- a/tools/perf/builtin-stat.c > +++ b/tools/perf/builtin-stat.c > @@ -1184,26 +1184,6 @@ static int parse_stat_cgroups(const struct option *opt, > return parse_cgroups(opt, str, unset); > } > > -static int parse_hybrid_type(const struct option *opt, > - const char *str, > - int unset __maybe_unused) > -{ > - struct evlist *evlist = *(struct evlist **)opt->value; > - > - if (!list_empty(&evlist->core.entries)) { > - fprintf(stderr, "Must define cputype before events/metrics\n"); > - return -1; > - } > - > - evlist->hybrid_pmu_name = perf_pmu__hybrid_type_to_pmu(str); > - if (!evlist->hybrid_pmu_name) { > - fprintf(stderr, "--cputype %s is not supported!\n", str); > - return -1; > - } > - > - return 0; > -} > - > static struct option stat_options[] = { > OPT_BOOLEAN('T', "transaction", &transaction_run, > "hardware transaction statistics"), > diff --git a/tools/perf/util/pmu-hybrid.c b/tools/perf/util/pmu-hybrid.c > index f51ccaac60ee..5c490b5201b7 100644 > --- a/tools/perf/util/pmu-hybrid.c > +++ b/tools/perf/util/pmu-hybrid.c > @@ -13,6 +13,7 @@ > #include <stdarg.h> > #include <locale.h> > #include <api/fs/fs.h> > +#include "util/evlist.h" > #include "fncache.h" > #include "pmu-hybrid.h" > > @@ -87,3 +88,21 @@ char *perf_pmu__hybrid_type_to_pmu(const char *type) > free(pmu_name); > return NULL; > } > + > +int parse_hybrid_type(const struct option *opt, const char *str, int unset __maybe_unused) > +{ > + struct evlist *evlist = *(struct evlist **)opt->value; > + > + if (!list_empty(&evlist->core.entries)) { > + fprintf(stderr, "Must define cputype before events/metrics\n"); > + return -1; > + } > + > + evlist->hybrid_pmu_name = perf_pmu__hybrid_type_to_pmu(str); > + if (!evlist->hybrid_pmu_name) { > + fprintf(stderr, "--cputype %s is not supported!\n", str); > + return -1; > + } > + > + return 0; > +} > diff --git a/tools/perf/util/pmu-hybrid.h b/tools/perf/util/pmu-hybrid.h > index 2b186c26a43e..26101f134a3a 100644 > --- a/tools/perf/util/pmu-hybrid.h > +++ b/tools/perf/util/pmu-hybrid.h > @@ -5,6 +5,7 @@ > #include <linux/perf_event.h> > #include <linux/compiler.h> > #include <linux/list.h> > +#include <subcmd/parse-options.h> > #include <stdbool.h> > #include "pmu.h" > > @@ -18,6 +19,7 @@ bool perf_pmu__hybrid_mounted(const char *name); > struct perf_pmu *perf_pmu__find_hybrid_pmu(const char *name); > bool perf_pmu__is_hybrid(const char *name); > char *perf_pmu__hybrid_type_to_pmu(const char *type); > +int parse_hybrid_type(const struct option *opt, const char *str, int unset __maybe_unused); > > static inline int perf_pmu__hybrid_pmu_num(void) > { > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf record: Support "--cputype" option for hybrid events 2022-06-15 15:16 ` Ian Rogers @ 2022-06-16 8:31 ` Xing Zhengjun 2022-06-16 14:31 ` Liang, Kan 0 siblings, 1 reply; 6+ messages in thread From: Xing Zhengjun @ 2022-06-16 8:31 UTC (permalink / raw) To: Ian Rogers Cc: acme, peterz, mingo, alexander.shishkin, jolsa, namhyung, linux-kernel, linux-perf-users, ak, kan.liang Hi Ian, On 6/15/2022 11:16 PM, Ian Rogers wrote: > On Wed, Jun 15, 2022 at 8:07 AM <zhengjun.xing@linux.intel.com> wrote: >> >> From: Zhengjun Xing <zhengjun.xing@linux.intel.com> >> >> perf stat already has the "--cputype" option to enable events only on the >> specified PMU for the hybrid platform, this commit extends the "--cputype" >> support to perf record. >> >> Without "--cputype", it reports events for both cpu_core and cpu_atom. >> >> # ./perf record -e cycles -a sleep 1 | ./perf report >> >> # To display the perf.data header info, please use --header/--header-only options. >> # >> [ perf record: Woken up 1 times to write data ] >> [ perf record: Captured and wrote 0.000 MB (null) ] >> # >> # Total Lost Samples: 0 >> # >> # Samples: 335 of event 'cpu_core/cycles/' >> # Event count (approx.): 35855267 >> # >> # Overhead Command Shared Object Symbol >> # ........ ............... ................. ......................................... >> # >> 10.31% swapper [kernel.kallsyms] [k] poll_idle >> 9.42% swapper [kernel.kallsyms] [k] menu_select >> ... ... ... ... ... >> >> # Samples: 61 of event 'cpu_atom/cycles/' >> # Event count (approx.): 16453825 >> # >> # Overhead Command Shared Object Symbol >> # ........ ............. ................. ...................................... >> # >> 26.36% snapd [unknown] [.] 0x0000563cc6d03841 >> 7.43% migration/13 [kernel.kallsyms] [k] update_sd_lb_stats.constprop.0 >> ... ... ... ... ... >> >> With "--cputype", it reports events only for the specified PMU. >> >> # ./perf record --cputype core -e cycles -a sleep 1 | ./perf report >> >> # To display the perf.data header info, please use --header/--header-only options. >> # >> [ perf record: Woken up 1 times to write data ] >> [ perf record: Captured and wrote 0.000 MB (null) ] >> # >> # Total Lost Samples: 0 >> # >> # Samples: 221 of event 'cpu_core/cycles/' >> # Event count (approx.): 27121818 >> # >> # Overhead Command Shared Object Symbol >> # ........ ............... ................. ......................................... >> # >> 11.24% swapper [kernel.kallsyms] [k] e1000_irq_enable >> 7.77% swapper [kernel.kallsyms] [k] mwait_idle_with_hints.constprop.0 >> ... ... ... ... ... > > This is already possible by having the PMU name as part of the event, > cpu_atom/cycles/ or cpu_core/cycles/. I don't know why we're adding > "hybrid" all over the code base, I wish it would stop. You are asking > for an option here for an implied PMU for events that don't specify a > PMU. The option should be called --pmutype and consider all PMUs. We > should remove the "hybrid" PMU list and make all of the "hybrid" code > generic. > I can change the option name from "cputype" to "pmutype". We have planned to clean up the hybrid code, and try to reduce the code with "if perf_pmu__has_hybrid()", this will be done step by step, from this patch, we have begun to do some clean-up, move the hybrid API from the common file to the hybrid-specific files. For the clean-up, Do you have any ideas? Thanks. > Thanks, > Ian > >> Signed-off-by: Zhengjun Xing <zhengjun.xing@linux.intel.com> >> Reviewed-by: Kan Liang <kan.liang@linux.intel.com> >> --- >> tools/perf/Documentation/perf-record.txt | 4 ++++ >> tools/perf/builtin-record.c | 3 +++ >> tools/perf/builtin-stat.c | 20 -------------------- >> tools/perf/util/pmu-hybrid.c | 19 +++++++++++++++++++ >> tools/perf/util/pmu-hybrid.h | 2 ++ >> 5 files changed, 28 insertions(+), 20 deletions(-) >> >> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt >> index cf8ad50f3de1..ba8d680da1ac 100644 >> --- a/tools/perf/Documentation/perf-record.txt >> +++ b/tools/perf/Documentation/perf-record.txt >> @@ -402,6 +402,10 @@ Enable weightened sampling. An additional weight is recorded per sample and can >> displayed with the weight and local_weight sort keys. This currently works for TSX >> abort events and some memory events in precise mode on modern Intel CPUs. >> >> +--cputype:: >> +Only enable events on applying cpu with this type for hybrid platform(e.g. core or atom). >> +For non-hybrid events, it should be no effect. >> + >> --namespaces:: >> Record events of type PERF_RECORD_NAMESPACES. This enables 'cgroup_id' sort key. >> >> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c >> index 9a71f0330137..e1edd4e98358 100644 >> --- a/tools/perf/builtin-record.c >> +++ b/tools/perf/builtin-record.c >> @@ -3183,6 +3183,9 @@ static struct option __record_options[] = { >> OPT_INCR('v', "verbose", &verbose, >> "be more verbose (show counter open errors, etc)"), >> OPT_BOOLEAN('q', "quiet", &quiet, "don't print any message"), >> + OPT_CALLBACK(0, "cputype", &record.evlist, "hybrid cpu type", >> + "Only enable events on applying cpu with this type for hybrid platform (e.g. core or atom)", >> + parse_hybrid_type), >> OPT_BOOLEAN('s', "stat", &record.opts.inherit_stat, >> "per thread counts"), >> OPT_BOOLEAN('d', "data", &record.opts.sample_address, "Record the sample addresses"), >> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c >> index 4ce87a8eb7d7..0d95b29273f4 100644 >> --- a/tools/perf/builtin-stat.c >> +++ b/tools/perf/builtin-stat.c >> @@ -1184,26 +1184,6 @@ static int parse_stat_cgroups(const struct option *opt, >> return parse_cgroups(opt, str, unset); >> } >> >> -static int parse_hybrid_type(const struct option *opt, >> - const char *str, >> - int unset __maybe_unused) >> -{ >> - struct evlist *evlist = *(struct evlist **)opt->value; >> - >> - if (!list_empty(&evlist->core.entries)) { >> - fprintf(stderr, "Must define cputype before events/metrics\n"); >> - return -1; >> - } >> - >> - evlist->hybrid_pmu_name = perf_pmu__hybrid_type_to_pmu(str); >> - if (!evlist->hybrid_pmu_name) { >> - fprintf(stderr, "--cputype %s is not supported!\n", str); >> - return -1; >> - } >> - >> - return 0; >> -} >> - >> static struct option stat_options[] = { >> OPT_BOOLEAN('T', "transaction", &transaction_run, >> "hardware transaction statistics"), >> diff --git a/tools/perf/util/pmu-hybrid.c b/tools/perf/util/pmu-hybrid.c >> index f51ccaac60ee..5c490b5201b7 100644 >> --- a/tools/perf/util/pmu-hybrid.c >> +++ b/tools/perf/util/pmu-hybrid.c >> @@ -13,6 +13,7 @@ >> #include <stdarg.h> >> #include <locale.h> >> #include <api/fs/fs.h> >> +#include "util/evlist.h" >> #include "fncache.h" >> #include "pmu-hybrid.h" >> >> @@ -87,3 +88,21 @@ char *perf_pmu__hybrid_type_to_pmu(const char *type) >> free(pmu_name); >> return NULL; >> } >> + >> +int parse_hybrid_type(const struct option *opt, const char *str, int unset __maybe_unused) >> +{ >> + struct evlist *evlist = *(struct evlist **)opt->value; >> + >> + if (!list_empty(&evlist->core.entries)) { >> + fprintf(stderr, "Must define cputype before events/metrics\n"); >> + return -1; >> + } >> + >> + evlist->hybrid_pmu_name = perf_pmu__hybrid_type_to_pmu(str); >> + if (!evlist->hybrid_pmu_name) { >> + fprintf(stderr, "--cputype %s is not supported!\n", str); >> + return -1; >> + } >> + >> + return 0; >> +} >> diff --git a/tools/perf/util/pmu-hybrid.h b/tools/perf/util/pmu-hybrid.h >> index 2b186c26a43e..26101f134a3a 100644 >> --- a/tools/perf/util/pmu-hybrid.h >> +++ b/tools/perf/util/pmu-hybrid.h >> @@ -5,6 +5,7 @@ >> #include <linux/perf_event.h> >> #include <linux/compiler.h> >> #include <linux/list.h> >> +#include <subcmd/parse-options.h> >> #include <stdbool.h> >> #include "pmu.h" >> >> @@ -18,6 +19,7 @@ bool perf_pmu__hybrid_mounted(const char *name); >> struct perf_pmu *perf_pmu__find_hybrid_pmu(const char *name); >> bool perf_pmu__is_hybrid(const char *name); >> char *perf_pmu__hybrid_type_to_pmu(const char *type); >> +int parse_hybrid_type(const struct option *opt, const char *str, int unset __maybe_unused); >> >> static inline int perf_pmu__hybrid_pmu_num(void) >> { >> -- >> 2.25.1 >> -- Zhengjun Xing ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf record: Support "--cputype" option for hybrid events 2022-06-16 8:31 ` Xing Zhengjun @ 2022-06-16 14:31 ` Liang, Kan 2022-06-21 5:13 ` Xing Zhengjun 0 siblings, 1 reply; 6+ messages in thread From: Liang, Kan @ 2022-06-16 14:31 UTC (permalink / raw) To: Xing Zhengjun, Ian Rogers Cc: acme, peterz, mingo, alexander.shishkin, jolsa, namhyung, linux-kernel, linux-perf-users, ak On 6/16/2022 4:31 AM, Xing Zhengjun wrote: > Hi Ian, > > On 6/15/2022 11:16 PM, Ian Rogers wrote: >> On Wed, Jun 15, 2022 at 8:07 AM <zhengjun.xing@linux.intel.com> wrote: >>> >>> From: Zhengjun Xing <zhengjun.xing@linux.intel.com> >>> >>> perf stat already has the "--cputype" option to enable events only on >>> the >>> specified PMU for the hybrid platform, this commit extends the >>> "--cputype" >>> support to perf record. >>> >>> Without "--cputype", it reports events for both cpu_core and cpu_atom. >>> >>> # ./perf record -e cycles -a sleep 1 | ./perf report >>> >>> # To display the perf.data header info, please use >>> --header/--header-only options. >>> # >>> [ perf record: Woken up 1 times to write data ] >>> [ perf record: Captured and wrote 0.000 MB (null) ] >>> # >>> # Total Lost Samples: 0 >>> # >>> # Samples: 335 of event 'cpu_core/cycles/' >>> # Event count (approx.): 35855267 >>> # >>> # Overhead Command Shared Object Symbol >>> # ........ ............... ................. >>> ......................................... >>> # >>> 10.31% swapper [kernel.kallsyms] [k] poll_idle >>> 9.42% swapper [kernel.kallsyms] [k] menu_select >>> ... ... ... ... ... >>> >>> # Samples: 61 of event 'cpu_atom/cycles/' >>> # Event count (approx.): 16453825 >>> # >>> # Overhead Command Shared Object Symbol >>> # ........ ............. ................. >>> ...................................... >>> # >>> 26.36% snapd [unknown] [.] 0x0000563cc6d03841 >>> 7.43% migration/13 [kernel.kallsyms] [k] >>> update_sd_lb_stats.constprop.0 >>> ... ... ... ... ... >>> >>> With "--cputype", it reports events only for the specified PMU. >>> >>> # ./perf record --cputype core -e cycles -a sleep 1 | ./perf report >>> >>> # To display the perf.data header info, please use >>> --header/--header-only options. >>> # >>> [ perf record: Woken up 1 times to write data ] >>> [ perf record: Captured and wrote 0.000 MB (null) ] >>> # >>> # Total Lost Samples: 0 >>> # >>> # Samples: 221 of event 'cpu_core/cycles/' >>> # Event count (approx.): 27121818 >>> # >>> # Overhead Command Shared Object Symbol >>> # ........ ............... ................. >>> ......................................... >>> # >>> 11.24% swapper [kernel.kallsyms] [k] e1000_irq_enable >>> 7.77% swapper [kernel.kallsyms] [k] >>> mwait_idle_with_hints.constprop.0 >>> ... ... ... ... ... >> >> This is already possible by having the PMU name as part of the event, >> cpu_atom/cycles/ or cpu_core/cycles/. I don't know why we're adding >> "hybrid" all over the code base, I wish it would stop. You are asking >> for an option here for an implied PMU for events that don't specify a >> PMU. The option should be called --pmutype and consider all PMUs. The --pmutype should be redundant because other PMUs have to specify the PMU name with the event. Only the CPU type of PMUs have events which doesn't have a PMU name prefix, e.g., cycles, instructions. So the "--cputype" was introduced for perf stat to avoid the annoying pmu prefix for the CPU type of PMU. This patch just extends the "--cputype" to perf record to address the request from the community. It reuses the existing function. >> We >> should remove the "hybrid" PMU list and make all of the "hybrid" code >> generic. We already start to rework on the "hybrid" code. We recently rework the perf stat default https://lore.kernel.org/lkml/20220610025449.2089232-1-zhengjun.xing@linux.intel.com/ With the help of Ravi, we clean up the hybrid CPU in the perf header. https://lore.kernel.org/all/20220604044519.594-6-ravi.bangoria@amd.com/ I think Zhengjun will work on the perf record default cleanup shortly. Then I guess we can further clean up the "--cputype", e.g., removing hybrid_pmu_name from evlist... as next step. Thanks, Kan >> > > I can change the option name from "cputype" to "pmutype". We have > planned to clean up the hybrid code, and try to reduce the code with "if > perf_pmu__has_hybrid()", this will be done step by step, from this > patch, we have begun to do some clean-up, move the hybrid API from the > common file to the hybrid-specific files. For the clean-up, Do you have > any ideas? Thanks. > >> Thanks, >> Ian >> >>> Signed-off-by: Zhengjun Xing <zhengjun.xing@linux.intel.com> >>> Reviewed-by: Kan Liang <kan.liang@linux.intel.com> >>> --- >>> tools/perf/Documentation/perf-record.txt | 4 ++++ >>> tools/perf/builtin-record.c | 3 +++ >>> tools/perf/builtin-stat.c | 20 -------------------- >>> tools/perf/util/pmu-hybrid.c | 19 +++++++++++++++++++ >>> tools/perf/util/pmu-hybrid.h | 2 ++ >>> 5 files changed, 28 insertions(+), 20 deletions(-) >>> >>> diff --git a/tools/perf/Documentation/perf-record.txt >>> b/tools/perf/Documentation/perf-record.txt >>> index cf8ad50f3de1..ba8d680da1ac 100644 >>> --- a/tools/perf/Documentation/perf-record.txt >>> +++ b/tools/perf/Documentation/perf-record.txt >>> @@ -402,6 +402,10 @@ Enable weightened sampling. An additional weight >>> is recorded per sample and can >>> displayed with the weight and local_weight sort keys. This >>> currently works for TSX >>> abort events and some memory events in precise mode on modern Intel >>> CPUs. >>> >>> +--cputype:: >>> +Only enable events on applying cpu with this type for hybrid >>> platform(e.g. core or atom). >>> +For non-hybrid events, it should be no effect. >>> + >>> --namespaces:: >>> Record events of type PERF_RECORD_NAMESPACES. This enables >>> 'cgroup_id' sort key. >>> >>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c >>> index 9a71f0330137..e1edd4e98358 100644 >>> --- a/tools/perf/builtin-record.c >>> +++ b/tools/perf/builtin-record.c >>> @@ -3183,6 +3183,9 @@ static struct option __record_options[] = { >>> OPT_INCR('v', "verbose", &verbose, >>> "be more verbose (show counter open errors, etc)"), >>> OPT_BOOLEAN('q', "quiet", &quiet, "don't print any message"), >>> + OPT_CALLBACK(0, "cputype", &record.evlist, "hybrid cpu type", >>> + "Only enable events on applying cpu with this >>> type for hybrid platform (e.g. core or atom)", >>> + parse_hybrid_type), >>> OPT_BOOLEAN('s', "stat", &record.opts.inherit_stat, >>> "per thread counts"), >>> OPT_BOOLEAN('d', "data", &record.opts.sample_address, >>> "Record the sample addresses"), >>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c >>> index 4ce87a8eb7d7..0d95b29273f4 100644 >>> --- a/tools/perf/builtin-stat.c >>> +++ b/tools/perf/builtin-stat.c >>> @@ -1184,26 +1184,6 @@ static int parse_stat_cgroups(const struct >>> option *opt, >>> return parse_cgroups(opt, str, unset); >>> } >>> >>> -static int parse_hybrid_type(const struct option *opt, >>> - const char *str, >>> - int unset __maybe_unused) >>> -{ >>> - struct evlist *evlist = *(struct evlist **)opt->value; >>> - >>> - if (!list_empty(&evlist->core.entries)) { >>> - fprintf(stderr, "Must define cputype before >>> events/metrics\n"); >>> - return -1; >>> - } >>> - >>> - evlist->hybrid_pmu_name = perf_pmu__hybrid_type_to_pmu(str); >>> - if (!evlist->hybrid_pmu_name) { >>> - fprintf(stderr, "--cputype %s is not supported!\n", >>> str); >>> - return -1; >>> - } >>> - >>> - return 0; >>> -} >>> - >>> static struct option stat_options[] = { >>> OPT_BOOLEAN('T', "transaction", &transaction_run, >>> "hardware transaction statistics"), >>> diff --git a/tools/perf/util/pmu-hybrid.c b/tools/perf/util/pmu-hybrid.c >>> index f51ccaac60ee..5c490b5201b7 100644 >>> --- a/tools/perf/util/pmu-hybrid.c >>> +++ b/tools/perf/util/pmu-hybrid.c >>> @@ -13,6 +13,7 @@ >>> #include <stdarg.h> >>> #include <locale.h> >>> #include <api/fs/fs.h> >>> +#include "util/evlist.h" >>> #include "fncache.h" >>> #include "pmu-hybrid.h" >>> >>> @@ -87,3 +88,21 @@ char *perf_pmu__hybrid_type_to_pmu(const char *type) >>> free(pmu_name); >>> return NULL; >>> } >>> + >>> +int parse_hybrid_type(const struct option *opt, const char *str, int >>> unset __maybe_unused) >>> +{ >>> + struct evlist *evlist = *(struct evlist **)opt->value; >>> + >>> + if (!list_empty(&evlist->core.entries)) { >>> + fprintf(stderr, "Must define cputype before >>> events/metrics\n"); >>> + return -1; >>> + } >>> + >>> + evlist->hybrid_pmu_name = perf_pmu__hybrid_type_to_pmu(str); >>> + if (!evlist->hybrid_pmu_name) { >>> + fprintf(stderr, "--cputype %s is not supported!\n", >>> str); >>> + return -1; >>> + } >>> + >>> + return 0; >>> +} >>> diff --git a/tools/perf/util/pmu-hybrid.h b/tools/perf/util/pmu-hybrid.h >>> index 2b186c26a43e..26101f134a3a 100644 >>> --- a/tools/perf/util/pmu-hybrid.h >>> +++ b/tools/perf/util/pmu-hybrid.h >>> @@ -5,6 +5,7 @@ >>> #include <linux/perf_event.h> >>> #include <linux/compiler.h> >>> #include <linux/list.h> >>> +#include <subcmd/parse-options.h> >>> #include <stdbool.h> >>> #include "pmu.h" >>> >>> @@ -18,6 +19,7 @@ bool perf_pmu__hybrid_mounted(const char *name); >>> struct perf_pmu *perf_pmu__find_hybrid_pmu(const char *name); >>> bool perf_pmu__is_hybrid(const char *name); >>> char *perf_pmu__hybrid_type_to_pmu(const char *type); >>> +int parse_hybrid_type(const struct option *opt, const char *str, int >>> unset __maybe_unused); >>> >>> static inline int perf_pmu__hybrid_pmu_num(void) >>> { >>> -- >>> 2.25.1 >>> > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf record: Support "--cputype" option for hybrid events 2022-06-16 14:31 ` Liang, Kan @ 2022-06-21 5:13 ` Xing Zhengjun 2022-07-04 9:10 ` Xing Zhengjun 0 siblings, 1 reply; 6+ messages in thread From: Xing Zhengjun @ 2022-06-21 5:13 UTC (permalink / raw) To: Ian Rogers Cc: acme, peterz, mingo, alexander.shishkin, jolsa, namhyung, linux-kernel, linux-perf-users, ak, Liang, Kan Hi Ian, On 6/16/2022 10:31 PM, Liang, Kan wrote: > > > On 6/16/2022 4:31 AM, Xing Zhengjun wrote: >> Hi Ian, >> >> On 6/15/2022 11:16 PM, Ian Rogers wrote: >>> On Wed, Jun 15, 2022 at 8:07 AM <zhengjun.xing@linux.intel.com> wrote: >>>> >>>> From: Zhengjun Xing <zhengjun.xing@linux.intel.com> >>>> >>>> perf stat already has the "--cputype" option to enable events only >>>> on the >>>> specified PMU for the hybrid platform, this commit extends the >>>> "--cputype" >>>> support to perf record. >>>> >>>> Without "--cputype", it reports events for both cpu_core and cpu_atom. >>>> >>>> # ./perf record -e cycles -a sleep 1 | ./perf report >>>> >>>> # To display the perf.data header info, please use >>>> --header/--header-only options. >>>> # >>>> [ perf record: Woken up 1 times to write data ] >>>> [ perf record: Captured and wrote 0.000 MB (null) ] >>>> # >>>> # Total Lost Samples: 0 >>>> # >>>> # Samples: 335 of event 'cpu_core/cycles/' >>>> # Event count (approx.): 35855267 >>>> # >>>> # Overhead Command Shared Object Symbol >>>> # ........ ............... ................. >>>> ......................................... >>>> # >>>> 10.31% swapper [kernel.kallsyms] [k] poll_idle >>>> 9.42% swapper [kernel.kallsyms] [k] menu_select >>>> ... ... ... ... ... >>>> >>>> # Samples: 61 of event 'cpu_atom/cycles/' >>>> # Event count (approx.): 16453825 >>>> # >>>> # Overhead Command Shared Object Symbol >>>> # ........ ............. ................. >>>> ...................................... >>>> # >>>> 26.36% snapd [unknown] [.] 0x0000563cc6d03841 >>>> 7.43% migration/13 [kernel.kallsyms] [k] >>>> update_sd_lb_stats.constprop.0 >>>> ... ... ... ... ... >>>> >>>> With "--cputype", it reports events only for the specified PMU. >>>> >>>> # ./perf record --cputype core -e cycles -a sleep 1 | ./perf report >>>> >>>> # To display the perf.data header info, please use >>>> --header/--header-only options. >>>> # >>>> [ perf record: Woken up 1 times to write data ] >>>> [ perf record: Captured and wrote 0.000 MB (null) ] >>>> # >>>> # Total Lost Samples: 0 >>>> # >>>> # Samples: 221 of event 'cpu_core/cycles/' >>>> # Event count (approx.): 27121818 >>>> # >>>> # Overhead Command Shared Object Symbol >>>> # ........ ............... ................. >>>> ......................................... >>>> # >>>> 11.24% swapper [kernel.kallsyms] [k] e1000_irq_enable >>>> 7.77% swapper [kernel.kallsyms] [k] >>>> mwait_idle_with_hints.constprop.0 >>>> ... ... ... ... ... >>> >>> This is already possible by having the PMU name as part of the event, >>> cpu_atom/cycles/ or cpu_core/cycles/. I don't know why we're adding >>> "hybrid" all over the code base, I wish it would stop. You are asking >>> for an option here for an implied PMU for events that don't specify a >>> PMU. The option should be called --pmutype and consider all PMUs. > > The --pmutype should be redundant because other PMUs have to specify the > PMU name with the event. Only the CPU type of PMUs have events which > doesn't have a PMU name prefix, e.g., cycles, instructions. > So the "--cputype" was introduced for perf stat to avoid the annoying > pmu prefix for the CPU type of PMU. > > This patch just extends the "--cputype" to perf record to address the > request from the community. It reuses the existing function. > Yes, "--cputype" is better. Ian, what do you think? Thanks. >>> We >>> should remove the "hybrid" PMU list and make all of the "hybrid" code >>> generic. > > We already start to rework on the "hybrid" code. > > We recently rework the perf stat default > https://lore.kernel.org/lkml/20220610025449.2089232-1-zhengjun.xing@linux.intel.com/ > > > With the help of Ravi, we clean up the hybrid CPU in the perf header. > https://lore.kernel.org/all/20220604044519.594-6-ravi.bangoria@amd.com/ > > I think Zhengjun will work on the perf record default cleanup shortly. > > Then I guess we can further clean up the "--cputype", e.g., removing > hybrid_pmu_name from evlist... as next step. > > Thanks, > Kan >>> >> >> I can change the option name from "cputype" to "pmutype". We have >> planned to clean up the hybrid code, and try to reduce the code with >> "if perf_pmu__has_hybrid()", this will be done step by step, from this >> patch, we have begun to do some clean-up, move the hybrid API from the >> common file to the hybrid-specific files. For the clean-up, Do you >> have any ideas? Thanks. >> >>> Thanks, >>> Ian >>> >>>> Signed-off-by: Zhengjun Xing <zhengjun.xing@linux.intel.com> >>>> Reviewed-by: Kan Liang <kan.liang@linux.intel.com> >>>> --- >>>> tools/perf/Documentation/perf-record.txt | 4 ++++ >>>> tools/perf/builtin-record.c | 3 +++ >>>> tools/perf/builtin-stat.c | 20 -------------------- >>>> tools/perf/util/pmu-hybrid.c | 19 +++++++++++++++++++ >>>> tools/perf/util/pmu-hybrid.h | 2 ++ >>>> 5 files changed, 28 insertions(+), 20 deletions(-) >>>> >>>> diff --git a/tools/perf/Documentation/perf-record.txt >>>> b/tools/perf/Documentation/perf-record.txt >>>> index cf8ad50f3de1..ba8d680da1ac 100644 >>>> --- a/tools/perf/Documentation/perf-record.txt >>>> +++ b/tools/perf/Documentation/perf-record.txt >>>> @@ -402,6 +402,10 @@ Enable weightened sampling. An additional >>>> weight is recorded per sample and can >>>> displayed with the weight and local_weight sort keys. This >>>> currently works for TSX >>>> abort events and some memory events in precise mode on modern >>>> Intel CPUs. >>>> >>>> +--cputype:: >>>> +Only enable events on applying cpu with this type for hybrid >>>> platform(e.g. core or atom). >>>> +For non-hybrid events, it should be no effect. >>>> + >>>> --namespaces:: >>>> Record events of type PERF_RECORD_NAMESPACES. This enables >>>> 'cgroup_id' sort key. >>>> >>>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c >>>> index 9a71f0330137..e1edd4e98358 100644 >>>> --- a/tools/perf/builtin-record.c >>>> +++ b/tools/perf/builtin-record.c >>>> @@ -3183,6 +3183,9 @@ static struct option __record_options[] = { >>>> OPT_INCR('v', "verbose", &verbose, >>>> "be more verbose (show counter open errors, >>>> etc)"), >>>> OPT_BOOLEAN('q', "quiet", &quiet, "don't print any message"), >>>> + OPT_CALLBACK(0, "cputype", &record.evlist, "hybrid cpu type", >>>> + "Only enable events on applying cpu with this >>>> type for hybrid platform (e.g. core or atom)", >>>> + parse_hybrid_type), >>>> OPT_BOOLEAN('s', "stat", &record.opts.inherit_stat, >>>> "per thread counts"), >>>> OPT_BOOLEAN('d', "data", &record.opts.sample_address, >>>> "Record the sample addresses"), >>>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c >>>> index 4ce87a8eb7d7..0d95b29273f4 100644 >>>> --- a/tools/perf/builtin-stat.c >>>> +++ b/tools/perf/builtin-stat.c >>>> @@ -1184,26 +1184,6 @@ static int parse_stat_cgroups(const struct >>>> option *opt, >>>> return parse_cgroups(opt, str, unset); >>>> } >>>> >>>> -static int parse_hybrid_type(const struct option *opt, >>>> - const char *str, >>>> - int unset __maybe_unused) >>>> -{ >>>> - struct evlist *evlist = *(struct evlist **)opt->value; >>>> - >>>> - if (!list_empty(&evlist->core.entries)) { >>>> - fprintf(stderr, "Must define cputype before >>>> events/metrics\n"); >>>> - return -1; >>>> - } >>>> - >>>> - evlist->hybrid_pmu_name = perf_pmu__hybrid_type_to_pmu(str); >>>> - if (!evlist->hybrid_pmu_name) { >>>> - fprintf(stderr, "--cputype %s is not supported!\n", >>>> str); >>>> - return -1; >>>> - } >>>> - >>>> - return 0; >>>> -} >>>> - >>>> static struct option stat_options[] = { >>>> OPT_BOOLEAN('T', "transaction", &transaction_run, >>>> "hardware transaction statistics"), >>>> diff --git a/tools/perf/util/pmu-hybrid.c >>>> b/tools/perf/util/pmu-hybrid.c >>>> index f51ccaac60ee..5c490b5201b7 100644 >>>> --- a/tools/perf/util/pmu-hybrid.c >>>> +++ b/tools/perf/util/pmu-hybrid.c >>>> @@ -13,6 +13,7 @@ >>>> #include <stdarg.h> >>>> #include <locale.h> >>>> #include <api/fs/fs.h> >>>> +#include "util/evlist.h" >>>> #include "fncache.h" >>>> #include "pmu-hybrid.h" >>>> >>>> @@ -87,3 +88,21 @@ char *perf_pmu__hybrid_type_to_pmu(const char *type) >>>> free(pmu_name); >>>> return NULL; >>>> } >>>> + >>>> +int parse_hybrid_type(const struct option *opt, const char *str, >>>> int unset __maybe_unused) >>>> +{ >>>> + struct evlist *evlist = *(struct evlist **)opt->value; >>>> + >>>> + if (!list_empty(&evlist->core.entries)) { >>>> + fprintf(stderr, "Must define cputype before >>>> events/metrics\n"); >>>> + return -1; >>>> + } >>>> + >>>> + evlist->hybrid_pmu_name = perf_pmu__hybrid_type_to_pmu(str); >>>> + if (!evlist->hybrid_pmu_name) { >>>> + fprintf(stderr, "--cputype %s is not supported!\n", >>>> str); >>>> + return -1; >>>> + } >>>> + >>>> + return 0; >>>> +} >>>> diff --git a/tools/perf/util/pmu-hybrid.h >>>> b/tools/perf/util/pmu-hybrid.h >>>> index 2b186c26a43e..26101f134a3a 100644 >>>> --- a/tools/perf/util/pmu-hybrid.h >>>> +++ b/tools/perf/util/pmu-hybrid.h >>>> @@ -5,6 +5,7 @@ >>>> #include <linux/perf_event.h> >>>> #include <linux/compiler.h> >>>> #include <linux/list.h> >>>> +#include <subcmd/parse-options.h> >>>> #include <stdbool.h> >>>> #include "pmu.h" >>>> >>>> @@ -18,6 +19,7 @@ bool perf_pmu__hybrid_mounted(const char *name); >>>> struct perf_pmu *perf_pmu__find_hybrid_pmu(const char *name); >>>> bool perf_pmu__is_hybrid(const char *name); >>>> char *perf_pmu__hybrid_type_to_pmu(const char *type); >>>> +int parse_hybrid_type(const struct option *opt, const char *str, >>>> int unset __maybe_unused); >>>> >>>> static inline int perf_pmu__hybrid_pmu_num(void) >>>> { >>>> -- >>>> 2.25.1 >>>> >> -- Zhengjun Xing ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf record: Support "--cputype" option for hybrid events 2022-06-21 5:13 ` Xing Zhengjun @ 2022-07-04 9:10 ` Xing Zhengjun 0 siblings, 0 replies; 6+ messages in thread From: Xing Zhengjun @ 2022-07-04 9:10 UTC (permalink / raw) To: Ian Rogers Cc: acme, peterz, mingo, alexander.shishkin, jolsa, namhyung, linux-kernel, linux-perf-users, ak, Liang, Kan Hi Ian, On 6/21/2022 1:13 PM, Xing Zhengjun wrote: > Hi Ian, > > On 6/16/2022 10:31 PM, Liang, Kan wrote: >> >> >> On 6/16/2022 4:31 AM, Xing Zhengjun wrote: >>> Hi Ian, >>> >>> On 6/15/2022 11:16 PM, Ian Rogers wrote: >>>> On Wed, Jun 15, 2022 at 8:07 AM <zhengjun.xing@linux.intel.com> wrote: >>>>> >>>>> From: Zhengjun Xing <zhengjun.xing@linux.intel.com> >>>>> >>>>> perf stat already has the "--cputype" option to enable events only >>>>> on the >>>>> specified PMU for the hybrid platform, this commit extends the >>>>> "--cputype" >>>>> support to perf record. >>>>> >>>>> Without "--cputype", it reports events for both cpu_core and cpu_atom. >>>>> >>>>> # ./perf record -e cycles -a sleep 1 | ./perf report >>>>> >>>>> # To display the perf.data header info, please use >>>>> --header/--header-only options. >>>>> # >>>>> [ perf record: Woken up 1 times to write data ] >>>>> [ perf record: Captured and wrote 0.000 MB (null) ] >>>>> # >>>>> # Total Lost Samples: 0 >>>>> # >>>>> # Samples: 335 of event 'cpu_core/cycles/' >>>>> # Event count (approx.): 35855267 >>>>> # >>>>> # Overhead Command Shared Object Symbol >>>>> # ........ ............... ................. >>>>> ......................................... >>>>> # >>>>> 10.31% swapper [kernel.kallsyms] [k] poll_idle >>>>> 9.42% swapper [kernel.kallsyms] [k] menu_select >>>>> ... ... ... ... ... >>>>> >>>>> # Samples: 61 of event 'cpu_atom/cycles/' >>>>> # Event count (approx.): 16453825 >>>>> # >>>>> # Overhead Command Shared Object Symbol >>>>> # ........ ............. ................. >>>>> ...................................... >>>>> # >>>>> 26.36% snapd [unknown] [.] 0x0000563cc6d03841 >>>>> 7.43% migration/13 [kernel.kallsyms] [k] >>>>> update_sd_lb_stats.constprop.0 >>>>> ... ... ... ... ... >>>>> >>>>> With "--cputype", it reports events only for the specified PMU. >>>>> >>>>> # ./perf record --cputype core -e cycles -a sleep 1 | ./perf report >>>>> >>>>> # To display the perf.data header info, please use >>>>> --header/--header-only options. >>>>> # >>>>> [ perf record: Woken up 1 times to write data ] >>>>> [ perf record: Captured and wrote 0.000 MB (null) ] >>>>> # >>>>> # Total Lost Samples: 0 >>>>> # >>>>> # Samples: 221 of event 'cpu_core/cycles/' >>>>> # Event count (approx.): 27121818 >>>>> # >>>>> # Overhead Command Shared Object Symbol >>>>> # ........ ............... ................. >>>>> ......................................... >>>>> # >>>>> 11.24% swapper [kernel.kallsyms] [k] e1000_irq_enable >>>>> 7.77% swapper [kernel.kallsyms] [k] >>>>> mwait_idle_with_hints.constprop.0 >>>>> ... ... ... ... ... >>>> >>>> This is already possible by having the PMU name as part of the event, >>>> cpu_atom/cycles/ or cpu_core/cycles/. I don't know why we're adding >>>> "hybrid" all over the code base, I wish it would stop. You are asking >>>> for an option here for an implied PMU for events that don't specify a >>>> PMU. The option should be called --pmutype and consider all PMUs. >> >> The --pmutype should be redundant because other PMUs have to specify >> the PMU name with the event. Only the CPU type of PMUs have events >> which doesn't have a PMU name prefix, e.g., cycles, instructions. >> So the "--cputype" was introduced for perf stat to avoid the annoying >> pmu prefix for the CPU type of PMU. >> >> This patch just extends the "--cputype" to perf record to address the >> request from the community. It reuses the existing function. >> > > Yes, "--cputype" is better. Ian, what do you think? Thanks. Ian, Could you help to comment on it? Thanks. > >>>> We >>>> should remove the "hybrid" PMU list and make all of the "hybrid" code >>>> generic. >> >> We already start to rework on the "hybrid" code. >> >> We recently rework the perf stat default >> https://lore.kernel.org/lkml/20220610025449.2089232-1-zhengjun.xing@linux.intel.com/ >> >> >> With the help of Ravi, we clean up the hybrid CPU in the perf header. >> https://lore.kernel.org/all/20220604044519.594-6-ravi.bangoria@amd.com/ >> >> I think Zhengjun will work on the perf record default cleanup shortly. >> >> Then I guess we can further clean up the "--cputype", e.g., removing >> hybrid_pmu_name from evlist... as next step. >> >> Thanks, >> Kan >>>> >>> >>> I can change the option name from "cputype" to "pmutype". We have >>> planned to clean up the hybrid code, and try to reduce the code with >>> "if perf_pmu__has_hybrid()", this will be done step by step, from >>> this patch, we have begun to do some clean-up, move the hybrid API >>> from the common file to the hybrid-specific files. For the clean-up, >>> Do you have any ideas? Thanks. >>> >>>> Thanks, >>>> Ian >>>> >>>>> Signed-off-by: Zhengjun Xing <zhengjun.xing@linux.intel.com> >>>>> Reviewed-by: Kan Liang <kan.liang@linux.intel.com> >>>>> --- >>>>> tools/perf/Documentation/perf-record.txt | 4 ++++ >>>>> tools/perf/builtin-record.c | 3 +++ >>>>> tools/perf/builtin-stat.c | 20 -------------------- >>>>> tools/perf/util/pmu-hybrid.c | 19 +++++++++++++++++++ >>>>> tools/perf/util/pmu-hybrid.h | 2 ++ >>>>> 5 files changed, 28 insertions(+), 20 deletions(-) >>>>> >>>>> diff --git a/tools/perf/Documentation/perf-record.txt >>>>> b/tools/perf/Documentation/perf-record.txt >>>>> index cf8ad50f3de1..ba8d680da1ac 100644 >>>>> --- a/tools/perf/Documentation/perf-record.txt >>>>> +++ b/tools/perf/Documentation/perf-record.txt >>>>> @@ -402,6 +402,10 @@ Enable weightened sampling. An additional >>>>> weight is recorded per sample and can >>>>> displayed with the weight and local_weight sort keys. This >>>>> currently works for TSX >>>>> abort events and some memory events in precise mode on modern >>>>> Intel CPUs. >>>>> >>>>> +--cputype:: >>>>> +Only enable events on applying cpu with this type for hybrid >>>>> platform(e.g. core or atom). >>>>> +For non-hybrid events, it should be no effect. >>>>> + >>>>> --namespaces:: >>>>> Record events of type PERF_RECORD_NAMESPACES. This enables >>>>> 'cgroup_id' sort key. >>>>> >>>>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c >>>>> index 9a71f0330137..e1edd4e98358 100644 >>>>> --- a/tools/perf/builtin-record.c >>>>> +++ b/tools/perf/builtin-record.c >>>>> @@ -3183,6 +3183,9 @@ static struct option __record_options[] = { >>>>> OPT_INCR('v', "verbose", &verbose, >>>>> "be more verbose (show counter open errors, >>>>> etc)"), >>>>> OPT_BOOLEAN('q', "quiet", &quiet, "don't print any message"), >>>>> + OPT_CALLBACK(0, "cputype", &record.evlist, "hybrid cpu type", >>>>> + "Only enable events on applying cpu with this >>>>> type for hybrid platform (e.g. core or atom)", >>>>> + parse_hybrid_type), >>>>> OPT_BOOLEAN('s', "stat", &record.opts.inherit_stat, >>>>> "per thread counts"), >>>>> OPT_BOOLEAN('d', "data", &record.opts.sample_address, >>>>> "Record the sample addresses"), >>>>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c >>>>> index 4ce87a8eb7d7..0d95b29273f4 100644 >>>>> --- a/tools/perf/builtin-stat.c >>>>> +++ b/tools/perf/builtin-stat.c >>>>> @@ -1184,26 +1184,6 @@ static int parse_stat_cgroups(const struct >>>>> option *opt, >>>>> return parse_cgroups(opt, str, unset); >>>>> } >>>>> >>>>> -static int parse_hybrid_type(const struct option *opt, >>>>> - const char *str, >>>>> - int unset __maybe_unused) >>>>> -{ >>>>> - struct evlist *evlist = *(struct evlist **)opt->value; >>>>> - >>>>> - if (!list_empty(&evlist->core.entries)) { >>>>> - fprintf(stderr, "Must define cputype before >>>>> events/metrics\n"); >>>>> - return -1; >>>>> - } >>>>> - >>>>> - evlist->hybrid_pmu_name = perf_pmu__hybrid_type_to_pmu(str); >>>>> - if (!evlist->hybrid_pmu_name) { >>>>> - fprintf(stderr, "--cputype %s is not supported!\n", >>>>> str); >>>>> - return -1; >>>>> - } >>>>> - >>>>> - return 0; >>>>> -} >>>>> - >>>>> static struct option stat_options[] = { >>>>> OPT_BOOLEAN('T', "transaction", &transaction_run, >>>>> "hardware transaction statistics"), >>>>> diff --git a/tools/perf/util/pmu-hybrid.c >>>>> b/tools/perf/util/pmu-hybrid.c >>>>> index f51ccaac60ee..5c490b5201b7 100644 >>>>> --- a/tools/perf/util/pmu-hybrid.c >>>>> +++ b/tools/perf/util/pmu-hybrid.c >>>>> @@ -13,6 +13,7 @@ >>>>> #include <stdarg.h> >>>>> #include <locale.h> >>>>> #include <api/fs/fs.h> >>>>> +#include "util/evlist.h" >>>>> #include "fncache.h" >>>>> #include "pmu-hybrid.h" >>>>> >>>>> @@ -87,3 +88,21 @@ char *perf_pmu__hybrid_type_to_pmu(const char >>>>> *type) >>>>> free(pmu_name); >>>>> return NULL; >>>>> } >>>>> + >>>>> +int parse_hybrid_type(const struct option *opt, const char *str, >>>>> int unset __maybe_unused) >>>>> +{ >>>>> + struct evlist *evlist = *(struct evlist **)opt->value; >>>>> + >>>>> + if (!list_empty(&evlist->core.entries)) { >>>>> + fprintf(stderr, "Must define cputype before >>>>> events/metrics\n"); >>>>> + return -1; >>>>> + } >>>>> + >>>>> + evlist->hybrid_pmu_name = perf_pmu__hybrid_type_to_pmu(str); >>>>> + if (!evlist->hybrid_pmu_name) { >>>>> + fprintf(stderr, "--cputype %s is not supported!\n", >>>>> str); >>>>> + return -1; >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>>> diff --git a/tools/perf/util/pmu-hybrid.h >>>>> b/tools/perf/util/pmu-hybrid.h >>>>> index 2b186c26a43e..26101f134a3a 100644 >>>>> --- a/tools/perf/util/pmu-hybrid.h >>>>> +++ b/tools/perf/util/pmu-hybrid.h >>>>> @@ -5,6 +5,7 @@ >>>>> #include <linux/perf_event.h> >>>>> #include <linux/compiler.h> >>>>> #include <linux/list.h> >>>>> +#include <subcmd/parse-options.h> >>>>> #include <stdbool.h> >>>>> #include "pmu.h" >>>>> >>>>> @@ -18,6 +19,7 @@ bool perf_pmu__hybrid_mounted(const char *name); >>>>> struct perf_pmu *perf_pmu__find_hybrid_pmu(const char *name); >>>>> bool perf_pmu__is_hybrid(const char *name); >>>>> char *perf_pmu__hybrid_type_to_pmu(const char *type); >>>>> +int parse_hybrid_type(const struct option *opt, const char *str, >>>>> int unset __maybe_unused); >>>>> >>>>> static inline int perf_pmu__hybrid_pmu_num(void) >>>>> { >>>>> -- >>>>> 2.25.1 >>>>> >>> > -- Zhengjun Xing ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-07-04 9:10 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-06-15 15:08 [PATCH] perf record: Support "--cputype" option for hybrid events zhengjun.xing 2022-06-15 15:16 ` Ian Rogers 2022-06-16 8:31 ` Xing Zhengjun 2022-06-16 14:31 ` Liang, Kan 2022-06-21 5:13 ` Xing Zhengjun 2022-07-04 9:10 ` Xing Zhengjun
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).