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: <72e58cc7-acde-600d-b9a1-b89d9d8a168b@amd.com>
Date:   Wed, 14 Jun 2023 09:19:59 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH 3/4] perf mem amd: Fix perf_pmus__num_mem_pmus()
Content-Language: en-US
To:     Ian Rogers <irogers@google.com>
Cc:     acme@kernel.org, jolsa@kernel.org, namhyung@kernel.org,
        mark.rutland@arm.com, peterz@infradead.org,
        adrian.hunter@intel.com, kan.liang@linux.intel.com,
        james.clark@arm.com, alisaidi@amazon.com, leo.yan@linaro.org,
        maddy@linux.ibm.com, linux-perf-users@vger.kernel.org,
        linux-kernel@vger.kernel.org, sandipan.das@amd.com,
        ananth.narayan@amd.com, santosh.shukla@amd.com,
        Ravi Bangoria <ravi.bangoria@amd.com>
References: <20230613095506.547-1-ravi.bangoria@amd.com>
 <20230613095506.547-4-ravi.bangoria@amd.com>
 <CAP-5=fW0R2Dkwvw0twdzL5GrLy6jy0CXmFHYCsZtC8UQnZ4mVQ@mail.gmail.com>
From:   Ravi Bangoria <ravi.bangoria@amd.com>
In-Reply-To: <CAP-5=fW0R2Dkwvw0twdzL5GrLy6jy0CXmFHYCsZtC8UQnZ4mVQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Z0lmSHVvUERjNGoyTUZWTGloMjFiMXpNT2pKL1B2QjlQQ09IbU1KN0I1blFn?=
 =?utf-8?B?bFpaK253Mm5PRFRxTGxpbTk5SXc1elpxRjhjK3dnVzZaRGhpYkFIcnRpN3J1?=
 =?utf-8?B?RmRmY2FPM3FGaUU3Qlg3dW81RTJuZXQrK2RhTEtkZHgySEw0Vy9jK0pSVnFQ?=
 =?utf-8?B?dm83aHdpZjFaMG1hVTRCTmFDZHM2VG12MjM0alBpY0ZMaFMzTC9BUjVqQUdt?=
 =?utf-8?B?RzlyWHQ0ZUJyaHBVYlUzUnFYVzhKZXFKZG55STBtWm9RNmZCU1RGdGphK09Q?=
 =?utf-8?B?THdTc1FGblF1OCtoUm9EWWRXNVMrU2hybGl6WGFvS3NGK2xQcHB5M05kUnhS?=
 =?utf-8?B?S015dE1UQjlkMGZHUTNzNzRQTUxtK0hweHhKZWo4UzJBVjhDOWFWUlM4dFZH?=
 =?utf-8?B?TndTZDkwcHhnYitHUjB6V0NSMDhLMnUrU1Vla01oRjBJdi9jQXJoa1piTXF1?=
 =?utf-8?B?M3R2VHBjNmZwbjlWYVRmN0F4TUVQSTFFL1FaV3lYUTVPVGhRYU9USWIxa2lY?=
 =?utf-8?B?cUljRkFtNEpoanJIVnRZSUlqbUl6VTVBNEgyQ1E5ZkJ3MkdSTGpEYnlsaStn?=
 =?utf-8?B?c2hYMERKcldTSjVzQWE2YkpESjR5RUxNeFZtT3NkNndwb3VxWjZMbE1vYzJt?=
 =?utf-8?B?c1I4enZtbUhVU1IyenBFMlJvV3ZiVFZjKzdyV3J4Ri95ZjBGR2NxQjE4TThS?=
 =?utf-8?B?TXBDN0ZMMFV0WVYyU2ZrTHBnYXgwWTZpRGJYb1g3SzhCYTdwQStleU5Za0ho?=
 =?utf-8?B?eXNzV01HSnh3U3NYUy9idjBlaW1iK2hzUktIUWs4NkUvM2ducmJnMDk3ODNX?=
 =?utf-8?B?dmx0dXgyZkJWTjBvaDJZak8vZmFYRzlCNmIwQUEvdmVHeUdXY1ErckEyaTA4?=
 =?utf-8?B?eDhtZUZMcitQcE1KaHRQN0sxTi9oUUhqMzR0RC9WeTJheC9FN3FYMElQUzlD?=
 =?utf-8?B?bVhUaGplNGtEMlNkd1ozdHZTTmZSV0MzYmY4MUpoTEd2NVNTRC8vc3l0Nkh0?=
 =?utf-8?B?cWR3M1JwNCtIcE5pT1dxS0FNQitXTXZ1WVQ3OEFRdXVjUFFZZk4zUmtNYnhJ?=
 =?utf-8?B?ckQ4UGdTZEp5dzJNMDdUWWllbnFKc0xjck9acXVQT2Z3bEpuNVZOMjVSQldH?=
 =?utf-8?B?eW5rb1RCR05SL3B4YkNFeDYxRVVrTHY4SHNQZFRqVStHaW9BUjdveTdBUUwv?=
 =?utf-8?B?Mll0eHNEclN6RmlWdDFGQWpiUXVaS1BQZkpXS0RGeDIrbFNUTHdjWUEwZkNn?=
 =?utf-8?B?dXlVSUg2VjYzNEhsa1N3YllFUGh2elhGd2dBYU1ma0hMVzZMOHIyaTBpQ3dB?=
 =?utf-8?B?RXhKaFhxME1FY1k4N3lXRnVMcUZsdGNLRnRDN0d2WFJKbWYvY0tJcHBiN3Rt?=
 =?utf-8?B?UHh5MU1RcTdQQWdZUWw0N2lGTUNzZ1VEb09wLy91cW8xK0Y0WWEweWVnWnds?=
 =?utf-8?B?UjYrTWZxcGhkU0ZyN0FScGVOd1RQK3RUWSsyRXllMGlXa1dDaDBhR0I1Yk13?=
 =?utf-8?B?c2dMUGtNUHNCM2hPWnBoWVl5dzBPZGJYeU92RnlMaUVxZTRUWWpLeWFXeUc0?=
 =?utf-8?B?alRyVEorclgrcEd4R005clJRRmFTbVdvNmVmdU94dU4vcVJ2Tk5HU2l2ajNo?=
 =?utf-8?B?eEt6UG04N2dzV2lJK1NsaitmbkorSzJrSXV2dmhRSVUxOUNUTEU3QlRiZStr?=
 =?utf-8?B?WFNMa3BQVW54SDRRM0k4L1VyMW9IVkVjV29xSW5IUDJ4N0IvOGs5V2VoUmNs?=
 =?utf-8?B?elpqZmdldWptSkNoVEprUm1SNDg2aW5DOFdGclNZYnRtSVFRWTVhRU5FbVJj?=
 =?utf-8?B?OUx1eDVJdk5IYndzUnZXUXZIOGtCWTJxMDRzWlNFQnp1QnJQMTMwdTk3cHVn?=
 =?utf-8?B?TENQbnAwM3JEZURlbnRJVGx1Wk92TXBkdUoraXJUVklSZDFrbWtvNVRqVWcv?=
 =?utf-8?B?ek12MmIxYTErZGdMaWYya0hTRWo1RWZZTVRNNy9QMzA4Mm42Ylk2clVOaWt1?=
 =?utf-8?B?TGltMjBwcy82MXlVSThvd3hBQVhpNHlOWmR5QzJjRGpKQWZpamppWDl5T2NW?=
 =?utf-8?B?WUFLU1lKcHlkU3hiMXBkOG9KOTcxcjFnWDVCVjJjYnVFYTVrbjZQR1N1T0s2?=
 =?utf-8?Q?sGM/iyztK2ByE9+IgdC/wFCEv?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6e5ec6cb-8aa0-4f76-3e65-08db6c8a779c
X-MS-Exchange-CrossTenant-AuthSource: PH7PR12MB6588.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Jun 2023 03:50:17.4415
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 9GwiwAe362dTge3WBRqkO7np50VWVHH67EoGRl8Hf0L3L9gQnv5DCmRrxPT+Ilem7VE/DR4hmPkMpNuK8Qc5oA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB4064
X-Spam-Status: No, score=-1.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,NICE_REPLY_A,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no 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

>> +int perf_pmus__num_mem_pmus(void)
>> +{
>> +       /*
>> +        * AMD does not have hybrid cores and also uses IBS OP
>> +        * pmu for perf mem/c2c.
>> +        */
>> +       if (x86__is_amd_cpu())
>> +               return 1;
> 
> The code and comment seem out of sync here. For the hybrid part
> perf_pmus__num_core_pmus() will yield 1 if there is no hybrid, so we
> can just use perf_pmus__num_core_pmus(). For the IBS OP part, does
> that mean that AMD should have 2 mem pmus? Or is IBS OP a core PMU?

Sure. Let me remove hybrid part from the comment.

There are two IBS pmus: ibs_fetch// and ibs_op//. Both of them are
independent of the core pmu (cpu//). An instance of all 3 pmus is
present in each hw SMT thread. And, perf mem/c2c internally uses
ibs_op// pmu on AMD. See tools/perf/arch/x86/util/mem-events.c

  static struct perf_mem_event perf_mem_events_amd[PERF_MEM_EVENTS__MAX] = {
          E(NULL,         NULL,           NULL),
          E(NULL,         NULL,           NULL),
          E("mem-ldst",   "ibs_op//",     "ibs_op"),
  };

Hope this clarifies.

> Can we add this as an example in the core/other documentation in patch
> 1, as you've done for ARM, for clarity.

Sure. Let me also add IBS example in the patch #1 comment.

Thanks,
Ravi