2022-04-22 19:39:42

by Adrian Hunter

[permalink] [raw]
Subject: [PATCH RFC 00/21] perf intel-pt: Better support for perf record --cpu

Hi

Here are patches to support capturing Intel PT sideband events such as
mmap, task, context switch, text poke etc, on every CPU even when tracing
selected user_requested_cpus. That is, when using the perf record -C or
--cpu option.

This is needed for:
1. text poke: a text poke on any CPU affects all CPUs
2. tracing user space: a user space process can migrate between CPUs so
mmap events that happen on a different CPU can be needed to decode a
user_requested_cpus CPU.

For example:

Trace on CPU 1:

perf record --kcore -C 1 -e intel_pt// &

Start a task on CPU 0:

taskset 0x1 testprog &

Migrate it to CPU 1:

taskset -p 0x2 <testprog pid>

Stop tracing:

kill %1

Prior to these changes there will be errors decoding testprog
in userspace because the comm and mmap events for testprog will not
have been captured.

There is quite a bit of preparation:

The first 5 patches stop auxtrace mixing up mmap idx between evlist and
evsel. That is going to matter when
evlist->all_cpus != evlist->user_requested_cpus != evsel->cpus:

libperf evsel: Factor out perf_evsel__ioctl()
libperf evsel: Add perf_evsel__enable_thread()
perf evlist: Use libperf functions in evlist__enable_event_idx()
perf auxtrace: Move evlist__enable_event_idx() to auxtrace.c
perf auxtrace: Do not mix up mmap idx

The next 6 patches stop attempts to auxtrace mmap when it is not an
auxtrace event e.g. when mmapping the CPUs on which only sideband is
captured:

libperf evlist: Remove ->idx() per_cpu parameter
libperf evlist: Move ->idx() into mmap_per_evsel()
libperf evlist: Add evsel as a parameter to ->idx()
perf auxtrace: Record whether an auxtrace mmap is needed
perf auxctrace: Add mmap_needed to auxtrace_mmap_params
perf auxtrace: Remove auxtrace_mmap_params__set_idx() per_cpu parameter

The next 5 patches switch to setting up dummy event maps before adding the
evsel so that the evsel is subject to map propagation, primarily to cause
addition of the evsel's CPUs to all_cpus.

perf evlist: Factor out evlist__dummy_event()
perf evlist: Add evlist__add_system_wide_dummy()
perf record: Use evlist__add_system_wide_dummy() in record__config_text_poke()
perf intel-pt: Use evlist__add_system_wide_dummy() for switch tracking
perf intel-pt: Track sideband system-wide when needed

The remaining 5 patches make more significant changes.

First change from using user_requested_cpus to using all_cpus where necessary:

perf tools: Allow all_cpus to be a superset of user_requested_cpus

Secondly, mmap all per-thread and all per-cpu events:

libperf evlist: Allow mixing per-thread and per-cpu mmaps

Stop using system_wide flag for uncore because it will not work anymore:

perf stat: Add per_cpu_only flag for uncore

Finally change map propagation so that system-wide events retain their cpus and
(dummy) threads:

perf tools: Allow system-wide events to keep their own CPUs
perf tools: Allow system-wide events to keep their own threads


Adrian Hunter (21):
libperf evsel: Factor out perf_evsel__ioctl()
libperf evsel: Add perf_evsel__enable_thread()
perf evlist: Use libperf functions in evlist__enable_event_idx()
perf auxtrace: Move evlist__enable_event_idx() to auxtrace.c
perf auxtrace: Do not mix up mmap idx
libperf evlist: Remove ->idx() per_cpu parameter
libperf evlist: Move ->idx() into mmap_per_evsel()
libperf evlist: Add evsel as a parameter to ->idx()
perf auxtrace: Record whether an auxtrace mmap is needed
perf auxctrace: Add mmap_needed to auxtrace_mmap_params
perf auxtrace: Remove auxtrace_mmap_params__set_idx() per_cpu parameter
perf evlist: Factor out evlist__dummy_event()
perf evlist: Add evlist__add_system_wide_dummy()
perf record: Use evlist__add_system_wide_dummy() in record__config_text_poke()
perf intel-pt: Use evlist__add_system_wide_dummy() for switch tracking
perf intel-pt: Track sideband system-wide when needed
perf tools: Allow all_cpus to be a superset of user_requested_cpus
libperf evlist: Allow mixing per-thread and per-cpu mmaps
perf stat: Add per_cpu_only flag for uncore
perf tools: Allow system-wide events to keep their own CPUs
perf tools: Allow system-wide events to keep their own threads

tools/lib/perf/evlist.c | 67 +++++++------------
tools/lib/perf/evsel.c | 29 +++++++--
tools/lib/perf/include/internal/evlist.h | 3 +-
tools/lib/perf/include/internal/evsel.h | 1 +
tools/lib/perf/include/perf/evsel.h | 1 +
tools/perf/arch/arm/util/cs-etm.c | 1 +
tools/perf/arch/arm64/util/arm-spe.c | 1 +
tools/perf/arch/s390/util/auxtrace.c | 1 +
tools/perf/arch/x86/util/intel-bts.c | 1 +
tools/perf/arch/x86/util/intel-pt.c | 32 ++++------
tools/perf/builtin-record.c | 39 +++++-------
tools/perf/builtin-stat.c | 5 +-
tools/perf/util/auxtrace.c | 31 +++++++--
tools/perf/util/auxtrace.h | 8 ++-
tools/perf/util/evlist.c | 106 +++++++++++++++----------------
tools/perf/util/evlist.h | 7 +-
tools/perf/util/evsel.c | 1 +
tools/perf/util/evsel.h | 1 +
tools/perf/util/mmap.c | 4 +-
tools/perf/util/parse-events.c | 2 +-
20 files changed, 176 insertions(+), 165 deletions(-)


Regards
Adrian


2022-04-22 20:10:20

by Adrian Hunter

[permalink] [raw]
Subject: [PATCH RFC 08/21] libperf evlist: Add evsel as a parameter to ->idx()

Add evsel as a parameter to ->idx() in preparation for correctly
determining whether an auxtrace mmap is needed.

Signed-off-by: Adrian Hunter <[email protected]>
---
tools/lib/perf/evlist.c | 2 +-
tools/lib/perf/include/internal/evlist.h | 3 ++-
tools/perf/util/evlist.c | 1 +
3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
index 673c267f900e..ad04da81c367 100644
--- a/tools/lib/perf/evlist.c
+++ b/tools/lib/perf/evlist.c
@@ -475,7 +475,7 @@ mmap_per_evsel(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
refcount_set(&map->refcnt, 2);

if (ops->idx)
- ops->idx(evlist, mp, idx);
+ ops->idx(evlist, evsel, mp, idx);

if (ops->mmap(map, mp, *output, evlist_cpu) < 0)
return -1;
diff --git a/tools/lib/perf/include/internal/evlist.h b/tools/lib/perf/include/internal/evlist.h
index 0d5c830431a7..6f89aec3e608 100644
--- a/tools/lib/perf/include/internal/evlist.h
+++ b/tools/lib/perf/include/internal/evlist.h
@@ -38,7 +38,8 @@ struct perf_evlist {
};

typedef void
-(*perf_evlist_mmap__cb_idx_t)(struct perf_evlist*, struct perf_mmap_param*, int);
+(*perf_evlist_mmap__cb_idx_t)(struct perf_evlist*, struct perf_evsel*,
+ struct perf_mmap_param*, int);
typedef struct perf_mmap*
(*perf_evlist_mmap__cb_get_t)(struct perf_evlist*, bool, int);
typedef int
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 09a1d3400fd9..7ae56b062f44 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -747,6 +747,7 @@ static struct mmap *evlist__alloc_mmap(struct evlist *evlist,

static void
perf_evlist__mmap_cb_idx(struct perf_evlist *_evlist,
+ struct perf_evsel *_evsel __maybe_unused,
struct perf_mmap_param *_mp,
int idx)
{
--
2.25.1

2022-04-22 20:21:42

by Adrian Hunter

[permalink] [raw]
Subject: [PATCH RFC 09/21] perf auxtrace: Record whether an auxtrace mmap is needed

Add a flag needs_auxtrace_mmap to record whether an auxtrace mmap is
needed, in preparation for correctly determining whether or not an
auxtrace mmap is needed.

Signed-off-by: Adrian Hunter <[email protected]>
---
tools/perf/arch/arm/util/cs-etm.c | 1 +
tools/perf/arch/arm64/util/arm-spe.c | 1 +
tools/perf/arch/s390/util/auxtrace.c | 1 +
tools/perf/arch/x86/util/intel-bts.c | 1 +
tools/perf/arch/x86/util/intel-pt.c | 1 +
tools/perf/util/evsel.h | 1 +
6 files changed, 6 insertions(+)

diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index 11c71aa219f7..1b54638d53b0 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -319,6 +319,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
}
evsel->core.attr.freq = 0;
evsel->core.attr.sample_period = 1;
+ evsel->needs_auxtrace_mmap = true;
cs_etm_evsel = evsel;
opts->full_auxtrace = true;
}
diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index af4d63af8072..e24c22f187df 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -159,6 +159,7 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
}
evsel->core.attr.freq = 0;
evsel->core.attr.sample_period = arm_spe_pmu->default_config->sample_period;
+ evsel->needs_auxtrace_mmap = true;
arm_spe_evsel = evsel;
opts->full_auxtrace = true;
}
diff --git a/tools/perf/arch/s390/util/auxtrace.c b/tools/perf/arch/s390/util/auxtrace.c
index 0db5c58c98e8..5068baa3e092 100644
--- a/tools/perf/arch/s390/util/auxtrace.c
+++ b/tools/perf/arch/s390/util/auxtrace.c
@@ -98,6 +98,7 @@ struct auxtrace_record *auxtrace_record__init(struct evlist *evlist,
evlist__for_each_entry(evlist, pos) {
if (pos->core.attr.config == PERF_EVENT_CPUM_SF_DIAG) {
diagnose = 1;
+ pos->needs_auxtrace_mmap = true;
break;
}
}
diff --git a/tools/perf/arch/x86/util/intel-bts.c b/tools/perf/arch/x86/util/intel-bts.c
index d68a0f48e41e..bcccfbade5c6 100644
--- a/tools/perf/arch/x86/util/intel-bts.c
+++ b/tools/perf/arch/x86/util/intel-bts.c
@@ -129,6 +129,7 @@ static int intel_bts_recording_options(struct auxtrace_record *itr,
}
evsel->core.attr.freq = 0;
evsel->core.attr.sample_period = 1;
+ evsel->needs_auxtrace_mmap = true;
intel_bts_evsel = evsel;
opts->full_auxtrace = true;
}
diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
index 38ec2666ec12..2eaac4638aab 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -649,6 +649,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr,
evsel->core.attr.freq = 0;
evsel->core.attr.sample_period = 1;
evsel->no_aux_samples = true;
+ evsel->needs_auxtrace_mmap = true;
intel_pt_evsel = evsel;
opts->full_auxtrace = true;
}
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 041b42d33bf5..1a07694afbdd 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -120,6 +120,7 @@ struct evsel {
bool merged_stat;
bool reset_group;
bool errored;
+ bool needs_auxtrace_mmap;
struct hashmap *per_pkg_mask;
int err;
struct {
--
2.25.1

2022-04-22 21:15:57

by Adrian Hunter

[permalink] [raw]
Subject: [PATCH RFC 18/21] libperf evlist: Allow mixing per-thread and per-cpu mmaps

mmap_per_evsel() will skip events that do not match the CPU, so all CPUs
can be iterated in any case.

Signed-off-by: Adrian Hunter <[email protected]>
---
tools/lib/perf/evlist.c | 34 +++++-----------------------------
1 file changed, 5 insertions(+), 29 deletions(-)

diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
index 048b546f9444..37dfa9d936a7 100644
--- a/tools/lib/perf/evlist.c
+++ b/tools/lib/perf/evlist.c
@@ -508,29 +508,6 @@ mmap_per_evsel(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
return 0;
}

-static int
-mmap_per_thread(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
- struct perf_mmap_param *mp)
-{
- int thread;
- int nr_threads = perf_thread_map__nr(evlist->threads);
-
- for (thread = 0; thread < nr_threads; thread++) {
- int output = -1;
- int output_overwrite = -1;
-
- if (mmap_per_evsel(evlist, ops, thread, mp, 0, thread,
- &output, &output_overwrite))
- goto out_unmap;
- }
-
- return 0;
-
-out_unmap:
- perf_evlist__munmap(evlist);
- return -1;
-}
-
static int
mmap_per_cpu(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
struct perf_mmap_param *mp)
@@ -561,9 +538,12 @@ static int perf_evlist__nr_mmaps(struct perf_evlist *evlist)
{
int nr_mmaps;

+ /* One for each CPU */
nr_mmaps = perf_cpu_map__nr(evlist->all_cpus);
- if (perf_cpu_map__empty(evlist->all_cpus))
- nr_mmaps = perf_thread_map__nr(evlist->threads);
+ /* One for each thread */
+ nr_mmaps += perf_thread_map__nr(evlist->threads);
+ /* Minus the dummy CPU or dummy thread */
+ nr_mmaps -= 1;

return nr_mmaps;
}
@@ -573,7 +553,6 @@ int perf_evlist__mmap_ops(struct perf_evlist *evlist,
struct perf_mmap_param *mp)
{
struct perf_evsel *evsel;
- const struct perf_cpu_map *cpus = evlist->all_cpus;

if (!ops || !ops->get || !ops->mmap)
return -EINVAL;
@@ -592,9 +571,6 @@ int perf_evlist__mmap_ops(struct perf_evlist *evlist,
if (evlist->pollfd.entries == NULL && perf_evlist__alloc_pollfd(evlist) < 0)
return -ENOMEM;

- if (perf_cpu_map__empty(cpus))
- return mmap_per_thread(evlist, ops, mp);
-
return mmap_per_cpu(evlist, ops, mp);
}

--
2.25.1

2022-04-22 21:24:05

by Adrian Hunter

[permalink] [raw]
Subject: [PATCH RFC 13/21] perf evlist: Add evlist__add_system_wide_dummy()

Add evlist__add_system_wide_dummy() to enable creating a system-wide dummy
event that sets up the system-wide maps before map propagation.

For convenience, add evlist__add_aux_dummy() so that the logic can be used
whether or not the event needs to be system-wide.

Signed-off-by: Adrian Hunter <[email protected]>
---
tools/perf/util/evlist.c | 40 ++++++++++++++++++++++++++++++++++++++++
tools/perf/util/evlist.h | 5 +++++
2 files changed, 45 insertions(+)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 78c47cbafbc2..58ea562ddbd2 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -264,6 +264,46 @@ int evlist__add_dummy(struct evlist *evlist)
return 0;
}

+static void evlist__add_system_wide(struct evlist *evlist, struct evsel *evsel)
+{
+ evsel->core.system_wide = true;
+
+ /* All CPUs */
+ perf_cpu_map__put(evsel->core.own_cpus);
+ evsel->core.own_cpus = perf_cpu_map__new(NULL);
+ perf_cpu_map__put(evsel->core.cpus);
+ evsel->core.cpus = perf_cpu_map__get(evsel->core.own_cpus);
+
+ /* No threads */
+ perf_thread_map__put(evsel->core.threads);
+ evsel->core.threads = perf_thread_map__new_dummy();
+
+ evlist__add(evlist, evsel);
+}
+
+struct evsel *evlist__add_aux_dummy(struct evlist *evlist, bool system_wide)
+{
+ struct evsel *evsel = evlist__dummy_event(evlist);
+
+ if (!evsel)
+ return NULL;
+
+ evsel->core.attr.exclude_kernel = 1;
+ evsel->core.attr.exclude_guest = 1;
+ evsel->core.attr.exclude_hv = 1;
+ evsel->core.attr.freq = 0;
+ evsel->core.attr.sample_period = 1;
+ evsel->no_aux_samples = true;
+ evsel->name = strdup("dummy:u");
+
+ if (system_wide)
+ evlist__add_system_wide(evlist, evsel);
+ else
+ evlist__add(evlist, evsel);
+
+ return evsel;
+}
+
static int evlist__add_attrs(struct evlist *evlist, struct perf_event_attr *attrs, size_t nr_attrs)
{
struct evsel *evsel, *n;
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 4062f5aebfc1..dd1af114e033 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -114,6 +114,11 @@ int arch_evlist__add_default_attrs(struct evlist *evlist);
struct evsel *arch_evlist__leader(struct list_head *list);

int evlist__add_dummy(struct evlist *evlist);
+struct evsel *evlist__add_aux_dummy(struct evlist *evlist, bool system_wide);
+static inline struct evsel *evlist__add_system_wide_dummy(struct evlist *evlist)
+{
+ return evlist__add_aux_dummy(evlist, true);
+}

int evlist__add_sb_event(struct evlist *evlist, struct perf_event_attr *attr,
evsel__sb_cb_t cb, void *data);
--
2.25.1

2022-04-22 21:38:30

by Adrian Hunter

[permalink] [raw]
Subject: [PATCH RFC 12/21] perf evlist: Factor out evlist__dummy_event()

Factor out evlist__dummy_event() so it can be reused.

Signed-off-by: Adrian Hunter <[email protected]>
---
tools/perf/util/evlist.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 25eae096bdac..78c47cbafbc2 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -242,14 +242,20 @@ int __evlist__add_default(struct evlist *evlist, bool precise)
return 0;
}

-int evlist__add_dummy(struct evlist *evlist)
+static struct evsel *evlist__dummy_event(struct evlist *evlist)
{
struct perf_event_attr attr = {
.type = PERF_TYPE_SOFTWARE,
.config = PERF_COUNT_SW_DUMMY,
.size = sizeof(attr), /* to capture ABI version */
};
- struct evsel *evsel = evsel__new_idx(&attr, evlist->core.nr_entries);
+
+ return evsel__new_idx(&attr, evlist->core.nr_entries);
+}
+
+int evlist__add_dummy(struct evlist *evlist)
+{
+ struct evsel *evsel = evlist__dummy_event(evlist);

if (evsel == NULL)
return -ENOMEM;
--
2.25.1

2022-04-22 21:39:49

by Adrian Hunter

[permalink] [raw]
Subject: [PATCH RFC 02/21] libperf evsel: Add perf_evsel__enable_thread()

Add perf_evsel__enable_thread() as a counterpart to
perf_evsel__enable_cpu(), to enable all events for a thread.

Signed-off-by: Adrian Hunter <[email protected]>
---
tools/lib/perf/evsel.c | 10 ++++++++++
tools/lib/perf/include/perf/evsel.h | 1 +
2 files changed, 11 insertions(+)

diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index 20ae9f5f8b30..2a1f07f877be 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -360,6 +360,16 @@ int perf_evsel__enable_cpu(struct perf_evsel *evsel, int cpu_map_idx)
return perf_evsel__run_ioctl(evsel, PERF_EVENT_IOC_ENABLE, NULL, cpu_map_idx);
}

+int perf_evsel__enable_thread(struct perf_evsel *evsel, int thread)
+{
+ int err = 0;
+ int i;
+
+ for (i = 0; i < xyarray__max_x(evsel->fd) && !err; i++)
+ err = perf_evsel__ioctl(evsel, PERF_EVENT_IOC_ENABLE, NULL, i, thread);
+ return err;
+}
+
int perf_evsel__enable(struct perf_evsel *evsel)
{
int i;
diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
index 2a9516b42d15..699c0ed97d34 100644
--- a/tools/lib/perf/include/perf/evsel.h
+++ b/tools/lib/perf/include/perf/evsel.h
@@ -36,6 +36,7 @@ LIBPERF_API int perf_evsel__read(struct perf_evsel *evsel, int cpu_map_idx, int
struct perf_counts_values *count);
LIBPERF_API int perf_evsel__enable(struct perf_evsel *evsel);
LIBPERF_API int perf_evsel__enable_cpu(struct perf_evsel *evsel, int cpu_map_idx);
+LIBPERF_API int perf_evsel__enable_thread(struct perf_evsel *evsel, int thread);
LIBPERF_API int perf_evsel__disable(struct perf_evsel *evsel);
LIBPERF_API int perf_evsel__disable_cpu(struct perf_evsel *evsel, int cpu_map_idx);
LIBPERF_API struct perf_cpu_map *perf_evsel__cpus(struct perf_evsel *evsel);
--
2.25.1

2022-04-22 21:42:39

by Adrian Hunter

[permalink] [raw]
Subject: [PATCH RFC 20/21] perf tools: Allow system-wide events to keep their own CPUs

Currently, user_requested_cpus supplants system-wide CPUs when the evlist
has_user_cpus. Change that so that system-wide events retain their own
CPUs and they are added to all_cpus.

Signed-off-by: Adrian Hunter <[email protected]>
---
tools/lib/perf/evlist.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
index 9fbcca3fc836..51fd550e326f 100644
--- a/tools/lib/perf/evlist.c
+++ b/tools/lib/perf/evlist.c
@@ -39,12 +39,11 @@ static void __perf_evlist__propagate_maps(struct perf_evlist *evlist,
* We already have cpus for evsel (via PMU sysfs) so
* keep it, if there's no target cpu list defined.
*/
- if (!evsel->own_cpus || evlist->has_user_cpus) {
- perf_cpu_map__put(evsel->cpus);
- evsel->cpus = perf_cpu_map__get(evlist->user_requested_cpus);
- } else if (!evsel->system_wide &&
- !evsel->requires_cpu &&
- perf_cpu_map__empty(evlist->user_requested_cpus)) {
+ if (!evsel->own_cpus ||
+ (!evsel->system_wide && evlist->has_user_cpus) ||
+ (!evsel->system_wide &&
+ !evsel->requires_cpu &&
+ perf_cpu_map__empty(evlist->user_requested_cpus))) {
perf_cpu_map__put(evsel->cpus);
evsel->cpus = perf_cpu_map__get(evlist->user_requested_cpus);
} else if (evsel->cpus != evsel->own_cpus) {
--
2.25.1

2022-04-22 21:43:53

by Adrian Hunter

[permalink] [raw]
Subject: [PATCH RFC 07/21] libperf evlist: Move ->idx() into mmap_per_evsel()

Move ->idx() into mmap_per_evsel() in preparation for adding evsel as a
parameter.

Signed-off-by: Adrian Hunter <[email protected]>
---
tools/lib/perf/evlist.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
index 6d0fa7b2f417..673c267f900e 100644
--- a/tools/lib/perf/evlist.c
+++ b/tools/lib/perf/evlist.c
@@ -474,6 +474,9 @@ mmap_per_evsel(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
*/
refcount_set(&map->refcnt, 2);

+ if (ops->idx)
+ ops->idx(evlist, mp, idx);
+
if (ops->mmap(map, mp, *output, evlist_cpu) < 0)
return -1;

@@ -516,9 +519,6 @@ mmap_per_thread(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
int output = -1;
int output_overwrite = -1;

- if (ops->idx)
- ops->idx(evlist, mp, thread);
-
if (mmap_per_evsel(evlist, ops, thread, mp, 0, thread,
&output, &output_overwrite))
goto out_unmap;
@@ -543,9 +543,6 @@ mmap_per_cpu(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
int output = -1;
int output_overwrite = -1;

- if (ops->idx)
- ops->idx(evlist, mp, cpu);
-
for (thread = 0; thread < nr_threads; thread++) {
if (mmap_per_evsel(evlist, ops, cpu, mp, cpu,
thread, &output, &output_overwrite))
--
2.25.1

2022-04-22 21:45:08

by Adrian Hunter

[permalink] [raw]
Subject: [PATCH RFC 10/21] perf auxctrace: Add mmap_needed to auxtrace_mmap_params

Add mmap_needed to auxtrace_mmap_params.

Currently an auxtrace mmap is always attempted even if the event is not an
auxtrace event. That works because, when AUX area tracing, there is always
an auxtrace event first for every mmap. Prepare for that not being the
case, which it won't be when sideband tracking events are allowed on
all CPUs even when auxtrace is limited to selected CPUs.

Signed-off-by: Adrian Hunter <[email protected]>
---
tools/perf/util/auxtrace.c | 10 ++++++++--
tools/perf/util/auxtrace.h | 7 +++++--
tools/perf/util/evlist.c | 5 +++--
tools/perf/util/mmap.c | 1 +
4 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index 2d015b0be549..9f01ce405971 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -125,7 +125,7 @@ int auxtrace_mmap__mmap(struct auxtrace_mmap *mm,
mm->tid = mp->tid;
mm->cpu = mp->cpu.cpu;

- if (!mp->len) {
+ if (!mp->len || !mp->mmap_needed) {
mm->base = NULL;
return 0;
}
@@ -168,9 +168,15 @@ void auxtrace_mmap_params__init(struct auxtrace_mmap_params *mp,
}

void auxtrace_mmap_params__set_idx(struct auxtrace_mmap_params *mp,
- struct evlist *evlist, int idx,
+ struct evlist *evlist,
+ struct evsel *evsel, int idx,
bool per_cpu)
{
+ mp->mmap_needed = evsel->needs_auxtrace_mmap;
+
+ if (!mp->mmap_needed)
+ return;
+
mp->idx = idx;

if (per_cpu) {
diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
index dc38b6f57232..4e715e2d9291 100644
--- a/tools/perf/util/auxtrace.h
+++ b/tools/perf/util/auxtrace.h
@@ -353,6 +353,7 @@ struct auxtrace_mmap_params {
int prot;
int idx;
pid_t tid;
+ bool mmap_needed;
struct perf_cpu cpu;
};

@@ -490,7 +491,8 @@ void auxtrace_mmap_params__init(struct auxtrace_mmap_params *mp,
unsigned int auxtrace_pages,
bool auxtrace_overwrite);
void auxtrace_mmap_params__set_idx(struct auxtrace_mmap_params *mp,
- struct evlist *evlist, int idx,
+ struct evlist *evlist,
+ struct evsel *evsel, int idx,
bool per_cpu);

typedef int (*process_auxtrace_t)(struct perf_tool *tool,
@@ -863,7 +865,8 @@ void auxtrace_mmap_params__init(struct auxtrace_mmap_params *mp,
unsigned int auxtrace_pages,
bool auxtrace_overwrite);
void auxtrace_mmap_params__set_idx(struct auxtrace_mmap_params *mp,
- struct evlist *evlist, int idx,
+ struct evlist *evlist,
+ struct evsel *evsel, int idx,
bool per_cpu);

#define ITRACE_HELP ""
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 7ae56b062f44..996bdc203616 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -747,15 +747,16 @@ static struct mmap *evlist__alloc_mmap(struct evlist *evlist,

static void
perf_evlist__mmap_cb_idx(struct perf_evlist *_evlist,
- struct perf_evsel *_evsel __maybe_unused,
+ struct perf_evsel *_evsel,
struct perf_mmap_param *_mp,
int idx)
{
struct evlist *evlist = container_of(_evlist, struct evlist, core);
struct mmap_params *mp = container_of(_mp, struct mmap_params, core);
bool per_cpu = !perf_cpu_map__empty(_evlist->user_requested_cpus);
+ struct evsel *evsel = container_of(_evsel, struct evsel, core);

- auxtrace_mmap_params__set_idx(&mp->auxtrace_mp, evlist, idx, per_cpu);
+ auxtrace_mmap_params__set_idx(&mp->auxtrace_mp, evlist, evsel, idx, per_cpu);
}

static struct perf_mmap*
diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 50502b4a7ca4..de59c4da852b 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -62,6 +62,7 @@ void __weak auxtrace_mmap_params__init(struct auxtrace_mmap_params *mp __maybe_u

void __weak auxtrace_mmap_params__set_idx(struct auxtrace_mmap_params *mp __maybe_unused,
struct evlist *evlist __maybe_unused,
+ struct evsel *evsel __maybe_unused,
int idx __maybe_unused,
bool per_cpu __maybe_unused)
{
--
2.25.1

2022-04-22 21:51:17

by Adrian Hunter

[permalink] [raw]
Subject: [PATCH RFC 06/21] libperf evlist: Remove ->idx() per_cpu parameter

Remove ->idx() per_cpu parameter because it isn't needed.

Signed-off-by: Adrian Hunter <[email protected]>
---
tools/lib/perf/evlist.c | 4 ++--
tools/lib/perf/include/internal/evlist.h | 2 +-
tools/perf/util/evlist.c | 3 ++-
3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
index a09315538a30..6d0fa7b2f417 100644
--- a/tools/lib/perf/evlist.c
+++ b/tools/lib/perf/evlist.c
@@ -517,7 +517,7 @@ mmap_per_thread(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
int output_overwrite = -1;

if (ops->idx)
- ops->idx(evlist, mp, thread, false);
+ ops->idx(evlist, mp, thread);

if (mmap_per_evsel(evlist, ops, thread, mp, 0, thread,
&output, &output_overwrite))
@@ -544,7 +544,7 @@ mmap_per_cpu(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
int output_overwrite = -1;

if (ops->idx)
- ops->idx(evlist, mp, cpu, true);
+ ops->idx(evlist, mp, cpu);

for (thread = 0; thread < nr_threads; thread++) {
if (mmap_per_evsel(evlist, ops, cpu, mp, cpu,
diff --git a/tools/lib/perf/include/internal/evlist.h b/tools/lib/perf/include/internal/evlist.h
index e3e64f37db7b..0d5c830431a7 100644
--- a/tools/lib/perf/include/internal/evlist.h
+++ b/tools/lib/perf/include/internal/evlist.h
@@ -38,7 +38,7 @@ struct perf_evlist {
};

typedef void
-(*perf_evlist_mmap__cb_idx_t)(struct perf_evlist*, struct perf_mmap_param*, int, bool);
+(*perf_evlist_mmap__cb_idx_t)(struct perf_evlist*, struct perf_mmap_param*, int);
typedef struct perf_mmap*
(*perf_evlist_mmap__cb_get_t)(struct perf_evlist*, bool, int);
typedef int
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index f1309b39afe4..09a1d3400fd9 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -748,10 +748,11 @@ static struct mmap *evlist__alloc_mmap(struct evlist *evlist,
static void
perf_evlist__mmap_cb_idx(struct perf_evlist *_evlist,
struct perf_mmap_param *_mp,
- int idx, bool per_cpu)
+ int idx)
{
struct evlist *evlist = container_of(_evlist, struct evlist, core);
struct mmap_params *mp = container_of(_mp, struct mmap_params, core);
+ bool per_cpu = !perf_cpu_map__empty(_evlist->user_requested_cpus);

auxtrace_mmap_params__set_idx(&mp->auxtrace_mp, evlist, idx, per_cpu);
}
--
2.25.1

2022-04-22 22:11:36

by Adrian Hunter

[permalink] [raw]
Subject: [PATCH RFC 01/21] libperf evsel: Factor out perf_evsel__ioctl()

Factor out perf_evsel__ioctl() so it can be reused.

Signed-off-by: Adrian Hunter <[email protected]>
---
tools/lib/perf/evsel.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index 210ea7c06ce8..20ae9f5f8b30 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -328,6 +328,17 @@ int perf_evsel__read(struct perf_evsel *evsel, int cpu_map_idx, int thread,
return 0;
}

+static int perf_evsel__ioctl(struct perf_evsel *evsel, int ioc, void *arg,
+ int cpu_map_idx, int thread)
+{
+ int *fd = FD(evsel, cpu_map_idx, thread);
+
+ if (fd == NULL || *fd < 0)
+ return -1;
+
+ return ioctl(*fd, ioc, arg);
+}
+
static int perf_evsel__run_ioctl(struct perf_evsel *evsel,
int ioc, void *arg,
int cpu_map_idx)
@@ -335,13 +346,7 @@ static int perf_evsel__run_ioctl(struct perf_evsel *evsel,
int thread;

for (thread = 0; thread < xyarray__max_y(evsel->fd); thread++) {
- int err;
- int *fd = FD(evsel, cpu_map_idx, thread);
-
- if (fd == NULL || *fd < 0)
- return -1;
-
- err = ioctl(*fd, ioc, arg);
+ int err = perf_evsel__ioctl(evsel, ioc, arg, cpu_map_idx, thread);

if (err)
return err;
--
2.25.1

2022-04-22 22:49:11

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH RFC 01/21] libperf evsel: Factor out perf_evsel__ioctl()

Em Fri, Apr 22, 2022 at 07:23:42PM +0300, Adrian Hunter escreveu:
> Factor out perf_evsel__ioctl() so it can be reused.

Cherry picking this one as I look at the patchset.

- Arnaldo

> Signed-off-by: Adrian Hunter <[email protected]>
> ---
> tools/lib/perf/evsel.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> index 210ea7c06ce8..20ae9f5f8b30 100644
> --- a/tools/lib/perf/evsel.c
> +++ b/tools/lib/perf/evsel.c
> @@ -328,6 +328,17 @@ int perf_evsel__read(struct perf_evsel *evsel, int cpu_map_idx, int thread,
> return 0;
> }
>
> +static int perf_evsel__ioctl(struct perf_evsel *evsel, int ioc, void *arg,
> + int cpu_map_idx, int thread)
> +{
> + int *fd = FD(evsel, cpu_map_idx, thread);
> +
> + if (fd == NULL || *fd < 0)
> + return -1;
> +
> + return ioctl(*fd, ioc, arg);
> +}
> +
> static int perf_evsel__run_ioctl(struct perf_evsel *evsel,
> int ioc, void *arg,
> int cpu_map_idx)
> @@ -335,13 +346,7 @@ static int perf_evsel__run_ioctl(struct perf_evsel *evsel,
> int thread;
>
> for (thread = 0; thread < xyarray__max_y(evsel->fd); thread++) {
> - int err;
> - int *fd = FD(evsel, cpu_map_idx, thread);
> -
> - if (fd == NULL || *fd < 0)
> - return -1;
> -
> - err = ioctl(*fd, ioc, arg);
> + int err = perf_evsel__ioctl(evsel, ioc, arg, cpu_map_idx, thread);
>
> if (err)
> return err;
> --
> 2.25.1

--

- Arnaldo

2022-04-27 23:37:05

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH RFC 02/21] libperf evsel: Add perf_evsel__enable_thread()

Hi Adrian,

On Fri, Apr 22, 2022 at 9:24 AM Adrian Hunter <[email protected]> wrote:
>
> Add perf_evsel__enable_thread() as a counterpart to
> perf_evsel__enable_cpu(), to enable all events for a thread.
>
> Signed-off-by: Adrian Hunter <[email protected]>
> ---
> tools/lib/perf/evsel.c | 10 ++++++++++
> tools/lib/perf/include/perf/evsel.h | 1 +
> 2 files changed, 11 insertions(+)
>
> diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> index 20ae9f5f8b30..2a1f07f877be 100644
> --- a/tools/lib/perf/evsel.c
> +++ b/tools/lib/perf/evsel.c
> @@ -360,6 +360,16 @@ int perf_evsel__enable_cpu(struct perf_evsel *evsel, int cpu_map_idx)
> return perf_evsel__run_ioctl(evsel, PERF_EVENT_IOC_ENABLE, NULL, cpu_map_idx);
> }
>
> +int perf_evsel__enable_thread(struct perf_evsel *evsel, int thread)
> +{
> + int err = 0;
> + int i;
> +
> + for (i = 0; i < xyarray__max_x(evsel->fd) && !err; i++)
> + err = perf_evsel__ioctl(evsel, PERF_EVENT_IOC_ENABLE, NULL, i, thread);

You might want to break the loop when it fails.

Thanks,
Namhyung


> + return err;
> +}
> +
> int perf_evsel__enable(struct perf_evsel *evsel)
> {
> int i;
> diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
> index 2a9516b42d15..699c0ed97d34 100644
> --- a/tools/lib/perf/include/perf/evsel.h
> +++ b/tools/lib/perf/include/perf/evsel.h
> @@ -36,6 +36,7 @@ LIBPERF_API int perf_evsel__read(struct perf_evsel *evsel, int cpu_map_idx, int
> struct perf_counts_values *count);
> LIBPERF_API int perf_evsel__enable(struct perf_evsel *evsel);
> LIBPERF_API int perf_evsel__enable_cpu(struct perf_evsel *evsel, int cpu_map_idx);
> +LIBPERF_API int perf_evsel__enable_thread(struct perf_evsel *evsel, int thread);
> LIBPERF_API int perf_evsel__disable(struct perf_evsel *evsel);
> LIBPERF_API int perf_evsel__disable_cpu(struct perf_evsel *evsel, int cpu_map_idx);
> LIBPERF_API struct perf_cpu_map *perf_evsel__cpus(struct perf_evsel *evsel);
> --
> 2.25.1
>

2022-04-28 08:08:18

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH RFC 02/21] libperf evsel: Add perf_evsel__enable_thread()

On 28/04/22 00:48, Namhyung Kim wrote:
> Hi Adrian,
>
> On Fri, Apr 22, 2022 at 9:24 AM Adrian Hunter <[email protected]> wrote:
>>
>> Add perf_evsel__enable_thread() as a counterpart to
>> perf_evsel__enable_cpu(), to enable all events for a thread.
>>
>> Signed-off-by: Adrian Hunter <[email protected]>
>> ---
>> tools/lib/perf/evsel.c | 10 ++++++++++
>> tools/lib/perf/include/perf/evsel.h | 1 +
>> 2 files changed, 11 insertions(+)
>>
>> diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
>> index 20ae9f5f8b30..2a1f07f877be 100644
>> --- a/tools/lib/perf/evsel.c
>> +++ b/tools/lib/perf/evsel.c
>> @@ -360,6 +360,16 @@ int perf_evsel__enable_cpu(struct perf_evsel *evsel, int cpu_map_idx)
>> return perf_evsel__run_ioctl(evsel, PERF_EVENT_IOC_ENABLE, NULL, cpu_map_idx);
>> }
>>
>> +int perf_evsel__enable_thread(struct perf_evsel *evsel, int thread)
>> +{
>> + int err = 0;
>> + int i;
>> +
>> + for (i = 0; i < xyarray__max_x(evsel->fd) && !err; i++)
>> + err = perf_evsel__ioctl(evsel, PERF_EVENT_IOC_ENABLE, NULL, i, thread);
>
> You might want to break the loop when it fails.

Thanks for looking at this. It should break because of " && !err".

>
> Thanks,
> Namhyung
>
>
>> + return err;
>> +}
>> +
>> int perf_evsel__enable(struct perf_evsel *evsel)
>> {
>> int i;
>> diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
>> index 2a9516b42d15..699c0ed97d34 100644
>> --- a/tools/lib/perf/include/perf/evsel.h
>> +++ b/tools/lib/perf/include/perf/evsel.h
>> @@ -36,6 +36,7 @@ LIBPERF_API int perf_evsel__read(struct perf_evsel *evsel, int cpu_map_idx, int
>> struct perf_counts_values *count);
>> LIBPERF_API int perf_evsel__enable(struct perf_evsel *evsel);
>> LIBPERF_API int perf_evsel__enable_cpu(struct perf_evsel *evsel, int cpu_map_idx);
>> +LIBPERF_API int perf_evsel__enable_thread(struct perf_evsel *evsel, int thread);
>> LIBPERF_API int perf_evsel__disable(struct perf_evsel *evsel);
>> LIBPERF_API int perf_evsel__disable_cpu(struct perf_evsel *evsel, int cpu_map_idx);
>> LIBPERF_API struct perf_cpu_map *perf_evsel__cpus(struct perf_evsel *evsel);
>> --
>> 2.25.1
>>

2022-04-29 16:06:48

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH RFC 02/21] libperf evsel: Add perf_evsel__enable_thread()

On Wed, Apr 27, 2022 at 9:15 PM Adrian Hunter <[email protected]> wrote:
>
> On 28/04/22 00:48, Namhyung Kim wrote:
> > Hi Adrian,
> >
> > On Fri, Apr 22, 2022 at 9:24 AM Adrian Hunter <[email protected]> wrote:
> >>
> >> Add perf_evsel__enable_thread() as a counterpart to
> >> perf_evsel__enable_cpu(), to enable all events for a thread.
> >>
> >> Signed-off-by: Adrian Hunter <[email protected]>
> >> ---
> >> tools/lib/perf/evsel.c | 10 ++++++++++
> >> tools/lib/perf/include/perf/evsel.h | 1 +
> >> 2 files changed, 11 insertions(+)
> >>
> >> diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> >> index 20ae9f5f8b30..2a1f07f877be 100644
> >> --- a/tools/lib/perf/evsel.c
> >> +++ b/tools/lib/perf/evsel.c
> >> @@ -360,6 +360,16 @@ int perf_evsel__enable_cpu(struct perf_evsel *evsel, int cpu_map_idx)
> >> return perf_evsel__run_ioctl(evsel, PERF_EVENT_IOC_ENABLE, NULL, cpu_map_idx);
> >> }
> >>
> >> +int perf_evsel__enable_thread(struct perf_evsel *evsel, int thread)
> >> +{
> >> + int err = 0;
> >> + int i;
> >> +
> >> + for (i = 0; i < xyarray__max_x(evsel->fd) && !err; i++)
> >> + err = perf_evsel__ioctl(evsel, PERF_EVENT_IOC_ENABLE, NULL, i, thread);
> >
> > You might want to break the loop when it fails.
>
> Thanks for looking at this. It should break because of " && !err".

Oh, I missed that part, sorry!

Thanks,
Namhyung

2022-05-04 03:26:36

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH RFC 18/21] libperf evlist: Allow mixing per-thread and per-cpu mmaps

On Fri, Apr 22, 2022 at 9:25 AM Adrian Hunter <[email protected]> wrote:
>
> mmap_per_evsel() will skip events that do not match the CPU, so all CPUs
> can be iterated in any case.
>
> Signed-off-by: Adrian Hunter <[email protected]>
> ---
[...]
> @@ -561,9 +538,12 @@ static int perf_evlist__nr_mmaps(struct perf_evlist *evlist)
> {
> int nr_mmaps;
>
> + /* One for each CPU */
> nr_mmaps = perf_cpu_map__nr(evlist->all_cpus);
> - if (perf_cpu_map__empty(evlist->all_cpus))
> - nr_mmaps = perf_thread_map__nr(evlist->threads);
> + /* One for each thread */
> + nr_mmaps += perf_thread_map__nr(evlist->threads);
> + /* Minus the dummy CPU or dummy thread */
> + nr_mmaps -= 1;

I'm not sure it'd work for per-task events with default-per-cpu mode.

Thanks,
Namhyung

>
> return nr_mmaps;
> }

2022-05-04 08:53:14

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH RFC 02/21] libperf evsel: Add perf_evsel__enable_thread()

On Fri, Apr 22, 2022 at 9:24 AM Adrian Hunter <[email protected]> wrote:
>
> Add perf_evsel__enable_thread() as a counterpart to
> perf_evsel__enable_cpu(), to enable all events for a thread.
>
> Signed-off-by: Adrian Hunter <[email protected]>
> ---
> tools/lib/perf/evsel.c | 10 ++++++++++
> tools/lib/perf/include/perf/evsel.h | 1 +
> 2 files changed, 11 insertions(+)
>
> diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> index 20ae9f5f8b30..2a1f07f877be 100644
> --- a/tools/lib/perf/evsel.c
> +++ b/tools/lib/perf/evsel.c
> @@ -360,6 +360,16 @@ int perf_evsel__enable_cpu(struct perf_evsel *evsel, int cpu_map_idx)
> return perf_evsel__run_ioctl(evsel, PERF_EVENT_IOC_ENABLE, NULL, cpu_map_idx);
> }
>
> +int perf_evsel__enable_thread(struct perf_evsel *evsel, int thread)
> +{
> + int err = 0;
> + int i;
> +
> + for (i = 0; i < xyarray__max_x(evsel->fd) && !err; i++)
> + err = perf_evsel__ioctl(evsel, PERF_EVENT_IOC_ENABLE, NULL, i, thread);

Looking at the argument names to perf_evsel__ioctl, i is the
cpu_map_idx. Would it be more intention revealing here to do:

perf_cpu_map__for_each_cpu(cpu, idx, evsel->cpus) {
if (err = perf_evsel__ioctl(evsel, PERF_EVENT_IOC_ENABLE, NULL, idx, thread))
break;
}

or perhaps:

for (idx = 0; idx < perf_cpu_map__nr(evsel->fd) && !err; idx++)

Thanks,
Ian

> + return err;
> +}
> +
> int perf_evsel__enable(struct perf_evsel *evsel)
> {
> int i;
> diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
> index 2a9516b42d15..699c0ed97d34 100644
> --- a/tools/lib/perf/include/perf/evsel.h
> +++ b/tools/lib/perf/include/perf/evsel.h
> @@ -36,6 +36,7 @@ LIBPERF_API int perf_evsel__read(struct perf_evsel *evsel, int cpu_map_idx, int
> struct perf_counts_values *count);
> LIBPERF_API int perf_evsel__enable(struct perf_evsel *evsel);
> LIBPERF_API int perf_evsel__enable_cpu(struct perf_evsel *evsel, int cpu_map_idx);
> +LIBPERF_API int perf_evsel__enable_thread(struct perf_evsel *evsel, int thread);
> LIBPERF_API int perf_evsel__disable(struct perf_evsel *evsel);
> LIBPERF_API int perf_evsel__disable_cpu(struct perf_evsel *evsel, int cpu_map_idx);
> LIBPERF_API struct perf_cpu_map *perf_evsel__cpus(struct perf_evsel *evsel);
> --
> 2.25.1
>

2022-05-04 16:57:36

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH RFC 00/21] perf intel-pt: Better support for perf record --cpu

On Fri, Apr 22, 2022 at 9:24 AM Adrian Hunter <[email protected]> wrote:
>
> Hi
>
> Here are patches to support capturing Intel PT sideband events such as
> mmap, task, context switch, text poke etc, on every CPU even when tracing
> selected user_requested_cpus. That is, when using the perf record -C or
> --cpu option.
>
> This is needed for:
> 1. text poke: a text poke on any CPU affects all CPUs
> 2. tracing user space: a user space process can migrate between CPUs so
> mmap events that happen on a different CPU can be needed to decode a
> user_requested_cpus CPU.
>
> For example:
>
> Trace on CPU 1:
>
> perf record --kcore -C 1 -e intel_pt// &
>
> Start a task on CPU 0:
>
> taskset 0x1 testprog &
>
> Migrate it to CPU 1:
>
> taskset -p 0x2 <testprog pid>
>
> Stop tracing:
>
> kill %1
>
> Prior to these changes there will be errors decoding testprog
> in userspace because the comm and mmap events for testprog will not
> have been captured.
>
> There is quite a bit of preparation:
>
> The first 5 patches stop auxtrace mixing up mmap idx between evlist and
> evsel. That is going to matter when
> evlist->all_cpus != evlist->user_requested_cpus != evsel->cpus:
>
> libperf evsel: Factor out perf_evsel__ioctl()
> libperf evsel: Add perf_evsel__enable_thread()
> perf evlist: Use libperf functions in evlist__enable_event_idx()
> perf auxtrace: Move evlist__enable_event_idx() to auxtrace.c
> perf auxtrace: Do not mix up mmap idx
>
> The next 6 patches stop attempts to auxtrace mmap when it is not an
> auxtrace event e.g. when mmapping the CPUs on which only sideband is
> captured:
>
> libperf evlist: Remove ->idx() per_cpu parameter
> libperf evlist: Move ->idx() into mmap_per_evsel()
> libperf evlist: Add evsel as a parameter to ->idx()
> perf auxtrace: Record whether an auxtrace mmap is needed
> perf auxctrace: Add mmap_needed to auxtrace_mmap_params
> perf auxtrace: Remove auxtrace_mmap_params__set_idx() per_cpu parameter
>
> The next 5 patches switch to setting up dummy event maps before adding the
> evsel so that the evsel is subject to map propagation, primarily to cause
> addition of the evsel's CPUs to all_cpus.
>
> perf evlist: Factor out evlist__dummy_event()
> perf evlist: Add evlist__add_system_wide_dummy()
> perf record: Use evlist__add_system_wide_dummy() in record__config_text_poke()
> perf intel-pt: Use evlist__add_system_wide_dummy() for switch tracking
> perf intel-pt: Track sideband system-wide when needed
>
> The remaining 5 patches make more significant changes.
>
> First change from using user_requested_cpus to using all_cpus where necessary:
>
> perf tools: Allow all_cpus to be a superset of user_requested_cpus
>
> Secondly, mmap all per-thread and all per-cpu events:
>
> libperf evlist: Allow mixing per-thread and per-cpu mmaps
>
> Stop using system_wide flag for uncore because it will not work anymore:
>
> perf stat: Add per_cpu_only flag for uncore
>
> Finally change map propagation so that system-wide events retain their cpus and
> (dummy) threads:
>
> perf tools: Allow system-wide events to keep their own CPUs
> perf tools: Allow system-wide events to keep their own threads
>
>
> Adrian Hunter (21):
> libperf evsel: Factor out perf_evsel__ioctl()
> libperf evsel: Add perf_evsel__enable_thread()
> perf evlist: Use libperf functions in evlist__enable_event_idx()
> perf auxtrace: Move evlist__enable_event_idx() to auxtrace.c
> perf auxtrace: Do not mix up mmap idx
> libperf evlist: Remove ->idx() per_cpu parameter
> libperf evlist: Move ->idx() into mmap_per_evsel()
> libperf evlist: Add evsel as a parameter to ->idx()
> perf auxtrace: Record whether an auxtrace mmap is needed
> perf auxctrace: Add mmap_needed to auxtrace_mmap_params
> perf auxtrace: Remove auxtrace_mmap_params__set_idx() per_cpu parameter
> perf evlist: Factor out evlist__dummy_event()
> perf evlist: Add evlist__add_system_wide_dummy()
> perf record: Use evlist__add_system_wide_dummy() in record__config_text_poke()
> perf intel-pt: Use evlist__add_system_wide_dummy() for switch tracking
> perf intel-pt: Track sideband system-wide when needed
> perf tools: Allow all_cpus to be a superset of user_requested_cpus
> libperf evlist: Allow mixing per-thread and per-cpu mmaps
> perf stat: Add per_cpu_only flag for uncore
> perf tools: Allow system-wide events to keep their own CPUs
> perf tools: Allow system-wide events to keep their own threads
>
> tools/lib/perf/evlist.c | 67 +++++++------------
> tools/lib/perf/evsel.c | 29 +++++++--
> tools/lib/perf/include/internal/evlist.h | 3 +-
> tools/lib/perf/include/internal/evsel.h | 1 +
> tools/lib/perf/include/perf/evsel.h | 1 +
> tools/perf/arch/arm/util/cs-etm.c | 1 +
> tools/perf/arch/arm64/util/arm-spe.c | 1 +
> tools/perf/arch/s390/util/auxtrace.c | 1 +
> tools/perf/arch/x86/util/intel-bts.c | 1 +
> tools/perf/arch/x86/util/intel-pt.c | 32 ++++------
> tools/perf/builtin-record.c | 39 +++++-------
> tools/perf/builtin-stat.c | 5 +-
> tools/perf/util/auxtrace.c | 31 +++++++--
> tools/perf/util/auxtrace.h | 8 ++-
> tools/perf/util/evlist.c | 106 +++++++++++++++----------------
> tools/perf/util/evlist.h | 7 +-
> tools/perf/util/evsel.c | 1 +
> tools/perf/util/evsel.h | 1 +
> tools/perf/util/mmap.c | 4 +-
> tools/perf/util/parse-events.c | 2 +-
> 20 files changed, 176 insertions(+), 165 deletions(-)
>
>
> Regards
> Adrian

Thanks Adrian, I'm very much in favor of this patch set. Can we add
some tests for intel-pt? They could be part of the 'perf record' shell
test:
https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/tests/shell/record.sh?h=perf/core
but probably better as there own thing. A command line broken prior to
this change would be great!

Aside from testing, we should clean up what dummy CPUs in the cpu maps
means. As per:
https://lore.kernel.org/linux-perf-users/CAP-5=fWfs2td9nZLGdEBD+C5s=upa_7SORab8tQ7qH=jX--F7w@mail.gmail.com/

I also think landing:
https://lore.kernel.org/linux-perf-users/[email protected]/
will help as it avoids the all_cpus map containing references to CPUs
from PMU sysfs that had been overridden.

Thanks,
Ian

2022-05-05 13:16:02

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH RFC 18/21] libperf evlist: Allow mixing per-thread and per-cpu mmaps

On 3/05/22 23:29, Namhyung Kim wrote:
> On Fri, Apr 22, 2022 at 9:25 AM Adrian Hunter <[email protected]> wrote:
>>
>> mmap_per_evsel() will skip events that do not match the CPU, so all CPUs
>> can be iterated in any case.
>>
>> Signed-off-by: Adrian Hunter <[email protected]>
>> ---
> [...]
>> @@ -561,9 +538,12 @@ static int perf_evlist__nr_mmaps(struct perf_evlist *evlist)
>> {
>> int nr_mmaps;
>>
>> + /* One for each CPU */
>> nr_mmaps = perf_cpu_map__nr(evlist->all_cpus);
>> - if (perf_cpu_map__empty(evlist->all_cpus))
>> - nr_mmaps = perf_thread_map__nr(evlist->threads);
>> + /* One for each thread */
>> + nr_mmaps += perf_thread_map__nr(evlist->threads);
>> + /* Minus the dummy CPU or dummy thread */
>> + nr_mmaps -= 1;
>
> I'm not sure it'd work for per-task events with default-per-cpu mode.

Thanks for noticing that. It ends up being too high which doesn't fail
immediately. I need to add a check that nr_mmaps matches the number
of mmaps actually made.

>
> Thanks,
> Namhyung
>
>>
>> return nr_mmaps;
>> }