Messages in this thread | | | Date | Mon, 8 Mar 2021 14:14:40 +0100 | From | Jiri Olsa <> | Subject | Re: [PATCH v3] perf pmu: Validate raw event with sysfs exported format bits |
| |
On Mon, Mar 08, 2021 at 08:57:49PM +0800, Jin, Yao wrote: > Hi Jiri, > > On 3/8/2021 6:35 PM, Jiri Olsa wrote: > > On Mon, Mar 08, 2021 at 11:15:06AM +0800, Jin Yao wrote: > > > > SNIP > > > > > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c > > > index 44ef28302fc7..03ab1e6d0418 100644 > > > --- a/tools/perf/util/pmu.c > > > +++ b/tools/perf/util/pmu.c > > > @@ -1812,3 +1812,39 @@ int perf_pmu__caps_parse(struct perf_pmu *pmu) > > > return nr_caps; > > > } > > > + > > > +void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config, > > > + char *name) > > > +{ > > > + struct perf_pmu_format *format; > > > + __u64 masks = 0, bits; > > > + char buf[100]; > > > + unsigned int i; > > > + > > > + list_for_each_entry(format, &pmu->format, list) { > > > + /* > > > + * Skip extra configs such as config1/config2. > > > + */ > > > + if (format->value > 0) > > > + continue; > > > > sorry I did not notice before, but could you please use more direct > > approach like: > > > > if (format->value == PERF_PMU_FORMAT_VALUE_CONFIG) { > > break; > > } > > > > this will be more obvious, also no need for the comment.. I spent some > > time looking what's the value for ;-) > > > > thanks, > > jirka > > > > Oh, yes, using PERF_PMU_FORMAT_VALUE_CONFIG is much more obvious. Sorry about that! > > While it can't break the loop, because we need to iterate over the whole > list to get the total valid bits. So like: > > if (format->value != PERF_PMU_FORMAT_VALUE_CONFIG) > continue; > > Is it right?
sure, what I meant was to process only PERF_PMU_FORMAT_VALUE_CONFIG and then call break, because there's no need to iterate further
jirka
| |