* [PATCH v2] perf evlist: Remove group option.
@ 2021-09-21 18:13 Ian Rogers
2021-09-22 21:16 ` Jiri Olsa
0 siblings, 1 reply; 3+ messages in thread
From: Ian Rogers @ 2021-09-21 18:13 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
linux-perf-users, linux-kernel
Cc: eranian, Ian Rogers
The group option predates grouping events using curly braces added in
commit 89efb029502d ("perf tools: Add support to parse event group syntax")
The --group option was retained for legacy support (in August 2012) but
keeping it adds complexity.
v2. is a rebase.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/Documentation/perf-record.txt | 4 ----
tools/perf/Documentation/perf-top.txt | 7 ++-----
tools/perf/builtin-record.c | 2 --
tools/perf/builtin-stat.c | 6 ------
tools/perf/builtin-top.c | 2 --
tools/perf/tests/attr/README | 2 --
tools/perf/tests/attr/test-record-group | 22 ----------------------
tools/perf/tests/attr/test-stat-group | 17 -----------------
tools/perf/util/evlist.c | 2 +-
tools/perf/util/evlist.h | 2 --
tools/perf/util/python.c | 8 --------
tools/perf/util/record.c | 7 -------
tools/perf/util/record.h | 1 -
13 files changed, 3 insertions(+), 79 deletions(-)
delete mode 100644 tools/perf/tests/attr/test-record-group
delete mode 100644 tools/perf/tests/attr/test-stat-group
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index f1079ee7f2ec..6e27b7227562 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -236,10 +236,6 @@ OPTIONS
Also, by adding a comma, the number of mmap pages for AUX
area tracing can be specified.
---group::
- Put all events in a single event group. This precedes the --event
- option and remains only for backward compatibility. See --event.
-
-g::
Enables call-graph (stack chain/backtrace) recording for both
kernel space and user space.
diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
index 9898a32b8d9c..67a1c65c8ebb 100644
--- a/tools/perf/Documentation/perf-top.txt
+++ b/tools/perf/Documentation/perf-top.txt
@@ -50,9 +50,6 @@ Default is to monitor all CPUS.
--count-filter=<count>::
Only display functions with more events than this.
---group::
- Put the counters into a counter group.
-
--group-sort-idx::
Sort the output by the event at the index n in group. If n is invalid,
sort by the first event. It can support multiple groups with different
@@ -312,10 +309,10 @@ use '-e e1 -e e2 -G foo,foo' or just use '-e e1 -e e2 -G foo'.
perf top -e cycles,probe:icmp_rcv --switch-on=probe:icmp_rcv
- Alternatively one can ask for --group and then two overhead columns
+ Alternatively one can ask for a group and then two overhead columns
will appear, the first for cycles and the second for the switch-on event.
- perf top --group -e cycles,probe:icmp_rcv --switch-on=probe:icmp_rcv
+ perf top -e '{cycles,probe:icmp_rcv}' --switch-on=probe:icmp_rcv
This may be interesting to measure a workload only after some initialization
phase is over, i.e. insert a perf probe at that point and use the above
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index b3509d9d20cc..6eab7d059599 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2487,8 +2487,6 @@ static struct option __record_options[] = {
OPT_CALLBACK(0, "mmap-flush", &record.opts, "number",
"Minimal number of bytes that is extracted from mmap data pages (default: 1)",
record__mmap_flush_parse),
- OPT_BOOLEAN(0, "group", &record.opts.group,
- "put the counters into a counter group"),
OPT_CALLBACK_NOOPT('g', NULL, &callchain_param,
NULL, "enables call-graph recording" ,
&record_callchain_opt),
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f6e87b7be5fa..f0f51e6966ac 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -180,7 +180,6 @@ static bool topdown_run = false;
static bool smi_cost = false;
static bool smi_reset = false;
static int big_num_opt = -1;
-static bool group = false;
static const char *pre_cmd = NULL;
static const char *post_cmd = NULL;
static bool sync_run = false;
@@ -800,9 +799,6 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
child_pid = evsel_list->workload.pid;
}
- if (group)
- evlist__set_leader(evsel_list);
-
if (affinity__setup(&affinity) < 0)
return -1;
@@ -1192,8 +1188,6 @@ static struct option stat_options[] = {
#endif
OPT_BOOLEAN('a', "all-cpus", &target.system_wide,
"system-wide collection from all CPUs"),
- OPT_BOOLEAN('g', "group", &group,
- "put the counters into a counter group"),
OPT_BOOLEAN(0, "scale", &stat_config.scale,
"Use --no-scale to disable counter scaling for multiplexing"),
OPT_INCR('v', "verbose", &verbose,
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index a3ae9176a83e..bd3704432376 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1472,8 +1472,6 @@ int cmd_top(int argc, const char **argv)
"dump the symbol table used for profiling"),
OPT_INTEGER('f', "count-filter", &top.count_filter,
"only display functions with more events than this"),
- OPT_BOOLEAN(0, "group", &opts->group,
- "put the counters into a counter group"),
OPT_BOOLEAN('i', "no-inherit", &opts->no_inherit,
"child tasks do not inherit counters"),
OPT_STRING(0, "sym-annotate", &top.sym_filter, "symbol name",
diff --git a/tools/perf/tests/attr/README b/tools/perf/tests/attr/README
index a36f49fb4dbe..f538272af57b 100644
--- a/tools/perf/tests/attr/README
+++ b/tools/perf/tests/attr/README
@@ -47,7 +47,6 @@ Following tests are defined (with perf commands):
perf record -g kill (test-record-graph-default)
perf record --call-graph dwarf kill (test-record-graph-dwarf)
perf record --call-graph fp kill (test-record-graph-fp)
- perf record --group -e cycles,instructions kill (test-record-group)
perf record -e '{cycles,instructions}' kill (test-record-group1)
perf record -e '{cycles/period=1/,instructions/period=2/}:S' kill (test-record-group2)
perf record -D kill (test-record-no-delay)
@@ -61,6 +60,5 @@ Following tests are defined (with perf commands):
perf stat -d kill (test-stat-detailed-1)
perf stat -dd kill (test-stat-detailed-2)
perf stat -ddd kill (test-stat-detailed-3)
- perf stat --group -e cycles,instructions kill (test-stat-group)
perf stat -e '{cycles,instructions}' kill (test-stat-group1)
perf stat -i -e cycles kill (test-stat-no-inherit)
diff --git a/tools/perf/tests/attr/test-record-group b/tools/perf/tests/attr/test-record-group
deleted file mode 100644
index 14ee60fd3f41..000000000000
--- a/tools/perf/tests/attr/test-record-group
+++ /dev/null
@@ -1,22 +0,0 @@
-[config]
-command = record
-args = --no-bpf-event --group -e cycles,instructions kill >/dev/null 2>&1
-ret = 1
-
-[event-1:base-record]
-fd=1
-group_fd=-1
-sample_type=327
-read_format=4
-
-[event-2:base-record]
-fd=2
-group_fd=1
-config=1
-sample_type=327
-read_format=4
-mmap=0
-comm=0
-task=0
-enable_on_exec=0
-disabled=0
diff --git a/tools/perf/tests/attr/test-stat-group b/tools/perf/tests/attr/test-stat-group
deleted file mode 100644
index e15d6946e9b3..000000000000
--- a/tools/perf/tests/attr/test-stat-group
+++ /dev/null
@@ -1,17 +0,0 @@
-[config]
-command = stat
-args = --group -e cycles,instructions kill >/dev/null 2>&1
-ret = 1
-
-[event-1:base-stat]
-fd=1
-group_fd=-1
-read_format=3|15
-
-[event-2:base-stat]
-fd=2
-group_fd=1
-config=1
-disabled=0
-enable_on_exec=0
-read_format=3|15
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 5f92319ce258..2e11d82d15e0 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -224,7 +224,7 @@ int __evlist__set_tracepoints_handlers(struct evlist *evlist,
return err;
}
-void evlist__set_leader(struct evlist *evlist)
+static void evlist__set_leader(struct evlist *evlist)
{
perf_evlist__set_leader(&evlist->core);
}
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 97bfb8d0be4f..f92460729d6b 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -201,8 +201,6 @@ void evlist__set_selected(struct evlist *evlist, struct evsel *evsel);
int evlist__create_maps(struct evlist *evlist, struct target *target);
int evlist__apply_filters(struct evlist *evlist, struct evsel **err_evsel);
-void evlist__set_leader(struct evlist *evlist);
-
u64 __evlist__combined_sample_type(struct evlist *evlist);
u64 evlist__combined_sample_type(struct evlist *evlist);
u64 evlist__combined_branch_type(struct evlist *evlist);
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 8feef3a05af7..9cd79513eebb 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -1109,14 +1109,6 @@ static PyObject *pyrf_evlist__open(struct pyrf_evlist *pevlist,
PyObject *args, PyObject *kwargs)
{
struct evlist *evlist = &pevlist->evlist;
- int group = 0;
- static char *kwlist[] = { "group", NULL };
-
- if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|OOii", kwlist, &group))
- return NULL;
-
- if (group)
- evlist__set_leader(evlist);
if (evlist__open(evlist) < 0) {
PyErr_SetFromErrno(PyExc_OSError);
diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c
index bff669b615ee..9e694db7c7ee 100644
--- a/tools/perf/util/record.c
+++ b/tools/perf/util/record.c
@@ -99,13 +99,6 @@ void evlist__config(struct evlist *evlist, struct record_opts *opts, struct call
bool use_comm_exec;
bool sample_id = opts->sample_id;
- /*
- * Set the evsel leader links before we configure attributes,
- * since some might depend on this info.
- */
- if (opts->group)
- evlist__set_leader(evlist);
-
if (evlist->core.cpus->map[0] < 0)
opts->no_inherit = true;
diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
index 68f471d9a88b..d71dee9ce41c 100644
--- a/tools/perf/util/record.h
+++ b/tools/perf/util/record.h
@@ -13,7 +13,6 @@ struct option;
struct record_opts {
struct target target;
- bool group;
bool inherit_stat;
bool no_buffering;
bool no_inherit;
--
2.33.0.464.g1972c5931b-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] perf evlist: Remove group option.
2021-09-21 18:13 [PATCH v2] perf evlist: Remove group option Ian Rogers
@ 2021-09-22 21:16 ` Jiri Olsa
2021-09-22 22:59 ` Ian Rogers
0 siblings, 1 reply; 3+ messages in thread
From: Jiri Olsa @ 2021-09-22 21:16 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, linux-perf-users,
linux-kernel, eranian
On Tue, Sep 21, 2021 at 11:13:49AM -0700, Ian Rogers wrote:
SNIP
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 8feef3a05af7..9cd79513eebb 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -1109,14 +1109,6 @@ static PyObject *pyrf_evlist__open(struct pyrf_evlist *pevlist,
> PyObject *args, PyObject *kwargs)
> {
> struct evlist *evlist = &pevlist->evlist;
> - int group = 0;
> - static char *kwlist[] = { "group", NULL };
> -
> - if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|OOii", kwlist, &group))
> - return NULL;
> -
> - if (group)
> - evlist__set_leader(evlist);
>
> if (evlist__open(evlist) < 0) {
> PyErr_SetFromErrno(PyExc_OSError);
> diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c
> index bff669b615ee..9e694db7c7ee 100644
> --- a/tools/perf/util/record.c
> +++ b/tools/perf/util/record.c
> @@ -99,13 +99,6 @@ void evlist__config(struct evlist *evlist, struct record_opts *opts, struct call
> bool use_comm_exec;
> bool sample_id = opts->sample_id;
>
> - /*
> - * Set the evsel leader links before we configure attributes,
> - * since some might depend on this info.
> - */
> - if (opts->group)
> - evlist__set_leader(evlist);
> -
I don't mind erasing that, but just curious if you're going
to add something which would clash with this? it does not
look too complex for the code.. but still, let's remove it
if it's in 'legacy mode' for this long ;-)
thanks,
jirka
> if (evlist->core.cpus->map[0] < 0)
> opts->no_inherit = true;
>
> diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
> index 68f471d9a88b..d71dee9ce41c 100644
> --- a/tools/perf/util/record.h
> +++ b/tools/perf/util/record.h
> @@ -13,7 +13,6 @@ struct option;
>
> struct record_opts {
> struct target target;
> - bool group;
> bool inherit_stat;
> bool no_buffering;
> bool no_inherit;
> --
> 2.33.0.464.g1972c5931b-goog
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] perf evlist: Remove group option.
2021-09-22 21:16 ` Jiri Olsa
@ 2021-09-22 22:59 ` Ian Rogers
0 siblings, 0 replies; 3+ messages in thread
From: Ian Rogers @ 2021-09-22 22:59 UTC (permalink / raw)
To: Jiri Olsa
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, linux-perf-users,
linux-kernel, eranian
On Wed, Sep 22, 2021 at 2:17 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Tue, Sep 21, 2021 at 11:13:49AM -0700, Ian Rogers wrote:
>
> SNIP
>
> > diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> > index 8feef3a05af7..9cd79513eebb 100644
> > --- a/tools/perf/util/python.c
> > +++ b/tools/perf/util/python.c
> > @@ -1109,14 +1109,6 @@ static PyObject *pyrf_evlist__open(struct pyrf_evlist *pevlist,
> > PyObject *args, PyObject *kwargs)
> > {
> > struct evlist *evlist = &pevlist->evlist;
> > - int group = 0;
> > - static char *kwlist[] = { "group", NULL };
> > -
> > - if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|OOii", kwlist, &group))
> > - return NULL;
> > -
> > - if (group)
> > - evlist__set_leader(evlist);
> >
> > if (evlist__open(evlist) < 0) {
> > PyErr_SetFromErrno(PyExc_OSError);
> > diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c
> > index bff669b615ee..9e694db7c7ee 100644
> > --- a/tools/perf/util/record.c
> > +++ b/tools/perf/util/record.c
> > @@ -99,13 +99,6 @@ void evlist__config(struct evlist *evlist, struct record_opts *opts, struct call
> > bool use_comm_exec;
> > bool sample_id = opts->sample_id;
> >
> > - /*
> > - * Set the evsel leader links before we configure attributes,
> > - * since some might depend on this info.
> > - */
> > - if (opts->group)
> > - evlist__set_leader(evlist);
> > -
>
> I don't mind erasing that, but just curious if you're going
> to add something which would clash with this? it does not
> look too complex for the code.. but still, let's remove it
> if it's in 'legacy mode' for this long ;-)
I've nothing conflicting but I came across old tutorials describing it
and I suspect usage of it is likely broken - in particular mixing
--group with -e {}.
Thanks,
Ian
> thanks,
> jirka
>
>
> > if (evlist->core.cpus->map[0] < 0)
> > opts->no_inherit = true;
> >
> > diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
> > index 68f471d9a88b..d71dee9ce41c 100644
> > --- a/tools/perf/util/record.h
> > +++ b/tools/perf/util/record.h
> > @@ -13,7 +13,6 @@ struct option;
> >
> > struct record_opts {
> > struct target target;
> > - bool group;
> > bool inherit_stat;
> > bool no_buffering;
> > bool no_inherit;
> > --
> > 2.33.0.464.g1972c5931b-goog
> >
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-09-22 22:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-21 18:13 [PATCH v2] perf evlist: Remove group option Ian Rogers
2021-09-22 21:16 ` Jiri Olsa
2021-09-22 22:59 ` Ian Rogers
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.