Messages in this thread | | | Subject | Re: [PATCH] perf evsel: Don't set sample_regs_intr/sample_regs_user for dummy event | From | "Jin, Yao" <> | Date | Sat, 4 Jul 2020 08:31:50 +0800 |
| |
Hi Jiri,
On 7/3/2020 7:00 PM, Jiri Olsa wrote: > On Fri, Jul 03, 2020 at 08:42:15AM +0800, Jin Yao wrote: >> Since commit 0a892c1c9472 ("perf record: Add dummy event during system wide synthesis"), >> a dummy event is added to capture mmaps. >> >> But if we run perf-record as, >> >> # perf record -e cycles:p -IXMM0 -a -- sleep 1 >> Error: >> dummy:HG: PMU Hardware doesn't support sampling/overflow-interrupts. Try 'perf stat' >> >> The issue is, if we enable the extended regs (-IXMM0), but the >> pmu->capabilities is not set with PERF_PMU_CAP_EXTENDED_REGS, the kernel >> will return -EOPNOTSUPP error. >> >> See following code pieces. >> >> /* in kernel/events/core.c */ >> static int perf_try_init_event(struct pmu *pmu, struct perf_event *event) >> >> { >> .... >> if (!(pmu->capabilities & PERF_PMU_CAP_EXTENDED_REGS) && >> has_extended_regs(event)) >> ret = -EOPNOTSUPP; >> .... >> } >> >> For software dummy event, the PMU should be not set with >> PERF_PMU_CAP_EXTENDED_REGS. But unfortunately in current code, the dummy >> event has possibility to be set with PERF_REG_EXTENDED_MASK bit. >> >> In evsel__config, /* tools/perf/util/evsel.c */ >> >> if (opts->sample_intr_regs) { >> attr->sample_regs_intr = opts->sample_intr_regs; >> } >> >> If we use -IXMM0, the attr>sample_regs_intr will be set with >> PERF_REG_EXTENDED_MASK bit. >> >> It doesn't make sense to set attr->sample_regs_intr for a >> software dummy event. >> >> This patch adds dummy event checking before setting >> attr->sample_regs_intr. >> >> After: >> # ./perf record -e cycles:p -IXMM0 -a -- sleep 1 >> [ perf record: Woken up 1 times to write data ] >> [ perf record: Captured and wrote 0.413 MB perf.data (45 samples) ] > > LGTM, Adrian (cc-ed) just added another check to the same place, > but it looks like both of them should be there: > > https://lore.kernel.org/lkml/20200630133935.11150-2-adrian.hunter@intel.com/ > > jirka >
Thanks Jiri! Yes, it looks like both of checks should be added here.
So do I post v2 (just rebase) once Adrian's patch gets merged?
Thanks Jin Yao
>> >> Fixes: 0a892c1c9472 ("perf record: Add dummy event during system wide synthesis") >> Signed-off-by: Jin Yao <yao.jin@linux.intel.com> >> --- >> tools/perf/util/evsel.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c >> index 96e5171dce41..df3315543e86 100644 >> --- a/tools/perf/util/evsel.c >> +++ b/tools/perf/util/evsel.c >> @@ -1020,12 +1020,12 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts, >> if (callchain && callchain->enabled && !evsel->no_aux_samples) >> evsel__config_callchain(evsel, opts, callchain); >> >> - if (opts->sample_intr_regs) { >> + if (opts->sample_intr_regs && !is_dummy_event(evsel)) { >> attr->sample_regs_intr = opts->sample_intr_regs; >> evsel__set_sample_bit(evsel, REGS_INTR); >> } >> >> - if (opts->sample_user_regs) { >> + if (opts->sample_user_regs && !is_dummy_event(evsel)) { >> attr->sample_regs_user |= opts->sample_user_regs; >> evsel__set_sample_bit(evsel, REGS_USER); >> } >> -- >> 2.17.1 >> >
| |