From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 26995C678D4 for ; Thu, 2 Mar 2023 20:28:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229677AbjCBU26 (ORCPT ); Thu, 2 Mar 2023 15:28:58 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57616 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229468AbjCBU25 (ORCPT ); Thu, 2 Mar 2023 15:28:57 -0500 Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 76BC816302; Thu, 2 Mar 2023 12:28:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1677788936; x=1709324936; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=/35HLiyYDFX+fShhJBDjECK1Nl4glfMnbm8AmHi0kt8=; b=Xls+RxrjDE/j1W8nVAoa40Yv3OzVTynM8glLm5OUA/bLHHnygXzSLSlS MLU5ahXBnuF3wDlvPymFo6cDPWSQpTpiwJLZAKL0wprcUcuhepPsjp7lk RtdMLCvwkVg7wiJhg//DdzsDow3zanptc5DqDFvQq9eu8Rbx+iaEBK4ra 8EiacVWisdeiQBozwWl46k2xJyZezPpu+JKRZBPlJPMyY6zKEPTzNK4H8 MTzX0uxPZd0Gqr7QpRqGGhM8/9+0cfwdN5XI+8xc+1ocmGgIq6JnHfuy5 zgG01D1/R86IQvmIWj3/N3cC+4//I5DhMrrV6jJEmeBVpbHoAoKkpe24T Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10637"; a="332327514" X-IronPort-AV: E=Sophos;i="5.98,228,1673942400"; d="scan'208";a="332327514" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Mar 2023 12:28:55 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10637"; a="764158004" X-IronPort-AV: E=Sophos;i="5.98,228,1673942400"; d="scan'208";a="764158004" Received: from linux.intel.com ([10.54.29.200]) by FMSMGA003.fm.intel.com with ESMTP; 02 Mar 2023 12:28:54 -0800 Received: from [10.209.43.74] (kliang2-mobl1.ccr.corp.intel.com [10.209.43.74]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by linux.intel.com (Postfix) with ESMTPS id 12148580689; Thu, 2 Mar 2023 12:28:51 -0800 (PST) Message-ID: Date: Thu, 2 Mar 2023 15:28:48 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCH v1 05/10] perf evsel: Limit in group test to CPUs To: Ian Rogers Cc: Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Zhengjun Xing , Ravi Bangoria , Adrian Hunter , "Steinar H. Gunderson" , Qi Liu , Kim Phillips , Florian Fischer , James Clark , Suzuki Poulouse , Sean Christopherson , Leo Yan , John Garry , Kajol Jain , linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org, Stephane Eranian References: <20230302041211.852330-1-irogers@google.com> <20230302041211.852330-6-irogers@google.com> Content-Language: en-US From: "Liang, Kan" In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-perf-users@vger.kernel.org On 2023-03-02 2:38 p.m., Ian Rogers wrote: > On Thu, Mar 2, 2023 at 7:24 AM Liang, Kan wrote: >> >> >> >> On 2023-03-01 11:12 p.m., Ian Rogers wrote: >>> Don't just match on the event name, restict based on the PMU too. >>> >>> Signed-off-by: Ian Rogers >>> --- >>> tools/perf/arch/x86/util/evsel.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/tools/perf/arch/x86/util/evsel.c b/tools/perf/arch/x86/util/evsel.c >>> index ea3972d785d1..580b0a172136 100644 >>> --- a/tools/perf/arch/x86/util/evsel.c >>> +++ b/tools/perf/arch/x86/util/evsel.c >>> @@ -61,6 +61,9 @@ bool arch_evsel__must_be_in_group(const struct evsel *evsel) >>> if (!evsel__sys_has_perf_metrics(evsel)) >>> return false; >>> >>> + if (evsel->pmu_name && strncmp(evsel->pmu_name, "cpu", 3)) >>> + return false; >> >> I'm not sure why we want to check the pmu name. It seems better to move >> it into evsel__sys_has_perf_metrics(), since perf_metrics is a core PMU >> only feature. >> >> I think the strncmp(evsel->pmu_name, "cpu", 3) is to check whether it is >> a core PMU. It is also used in other places. I think it's better to >> factor out it, e.g., arch_evsel__is_core_pmu(). It will deliver a clear >> message of what we are doing here. >> >> Thanks, >> Kan > > I missed the behavior of evsel__sys_has_perf_metrics and think we can > just drop this change. Yes, dropping the change is also OK for me. > We should probably rename > evsel__sys_has_perf_metrics perhaps something like > arch_evsel__pmu_has_topdown_events. The topdown is a tricky feature. For the big core, to support the topdown events, we have a dedicated perf metrics register, which has to be grouped with the fixed counter 3. That brings all of these troubles. Sorry for that. For the atom, the topdown events are still supported, but with GP counters. There is no perf metrics register at all. The evsel__sys_has_perf_metrics() is used to check whether the perf metrics register is supported. If so, we have to specially handle it. It's not to check whether the topdown events are supported. So I think it's better to keep the perf_metrics name, rather than topdown_events. Thanks, Kan > > Thanks, > Ian > >>> + >>> return evsel->name && >>> (strcasestr(evsel->name, "slots") || >>> strcasestr(evsel->name, "topdown"));