Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v1] perf tools: Fix pattern matching for same substring
 used in different pmu type
To:     Jiri Olsa <jolsa@redhat.com>, "Jin, Yao" <yao.jin@linux.intel.com>
Cc:     acme@kernel.org, jolsa@kernel.org, peterz@infradead.org,
        mingo@redhat.com, alexander.shishkin@linux.intel.com,
        Linux-kernel@vger.kernel.org, ak@linux.intel.com,
        kan.liang@intel.com, yao.jin@intel.com
References: <20210609045738.1051-1-yao.jin@linux.intel.com>
 <982714a5-8a5d-8f8a-4e30-bd9a497ffa40@linux.intel.com>
 <4787334d-cf28-5b25-8d11-c767c52288f1@linux.intel.com>
 <YNWr7zsEaNPCn4CR@krava>
 <14a70048-ddd0-3297-9ae9-6b76dd0f1000@linux.intel.com>
 <YNuNW/Afd/X25fNe@krava>
From:   "Liang, Kan" <kan.liang@linux.intel.com>
Message-ID: <844625a7-5903-519e-9ef4-ca6684661aef@linux.intel.com>
Date:   Tue, 29 Jun 2021 17:47:56 -0400
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <YNuNW/Afd/X25fNe@krava>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk


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