| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 25 Aug 2023 15:48:11 -0700
From: Ian Rogers <irogers@...gle.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Kan Liang <kan.liang@...ux.intel.com>,
Ravi Bangoria <ravi.bangoria@....com>,
Kajol Jain <kjain@...ux.ibm.com>,
John Garry <john.g.garry@...cle.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 1/2] perf pmus: Sort pmus by name then suffix
On Fri, Aug 25, 2023 at 1:23 PM Arnaldo Carvalho de Melo
<acme@...nel.org> wrote:
>
> Em Fri, Aug 25, 2023 at 06:52:36AM -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". Suffixes need to be treated specially as
> > otherwise they will be ordered like 0, 1, 10, 11, .., 2, 20, 21, ..,
> > etc. Only PMUs starting 'uncore_' are considered to have a potential
> > suffix.
> >
> > Sorting of PMUs is done so that later patches can skip duplicate
> > uncore PMUs that differ only by there suffix.
> >
> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> > 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 4dd5912617ff..b1f6a64693fe 100644
> > --- a/tools/perf/util/pmus.c
> > +++ b/tools/perf/util/pmus.c
> > @@ -1,8 +1,10 @@
> > // SPDX-License-Identifier: GPL-2.0
> > #include <linux/list.h>
> > +#include <linux/list_sort.h>
> > #include <linux/zalloc.h>
> > #include <subcmd/pager.h>
> > #include <sys/types.h>
> > +#include <ctype.h>
> > #include <dirent.h>
> > #include <pthread.h>
> > #include <string.h>
> > @@ -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);
> > +}
>
> Also, I got this in some distros/gcc versions:
>
> 31 12.95 opensuse:15.5 : FAIL gcc version 7.5.0 (SUSE Linux)
> util/pmus.c: In function 'pmus_cmp':
> util/pmus.c:169:57: error: 'lhs_num' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> return lhs_num < rhs_num ? -1 : (lhs_num > rhs_num ? 1 : 0);
> ~~~~~~~~~~~~~~~~~~~~~~~^~~~
> cc1: all warnings being treated as errors
> make[3]: *** [/git/perf-6.5.0-rc5/tools/build/Makefile.build:150: util] Error 2
>
> I applied this, please check.
Makes sense, thanks!
Ian
> diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> index bbf84ccc3aba7d5c..7316da1c0ddb8eaf 100644
> --- a/tools/perf/util/pmus.c
> +++ b/tools/perf/util/pmus.c
> @@ -153,7 +153,7 @@ static struct perf_pmu *perf_pmu__find2(int dirfd, const char *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;
> + unsigned long lhs_num = 0, rhs_num = 0;
> 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 ?: "";
Powered by blists - more mailing lists