Received: by 2002:a05:6a10:1287:0:0:0:0 with SMTP id d7csp5254200pxv; Wed, 21 Jul 2021 00:40:30 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzRSQRWgxxf8Ao3fpCYQOu1MfZj+KT6P7yXvC7yHdhepSUuuAhxpuSJTioDM8F5Hv1SJdbB X-Received: by 2002:a05:6402:3507:: with SMTP id b7mr47149315edd.254.1626853229962; Wed, 21 Jul 2021 00:40:29 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1626853229; cv=none; d=google.com; s=arc-20160816; b=FeI6lTtSba4KRcAYSbESpIDRN6C/lOChHpdw8Fo3OHyghUFuCrZZ2qCnqnfi/fVizN JzW3REQnZaSt+NcgSKqP0WCqQiSIsAlLMiLAxWM8WZ4nWm2+akgjOEbNkQQVmLBPo8Y8 IlONKo8byN2K8DtQOOuP9xoDo8NTQVTMf6S+vqVnOX706qPLW+uRsmF3+GYWODkURXqX gbSPYeZXNFJtKdWOn4s+QBiO/vvPHHMH6axgP9N5yA/mJ5F84lPReHgP9nE0z7Np1Ote pht9dsw6hJ5FQqBZkDmLm0yDJRZpDEFGQhjd0B0QXfWZwteA0pllFet6itZmX+5AIRLl rk1Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:content-language :in-reply-to:mime-version:user-agent:date:message-id:from:references :cc:to:subject; bh=OwkVaedySM426hlidlHI2NMHUM+1Jx0768iYveQAL2w=; b=ROr9WbZmMPL062ykDkTGEuP3EtNVlNsLt/RYamfoFqCsEn2qXQRP6ZWyhYEcWJQW8q NPcEx7muueea2Oukh19E+GnJnPQbB4x71odqfRyebCNgXxQ1bnYVPKb5z6dewUxey8J9 nPg4y4IpPWAnkT3o6XjQUZ+Ikx7gCUahgZRkwFGVMjAzOdoDMFaDt+rJsZIOmh/hqMHG Z2MvIQb714/Gpd4a4SB/ARIwmChS70oxWNzy9o9i3oMByWMoYTGBE5dh+IAxHdM3fMWe oZ/olqpcp7cWOm1T6dFJLpAcCnPsVI1lNjnX1cO6ctLbOgQM8o/c8JjALZRMCu2tVorF yCdg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=huawei.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id a26si8681979edr.155.2021.07.21.00.40.06; Wed, 21 Jul 2021 00:40:29 -0700 (PDT) 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; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=huawei.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235761AbhGUG5d (ORCPT + 99 others); Wed, 21 Jul 2021 02:57:33 -0400 Received: from frasgout.his.huawei.com ([185.176.79.56]:3440 "EHLO frasgout.his.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236038AbhGUG5H (ORCPT ); Wed, 21 Jul 2021 02:57:07 -0400 Received: from fraeml742-chm.china.huawei.com (unknown [172.18.147.206]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4GV6Vm5QWVz6D8n1; Wed, 21 Jul 2021 15:22:56 +0800 (CST) Received: from lhreml724-chm.china.huawei.com (10.201.108.75) by fraeml742-chm.china.huawei.com (10.206.15.223) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.2; Wed, 21 Jul 2021 09:37:42 +0200 Received: from [10.47.85.43] (10.47.85.43) by lhreml724-chm.china.huawei.com (10.201.108.75) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.2; Wed, 21 Jul 2021 08:37:41 +0100 Subject: Re: [PATCH] perf pmu: Fix alias matching To: "Jin, Yao" , , , , , , , , , CC: , References: <1626793819-79090-1-git-send-email-john.garry@huawei.com> <0b57fa9b-fba4-8143-bef6-b7c4f2987635@linux.intel.com> From: John Garry Message-ID: <0752a2b1-4770-4614-3b3f-73752a7d962a@huawei.com> Date: Wed, 21 Jul 2021 08:37:38 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.1 MIME-Version: 1.0 In-Reply-To: <0b57fa9b-fba4-8143-bef6-b7c4f2987635@linux.intel.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [10.47.85.43] X-ClientProxiedBy: lhreml734-chm.china.huawei.com (10.201.108.85) To lhreml724-chm.china.huawei.com (10.201.108.75) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org >> >> Fixes: c47a5599eda3 ("perf tools: Fix pattern matching for same >> substring in different PMU type") >> Signed-off-by: John Garry >> --- >> @Jin Yao, please test for your scenarios >> > > For x86, the form uncore_pmu_{digits} or the uncore_pmu itself are > supported. We don't have more complex case such as the name in the form > aaa_bbbX_cccY. So my test didn't cover that complex form. > My next thing to do is to add support for these more complex scenarios in the PMU events self tests > For my test, your patch works, thanks! :) Good > >> Note: >> About any effect in perf_pmu__match() -> perf_pmu__valid_suffix() >> callchain, this seems to be called for wildcard in PMU names in metric >> expressions. We don't have any metrics for arm64 which use feature. >> However, I hacked an existing metric to use a wildcard and it looks ok. >> Also the "DRAM_BW_Use" metric on my broadwell uses this feature, and it >> looks ok. >> >> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c >> index a1bd7007a8b4..fc683bc41715 100644 >> --- a/tools/perf/util/pmu.c >> +++ b/tools/perf/util/pmu.c >> @@ -742,9 +742,13 @@ struct pmu_events_map *__weak >> pmu_events_map__find(void) >>       return perf_pmu__find_map(NULL); >>   } >> -static bool perf_pmu__valid_suffix(char *pmu_name, char *tok) >> +/* >> + * Suffix must be in form tok_{digits}, or tok{digits}, or same as >> pmu_name >> + * to be valid. >> + */ >> +static bool perf_pmu__valid_suffix(const char *pmu_name, char *tok) >>   { >> -    char *p; >> +    const char *p; >>       if (strncmp(pmu_name, tok, strlen(tok))) >>           return false; >> @@ -753,12 +757,16 @@ static bool perf_pmu__valid_suffix(char >> *pmu_name, char *tok) >>       if (*p == 0) >>           return true; >> -    if (*p != '_') >> -        return false; >> +    if (*p == '_') >> +        ++p; >> -    ++p; >> -    if (*p == 0 || !isdigit(*p)) >> -        return false; >> +    /* Ensure we end in a number */ >> +    while (1) { >> +        if (!isdigit(*p)) >> +            return false; >> +        if (*(++p) == 0) >> +            break; >> +    } > > Do we check *p before first isdigit? For example, > > if (*p == 0) >     return false; > > While (*p) { >     if (!isdigit(*p) >         return false; >     ++p; > } > > But maybe isdigit can handle the null string well. I'm just feeling a > bit unsure. > isdigit() can safely handle 0 and returns 0 for that case, so what I added should be ok >>       return true; >>   } >> @@ -789,12 +797,19 @@ bool pmu_uncore_alias_match(const char >> *pmu_name, const char *name) >>        *        match "socket" in "socketX_pmunameY" and then >> "pmuname" in >>        *        "pmunameY". >>        */ >> -    for (; tok; name += strlen(tok), tok = strtok_r(NULL, ",", &tmp)) { >> +    while (1) { >> +        char *next_tok = strtok_r(NULL, ",", &tmp); >> + >>           name = strstr(name, tok); >> -        if (!name || !perf_pmu__valid_suffix((char *)name, tok)) { >> +        if (!name || >> +            (!next_tok && !perf_pmu__valid_suffix(name, tok))) { >>               res = false; >>               goto out; >>           } >> +        if (!next_tok) >> +            break; >> +        tok = next_tok; >> +        name += strlen(tok); >>       } >>       res = true; >> > > My test didn't cover the tokens which were delimited by ','. I assume > you have tested that on arm64 system. :) > Right Thanks, John