Re: [PATCH v1] perf tools: Fix pattern matching for same substring used in different pmu type

[Jin, Yao]

Hi Jiri, Hi Kan,

On 6/30/2021 5:47 AM, Liang, Kan wrote:
> On 6/29/2021 5:15 PM, Jiri Olsa wrote:
> > On Mon, Jun 28, 2021 at 09:52:42AM +0800, Jin, Yao wrote:
> >
> > SNIP
> >
> > > > > > > +    /*
> > > > > > > +     * The pmu_name has substring tok. If the format of
> > > > > > > +     * pmu_name is <tok> or <tok>_<digit>, return true.
> > > > > > > +     */
> > > > > > > +    p = pmu_name + strlen(tok);
> > > > > > > +    if (*p == 0)
> > > > > > > +        return true;
> > > > > > > +
> > > > > > > +    if (*p != '_')
> > > > > > > +        return false;
> > > > > > > +
> > > > > > > +    ++p;
> > > > > > > +    if (*p == 0 || !isdigit(*p))
> > > > > > > +        return false;
> > > > > > > +
> > > > > > > +    return true;
> > > > > > > +}
> > > >
> > > > hum, so we have pattern serch and then another function checking
> > > > if that search was ok..
> > >
> > > Yes, that's what this patch does.
> > >
> > > I understand that's convenient, because
> > > > it's on 2 different places
> > >
> > > Yes, on pmu_uncore_alias_match() and on parse-events.y.
> > >
> > > but could we have some generic solution,
> > > > line one function/search that returns/search for valid pmu name?
> > >
> > > Sorry, I don't understand this idea well. Would you like to further explain?
> > >
> > > Or can you accept the regex approach?
> >
> > I don't really have any suggestion, just would be great to have
> > this encapsulated in one function.. 
>
> Yes, I agree. One function is better.
>
> We just changed the design for the uncore PMU on SPR. There will be two PMU names for each uncore 
> unit, a real name and an alias. The perf tool should handle both names. So we have to compare both 
> names here.
> I think one generic function can facilitate the code rebase.
>
> https://lore.kernel.org/lkml/1624990443-168533-7-git-send-email-kan.liang@xxxxxxxxxxxxxxx/
>
> Thanks,
> Kan

Thanks for the comments!

I'm now preparing v2. In v2 version, I will provide a new function 'perf_pmu__pattern_match which 
wraps the matching and checking. Something like:

while ((pmu = perf_pmu__scan(pmu)) != NULL) {
	if (!perf_pmu__pattern_match(pmu, pattern, $1)) {
		...
	}
}

I will post v2 soon.

Thanks
Jin Yao