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 71627EB64DD for ; Fri, 11 Aug 2023 15:47:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233433AbjHKPrE (ORCPT ); Fri, 11 Aug 2023 11:47:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35262 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236439AbjHKPrC (ORCPT ); Fri, 11 Aug 2023 11:47:02 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 22EC7270F; Fri, 11 Aug 2023 08:47:00 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id AC5B664DE7; Fri, 11 Aug 2023 15:46:59 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A5815C433C7; Fri, 11 Aug 2023 15:46:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1691768819; bh=eltIzQyV4cMYnPohsCHCTRWfiqKbJADJmTzB21iR7Oc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=VfATPPpN3mE7Qw/AfWV/bQ6xNc+JHdeH3QvU5jwdNcF1OO/S7+b/ZfDhzyQ+4AcGe oGc1qFULWjlb/2ti7l351Q/O7buOrcOUa4e/hUolKuMN7WsfAAgjm5969pN01nH9aj Uc4EeErhvA1le1HM+HbtuL1N1D5UVXFschAM+6SMp/v/U+9tHXI3x6uPnDN+muVnCh vEF5tgby9IR9O3LQg8kN0POA0x6qNyhh5nmJyJPL69AZa8Bhr1AiZk7h+xNh7lDTsv LItTasiYpqp2XZdbDThvqzCuSaGUS92yFa6bhpYQeJDn8t4ATMUEu1jRSZro1LjAci X8BC2ghTa1l7A== Received: by quaco.ghostprotocols.net (Postfix, from userid 1000) id 3B929404DF; Fri, 11 Aug 2023 12:46:56 -0300 (-03) Date: Fri, 11 Aug 2023 12:46:56 -0300 From: Arnaldo Carvalho de Melo To: Ian Rogers Cc: Peter Zijlstra , Ingo Molnar , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Adrian Hunter , Kan Liang , Ravi Bangoria , Kajol Jain , John Garry , linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/3] perf pmus: Sort pmus by name then suffix Message-ID: References: <20230810214952.2934029-1-irogers@google.com> <20230810214952.2934029-2-irogers@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Url: http://acmel.wordpress.com Precedence: bulk List-ID: X-Mailing-List: linux-perf-users@vger.kernel.org Em Fri, Aug 11, 2023 at 08:19:00AM -0700, Ian Rogers escreveu: > On Fri, Aug 11, 2023 at 6:56 AM Arnaldo Carvalho de Melo > wrote: > > > > Em Thu, Aug 10, 2023 at 02:49:50PM -0700, Ian Rogers escreveu: > > > Sort PMUs by name. If two PMUs have the same name but differ by > > > suffix, sort the suffixes numerically. For example, "breakpoint" comes > > > before "cpu", "uncore_imc_free_running_0" comes before > > > "uncore_imc_free_running_1". > > > > Why is this needed? > > It is needed so that in the later patches we just "perf list" the > uncore_imc_free_running_0 and skip all the other suffix numbers. > Sorting using strcmp isn't sufficient as consider uncore_imc_10 and > uncore_imc_9, where 9 would appear before 10 if only the characters > were being compared. I think there will be a v2 for this series, from other reviews, so please add this to this patch so that we know what is its intent in addition to the description of what it is doing. > Thanks, > Ian > > > - Arnaldo > > > > > Signed-off-by: Ian Rogers > > > --- > > > tools/perf/util/pmus.c | 48 ++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 48 insertions(+) > > > > > > diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c > > > index c58ba9fb6a36..3581710667b0 100644 > > > --- a/tools/perf/util/pmus.c > > > +++ b/tools/perf/util/pmus.c > > > @@ -1,8 +1,10 @@ > > > // SPDX-License-Identifier: GPL-2.0 > > > #include > > > +#include > > > #include > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -33,6 +35,31 @@ static LIST_HEAD(other_pmus); > > > static bool read_sysfs_core_pmus; > > > static bool read_sysfs_all_pmus; > > > > > > +static int pmu_name_len_no_suffix(const char *str, unsigned long *num) > > > +{ > > > + int orig_len, len; > > > + > > > + orig_len = len = strlen(str); > > > + > > > + /* Non-uncore PMUs have their full length, for example, i915. */ > > > + if (strncmp(str, "uncore_", 7)) > > > + return len; > > > + > > > + /* > > > + * Count trailing digits and '_', if '_{num}' suffix isn't present use > > > + * the full length. > > > + */ > > > + while (len > 0 && isdigit(str[len - 1])) > > > + len--; > > > + > > > + if (len > 0 && len != orig_len && str[len - 1] == '_') { > > > + if (num) > > > + *num = strtoul(&str[len], NULL, 10); > > > + return len - 1; > > > + } > > > + return orig_len; > > > +} > > > + > > > void perf_pmus__destroy(void) > > > { > > > struct perf_pmu *pmu, *tmp; > > > @@ -122,6 +149,25 @@ static struct perf_pmu *perf_pmu__find2(int dirfd, const char *name) > > > return perf_pmu__lookup(core_pmu ? &core_pmus : &other_pmus, dirfd, name); > > > } > > > > > > +static int pmus_cmp(void *priv __maybe_unused, > > > + const struct list_head *lhs, const struct list_head *rhs) > > > +{ > > > + unsigned long lhs_num, rhs_num; > > > + struct perf_pmu *lhs_pmu = container_of(lhs, struct perf_pmu, list); > > > + struct perf_pmu *rhs_pmu = container_of(rhs, struct perf_pmu, list); > > > + const char *lhs_pmu_name = lhs_pmu->name ?: ""; > > > + const char *rhs_pmu_name = rhs_pmu->name ?: ""; > > > + int lhs_pmu_name_len = pmu_name_len_no_suffix(lhs_pmu_name, &lhs_num); > > > + int rhs_pmu_name_len = pmu_name_len_no_suffix(rhs_pmu_name, &rhs_num); > > > + int ret = strncmp(lhs_pmu_name, rhs_pmu_name, > > > + lhs_pmu_name_len < rhs_pmu_name_len ? lhs_pmu_name_len : rhs_pmu_name_len); > > > + > > > + if (lhs_pmu_name_len != rhs_pmu_name_len || ret != 0 || lhs_pmu_name_len == 0) > > > + return ret; > > > + > > > + return lhs_num < rhs_num ? -1 : (lhs_num > rhs_num ? 1 : 0); > > > +} > > > + > > > /* Add all pmus in sysfs to pmu list: */ > > > static void pmu_read_sysfs(bool core_only) > > > { > > > @@ -156,6 +202,8 @@ static void pmu_read_sysfs(bool core_only) > > > if (!perf_pmu__create_placeholder_core_pmu(&core_pmus)) > > > pr_err("Failure to set up any core PMUs\n"); > > > } > > > + list_sort(NULL, &core_pmus, pmus_cmp); > > > + list_sort(NULL, &other_pmus, pmus_cmp); > > > if (!list_empty(&core_pmus)) { > > > read_sysfs_core_pmus = true; > > > if (!core_only) > > > -- > > > 2.41.0.640.ga95def55d0-goog > > > > > > > -- > > > > - Arnaldo -- - Arnaldo