Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <bb828dd4-3c22-7383-008e-ab4f860ed686@oracle.com>
Date:   Mon, 3 Jul 2023 14:09:05 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH RFC 2/9] perf metrics: Don't iter sys metrics if we
 already found a CPU match
To:     Ian Rogers <irogers@google.com>
Cc:     acme@kernel.org, namhyung@kernel.org, jolsa@kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
        renyu.zj@linux.alibaba.com, shangxiaojing@huawei.com,
        zhangshaokun@hisilicon.com, qiangqing.zhang@nxp.com,
        kjain@linux.ibm.com, kan.liang@linux.intel.com
References: <20230628102949.2598096-1-john.g.garry@oracle.com>
 <20230628102949.2598096-3-john.g.garry@oracle.com>
 <CAP-5=fWB9H0KWrXYouepUiAbnfH3u29XuRVN6KXVw3CNbD4xrA@mail.gmail.com>
Content-Language: en-US
From:   John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <CAP-5=fWB9H0KWrXYouepUiAbnfH3u29XuRVN6KXVw3CNbD4xrA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dUYrYytSL1J5eEFiK3IzbXdHTVUrdWhKWVRtNmJYQktINjBEZnY3RDFKQlp4?=
 =?utf-8?B?NjBvaXo2ZmV2aHdpQjdLRzlleUt6VGxIcENvMDlmTXJSeTVYdE5rZmxWRzNR?=
 =?utf-8?B?WWRZdHBUOWY2aUgyRDVRN09qZ3VjL2tzakFUVUFQai9mT29ZcWJuMEM4cURK?=
 =?utf-8?B?b0crWUlnNC9Xa3dJTU9WZnBqRlp5SmQ0c051dW9hUDkvMGM0eW1XU0tBQk01?=
 =?utf-8?B?WTFVSThVaERDbjFmWnhiY3VhbkxKeWZ6bU5PWnJuTytIWUdNaC94dHFoN2FO?=
 =?utf-8?B?ZmNXM1pGV1QwQ3IwK2ttMjV4U3VHRWdVV1M5SE1JRUZTcWRzSVQwSUJIYzdG?=
 =?utf-8?B?dC93UitkUWNPYXJWZVZHMFpnVm05ZUpDYldmeWQxaGYyS29xTHpUdWpmYUk1?=
 =?utf-8?B?VmZSa1o4Zkg4NFJFa1BGY0pBU3BUeDJBc1NUS3RjUUhPZnpiQ1YxVlZIL29C?=
 =?utf-8?B?QXJHZjJWV3JEZjRlVnRmRjZEUnhCd3Fnd1BCSjFHdXFWclJraEJ4ZHQ1WWM4?=
 =?utf-8?B?b3dZNlNMQTVDaEVDRTFYVUduMTM4Z2l4UjlBcjlMTytrTnBNQjJqeEpwZ3VW?=
 =?utf-8?B?aCs3NFlCQndyVDZ4a3M2WXQwbzliN3V1dHlJTmx0bHVIRHNFQ25rWmNVT1NL?=
 =?utf-8?B?Zld2TzYrVDVPalV0Yi9NSUhoblNybzIrMXN4MmpIKzNkc21MNVk3Zm9tdzJu?=
 =?utf-8?B?Z0VJQ3lxYmw2RWpOMHNDZ1czU21IblVBTW5ES0ZMZmhYYUUzNy9adlc3dlha?=
 =?utf-8?B?YWdLa3hURjJWa0IzN0d1KzFkNWNtYm0veWVodUhOaGdpVHlIeGc1d1BKUFRV?=
 =?utf-8?B?UHhCdlhpbmpsYjlXSmhLZVhYaW81WWtESGVYMnhtcXl0U2lTQkNWQWV3OFk2?=
 =?utf-8?B?UkJqeUxBYnc2RzRSOUsycjdLZWtkTWdrYmtNTEJpNlV0R2NQL09MOFZrdEto?=
 =?utf-8?B?WkpyOXVhRGRreHdOQmtUM2QybnQxeTl2d041WVhvM1pjUi8yNGJ6YkpVZjVh?=
 =?utf-8?B?bUxQOGszRy9XblE5ak5UZHcvV25OTTQzWE50dnlzNytpS0I4WFc0VjN4UkVi?=
 =?utf-8?B?YnRzN0IwOHh0Yjk5b2tFaEFjS1RaWUpYY09BeWl1Y2RYbmdHNWFRaUpZZlBr?=
 =?utf-8?B?ejNtRWQ2MUhiTXJCN1pWdWVNUC9OMWY1OWVlZUFBNVQybVNTR2x4VHdEdWlR?=
 =?utf-8?B?YVlzaFYwck1vbTYraTJyNkQwNXpKRzk1cm1RMFZSVndHcDZhRFEzb28zNHhQ?=
 =?utf-8?B?MjJnR0VuSUNzRk8zNUJ0a3RoaWFjR1d3MmFEODlMaHd0cGJHL1dyRngzSjVN?=
 =?utf-8?B?ejhTdmZycjB5UHFpdThyTmswOXd2TmwvVUlRajZDSmFiYkJBcTZsRmppMS9T?=
 =?utf-8?B?ODJpdVhOS3Y0c24yUnVrOVcwdm9QbzlTMFJ4bHRWVGgyMlo0UG1wMk81VUFL?=
 =?utf-8?B?OEdKQTJQcWZ6T2U0dHBGbGg2L3F1V1M4SWg0L2paYWZXT3c0bkNWTmNLTzA0?=
 =?utf-8?B?T3RGSHp3TTUyMFFFT2JieVB2bGloZ0ljVzRoNnAwYWxYM08wem50NE9tSEJH?=
 =?utf-8?B?TU5HMGZMY2pNeFkra1pPemhDTjJBdmtRVS9NL25kbWxlZTNBenJRTms2bUJ4?=
 =?utf-8?B?b0ZoTU9xbnQ4WVhHNU9pekZ5VTN3MmoyNVdhbHkxSXBtQS9zZVBOTEtXR2pl?=
 =?utf-8?B?UjN0RjJoQXR5VjJoVnNocWRVR1dzNG92ZU13Vk9LQnM2cmR6ZXoxZDJHOTkv?=
 =?utf-8?B?QmJpbDB0d0grTEZZa25GNnlXQXQ5dTUxQ09ya2NRWStnS281Y1ZlVnVqRUZv?=
 =?utf-8?B?aGtROVRiM050bjl0WGorOURObTBoQ00zZ3Jsb1FMcEVTZUZSRDdPbDNYcWcy?=
 =?utf-8?B?WTc3NTlkU05WdStNOUFLd1IvVzBxVGNHaXV3dlpyMTd1VkVpUFRBS2JCSDc5?=
 =?utf-8?B?UzN6VWVudnZibStRdSsrR21tcHRKdVNKNHg1U1hZNFN0WC93UFJBRzhUcmRW?=
 =?utf-8?B?NGVjUjR4V2NxSXpGZ1RFY1V3YkEwSEt4ZnpDeEg2MUhreEk4MUZNRmpHbzVE?=
 =?utf-8?B?VkhWM2l4ajlEN2djRVVDa1NwbmppOWtoZEhtc21MemFrWE1tSmxIZFFqeTVw?=
 =?utf-8?Q?ae4RLlk2YIZy6rlk3eKBuCjcR?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?VzRHUnBUMHFycVB4U25HMWxYeCthc2NKQXNBSWtQSnRrS3hVWWpRd1JZM09m?=
 =?utf-8?B?TWdTMC92Tlk0QkZyQ3pXdzZDNDRLNHBkSkVMaDl6VUdzOFBaeldTN1VFNjI2?=
 =?utf-8?B?MFNQVytkYzBLM2x3OTA3MEc2RFdZaHdmcGpZTEZ3UmVJZTVqOTlmT284Uzhv?=
 =?utf-8?B?WUkvS3o4OTczZTBuMmZad3Y1Z1F2OU1kcXZ2VExEK3JCN212aDZsMitPZkp5?=
 =?utf-8?B?RkxsRWdmckpkZndVelh2YUt6Y1U2YnVSbDZjdjdDT3R5TzZrZjJwVHB3MDFq?=
 =?utf-8?B?cjAweEtYUUVtRWhMVms3clMvK3hhcGxIQ211ZVFGVE44a2hSWnQzV2MvYlph?=
 =?utf-8?B?Yk1mQlZNY1kzcWFydjRlZnI3a2FkZjM0ZXNsTFMxZkJiWTBCeDdMaHBIZS9V?=
 =?utf-8?B?SnRTbXJ3MG9qTUtxd3JvQ01hOGlvQUV0dGlkSUhtajNIandDZnRBNXF3b0hl?=
 =?utf-8?B?emhKNGcwMUJrTkNaazBoRHMxS0lUM1l5VFNoMmNRUm5kU042ekt4bHk5U0VS?=
 =?utf-8?B?TEpmQ0Rmd2E2OTY5eXZsUHdBU25vbXNlaUxyOUduaWRqYlVFYkJiOG1wbWlE?=
 =?utf-8?B?RGVnZk5yNkJPcVprWVBqbFhIUU9tSGRtRzVDUVdGYVlwaDh4VVBjZXE2TnZF?=
 =?utf-8?B?REdpYXA2a0txNHZ4d3RSYUsveWtJUjB6MmVPdzRJWm5XSXdDZElsVU04M2Rs?=
 =?utf-8?B?MDREY0xFR0J5N1czeWhPZHdiaUZJQUFpcDJGcTZUWmhLdGgwRjZIL3NJaVBy?=
 =?utf-8?B?NU5lNkEramRONThvckxQbVF4Y085OG4vb1pSUzIzekhYU0dtdVBFU25vM3VI?=
 =?utf-8?B?WHZ2T0RQN1NJaWJ0cDEyanNLNUhDa20rV3JHMlRHTUZoQ0lycExkZm1pSm9s?=
 =?utf-8?B?Z0VtU2M3UGVmL0w2VGdNWGZMaXVrK1MvdlhhZDQ5eWQwY0w2RDEzYUlpMFND?=
 =?utf-8?B?eVVkSzRBakJ0UjRtbzJSb2pxaUlUYVJzdnBOblZSZ3ZSemwrbmljZFkzT0JU?=
 =?utf-8?B?amdMdWVwL2R0cmhwdlRnNFVPbC96TEpYelN5bUJ5aklYSXJIYVZXakRWY2Yr?=
 =?utf-8?B?Rk55RzFrNXdvODBYYjBGNk5XaXdWWThTQlBjaVNmYkhZeW85RGlJdU1TL2xG?=
 =?utf-8?B?eFpKbFA5bk5DdGFDNGV0Y1FMc2ZlMmdSNFdCZHZEN1BMZDNnQldDTE5WeHdv?=
 =?utf-8?B?RWRLd1Q5Q1ZHSGxaQ2hqc0pobDN6c2ZNaVNkZVYxVEwvYUFXa0N1cXgwZkRE?=
 =?utf-8?B?TDNubjhJeVY1NjMxcXJ3N3g0bUxnY3hic0ZPQzAyckhmTk5lVkZkRVY3RllD?=
 =?utf-8?B?UGZ2N2dkMzhEZE1QS3k5c2c1UURzZGRPYVRwaS94Q3JuYWdMeUtuWm9KcmFK?=
 =?utf-8?B?S0tMNFlzYmFoamZYVVg2YTJLZUJkMkpFTW5LL2tMTG1hbTh5Z1QrcGRwZ0Rv?=
 =?utf-8?B?ZmdreEhVSUtRb08zKzcrcnQ2YmE3MTdEcGZxRDhRPT0=?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2e02f5c0-d1d8-40c4-9485-08db7bc6b098
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Jul 2023 13:09:10.1899
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: g8VHg1nGuk9yqHiYfXdXj6RBcIf1iu2RGd0kZzBa36NltO3eSSECzSiNqD23NU8DGsU2bUndZqXwT2bqiA+e9Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR10MB7339
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26
 definitions=2023-07-03_10,2023-06-30_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 mlxlogscore=999
 suspectscore=0 bulkscore=0 spamscore=0 mlxscore=0 adultscore=0
 malwarescore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2305260000 definitions=main-2307030119
X-Proofpoint-ORIG-GUID: JIxZczyvPcyTgQ4lVfDWo9ME_NeDgyWN
X-Proofpoint-GUID: JIxZczyvPcyTgQ4lVfDWo9ME_NeDgyWN
X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 30/06/2023 18:41, Ian Rogers wrote:
> On Wed, Jun 28, 2023 at 3:30 AM John Garry<john.g.garry@oracle.com>  wrote:
>> In metricgroup__add_metric() we still iter the sys metrics if we already
>> found a match from the CPU table, which is pretty pointless, so don't
>> bother.
>>
>> Signed-off-by: John Garry<john.g.garry@oracle.com>
>> ---
>>   tools/perf/util/metricgroup.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
>> index 4389ccd29fe7..8d2ac2513530 100644
>> --- a/tools/perf/util/metricgroup.c
>> +++ b/tools/perf/util/metricgroup.c
>> @@ -1261,6 +1261,12 @@ static int metricgroup__add_metric(const char *pmu, const char *metric_name, con
>>
>>                  has_match = data.has_match;
>>          }

Hi Ian,

>> +
>> +       if (has_match) {
>> +               ret = 0;
>> +               goto out;
>> +       }
>> +
> I think this can just be:
> 
> if (!has_match)

But ret has no initial value

> 
> However, I'm not sure I agree with the intent of the change. We may
> have a metric like IPC and want it to apply to all types of CPU, GPU,
> etc. If we short-cut here then that won't be possible.

A few points to make on this:
- Currently we don't have any same-named metrics like this, so not much 
use in supporting it in the code (yet).
- Even if we had some same-named metrics, I am not sure if it even works 
properly. Do we have any uncore PMU metrics which have same name as CPU 
metrics?
- Further to the previous point, do we really want same-named metrics 
for different PMUs in the future? I think event / metric names need to 
be chosen carefully to avoid clash for other PMUs or keywords. For your 
example, if I did ask for IPC metric, I'd like to be able to just know 
I'm getting IPC metric for CPUs or some other PMUs, but not both.

Thanks,
John

>