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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aa963629-f52f-d42b-de30-298f5755eb49@linux.intel.com>
Date:   Wed, 30 Jun 2021 16:15:26 +0800
From:   "Jin, Yao" <yao.jin@...ux.intel.com>
To:     "Liang, Kan" <kan.liang@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>
Cc:     acme@...nel.org, jolsa@...nel.org, peterz@...radead.org,
        mingo@...hat.com, alexander.shishkin@...ux.intel.com,
        Linux-kernel@...r.kernel.org, ak@...ux.intel.com,
        kan.liang@...el.com, yao.jin@...el.com
Subject: Re: [PATCH v1] perf tools: Fix pattern matching for same substring
 used in different pmu type

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@linux.intel.com/
> 
> 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