Messages in this thread | | | From | Rob Herring <> | Date | Tue, 4 Oct 2022 12:07:30 -0500 | Subject | Re: [PATCH v3 1/3] perf: Skip and warn on unknown format 'configN' attrs |
| |
On Thu, Sep 29, 2022 at 1:48 AM Leo Yan <leo.yan@linaro.org> wrote: > > Hi Rob, > > On Wed, Sep 14, 2022 at 03:08:34PM -0500, Rob Herring wrote: > > [...] > > > +void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu) > > +{ > > + struct perf_pmu_format *format; > > + > > + list_for_each_entry(format, &pmu->format, list) > > + if (format->value >= PERF_PMU_FORMAT_VALUE_CONFIG_END) { > > + pr_warning("WARNING: '%s' format '%s' requires 'perf_event_attr::config%d'" > > + "which is not supported by this version of perf!\n", > > + pmu->name, format->name, format->value); > > + return; > > + } > > +} > > Though I saw you and Namhyung have discussion in underway, this patch > set is fine for me. I validated the patches at my side (with a bit > hacking in Arm SPE driver for faking invert filter). You could add my > tested tag for this patch set: > > Tested-by: Leo Yan <leo.yan@linaro.org> > > But I want to remind two things after I used "perf test" to validate > this patch set: > > $ ./perf test list > 6: Parse event definition strings > 6:1: Test event parsing > 6:2: Test parsing of "hybrid" CPU events > 6:3: Parsing of all PMU events from sysfs > 6:4: Parsing of given PMU events from sysfs > 6:5: Parsing of aliased events from sysfs > 6:6: Parsing of aliased events > 6:7: Parsing of terms (event modifiers) > $ ./perf test -v 6 > > The first one is this patch set introduces segmentation fault for the > case "Parsing of aliased events" (See tests/parse-events.c). But the > issue is caused by the test case itself; we need to add below line into > test_event_fake_pmu() for initialisation list header.
Thanks.
> INIT_LIST_HEAD(&perf_pmu__fake.format);
I ended up fixing this in perf_pmu__warn_invalid_formats() instead as the test dealing with internal stuct pmu details didn't seem right:
+ /* fake pmu doesn't have format list */ + if (pmu == &perf_pmu__fake) + return; +
> > The second question is for testing config3 in "perf test". You could > see the file tests/parse-events.c has included several test cases for > config/config1/config2. It's good to add the same testing for config3 > as well, please see test__checkevent_pmu() and test__checkterms_simple() > for relevant code.
Okay, will add in the next version.
Rob
| |