[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20171122192659.GB2383@kernel.org>
Date: Wed, 22 Nov 2017 16:26:59 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: William Cohen <wcohen@...hat.com>
Cc: peterz@...radead.org, mingo@...hat.com, mpetlan@...hat.com,
alexander.shishkin@...ux.intel.com, jolsa@...hat.com,
namhyung@...nel.org,
Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com>,
sukadev@...ux.vnet.ibm.com, maddy@...ux.vnet.ibm.com,
shriyak@...ux.vnet.ibm.com,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Use more flexible pattern matching for CPU
identification for mapfile.csv
Em Wed, Nov 22, 2017 at 11:08:49AM -0500, William Cohen escreveu:
> The powerpc cpuid information includes chip revision information.
> Changes between chip revisions are usually minor bug fixes and usually
> do not affect the operation of the performance monitoring hardware.
> The original mapfile.csv matching requires enumerating every possible
> cpuid string. When a new minor chip revision is produced a new entry
> has to be added to the mapfile.csv and the code recompiled to allow
> perf to have the implementation specific perf events for this new
> minor revision. For users of various distibutions of Linux having to
> wait for a new release of the kernel's perf tool to be built with
> these trivial patches is inconvenient.
>
> Using regular expressions rather than exactly string matching of the
> entire cpuid string allows developers to write mapfile.csv files that
> do not require patches and recompiles for each of these minor version
> changes. If special cases need to be made for some particular
> versions, they can be placed earlier in the mapfile.csv file before
> the more general matches.
Looks ok, but you forgot to add lkml to the CC list, I'm doing that now
and also adding Ravi Bangoria @ IBM, which sent yet another of those
lines that would match your regexp.
Ravi & IBM crew, please see if we can proceed with Will's regexp approach, ok?
- Arnaldo
> Signed-off-by: William Cohen <wcohen@...hat.com>
> ---
> tools/perf/pmu-events/arch/powerpc/mapfile.csv | 14 +++------
> tools/perf/pmu-events/arch/x86/mapfile.csv | 5 +---
> tools/perf/pmu-events/jevents.c | 39 +++++++++++++++++++++++++-
> tools/perf/util/pmu.c | 20 ++++++++++++-
> 4 files changed, 62 insertions(+), 16 deletions(-)
>
> diff --git a/tools/perf/pmu-events/arch/powerpc/mapfile.csv b/tools/perf/pmu-events/arch/powerpc/mapfile.csv
> index a0f3a11ca19f..9ad0469057b8 100644
> --- a/tools/perf/pmu-events/arch/powerpc/mapfile.csv
> +++ b/tools/perf/pmu-events/arch/powerpc/mapfile.csv
> @@ -13,13 +13,7 @@
> #
>
> # Power8 entries
> -004b0000,1,power8,core
> -004b0201,1,power8,core
> -004c0000,1,power8,core
> -004d0000,1,power8,core
> -004d0100,1,power8,core
> -004d0200,1,power8,core
> -004c0100,1,power8,core
> -004e0100,1,power9,core
> -004e0200,1,power9,core
> -004e1200,1,power9,core
> +004b[[:xdigit:]]\+,1,power8,core
> +004c[[:xdigit:]]\+,1,power8,core
> +004d[[:xdigit:]]\+,1,power8,core
> +004e[[:xdigit:]]\+,1,power9,core
> diff --git a/tools/perf/pmu-events/arch/x86/mapfile.csv b/tools/perf/pmu-events/arch/x86/mapfile.csv
> index fe1a2c47cabf..93656f2fd53a 100644
> --- a/tools/perf/pmu-events/arch/x86/mapfile.csv
> +++ b/tools/perf/pmu-events/arch/x86/mapfile.csv
> @@ -23,10 +23,7 @@ GenuineIntel-6-1E,v2,nehalemep,core
> GenuineIntel-6-1F,v2,nehalemep,core
> GenuineIntel-6-1A,v2,nehalemep,core
> GenuineIntel-6-2E,v2,nehalemex,core
> -GenuineIntel-6-4E,v24,skylake,core
> -GenuineIntel-6-5E,v24,skylake,core
> -GenuineIntel-6-8E,v24,skylake,core
> -GenuineIntel-6-9E,v24,skylake,core
> +GenuineIntel-6-[4589]E,v24,skylake,core
> GenuineIntel-6-37,v13,silvermont,core
> GenuineIntel-6-4D,v13,silvermont,core
> GenuineIntel-6-4C,v13,silvermont,core
> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> index 9eb7047bafe4..4102664ac1bf 100644
> --- a/tools/perf/pmu-events/jevents.c
> +++ b/tools/perf/pmu-events/jevents.c
> @@ -116,6 +116,43 @@ static void fixdesc(char *s)
> *e = 0;
> }
>
> +/* Add escapes for '\' so they are proper C strings. */
> +static char *fixregex(char *s)
> +{
> + int len = 0;
> + int esc_count = 0;
> + char *fixed = NULL;
> + char *p, *q;
> +
> + /* Count the number of '\' in string */
> + for (p = s; *p; p++) {
> + ++len;
> + if (*p == '\\')
> + ++esc_count;
> + }
> +
> + if (esc_count == 0)
> + return s;
> +
> + /* allocate space for a new string */
> + fixed = (char *) malloc(len+1);
> + if (!fixed)
> + return NULL;
> +
> + /* copy over the characters */
> + q = fixed;
> + for (p = s; *p; p++) {
> + if (*p == '\\') {
> + *q = '\\';
> + ++q;
> + }
> + *q = *p;
> + ++q;
> + }
> + *q = '\0';
> + return fixed;
> +}
> +
> static struct msrmap {
> const char *num;
> const char *pname;
> @@ -648,7 +685,7 @@ static int process_mapfile(FILE *outfp, char *fpath)
> }
> line[strlen(line)-1] = '\0';
>
> - cpuid = strtok_r(p, ",", &save);
> + cpuid = fixregex(strtok_r(p, ",", &save));
> version = strtok_r(NULL, ",", &save);
> fname = strtok_r(NULL, ",", &save);
> type = strtok_r(NULL, ",", &save);
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 07cb2ac041d7..56942b748d87 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -12,6 +12,7 @@
> #include <dirent.h>
> #include <api/fs/fs.h>
> #include <locale.h>
> +#include <regex.h>
> #include "util.h"
> #include "pmu.h"
> #include "parse-events.h"
> @@ -570,14 +571,31 @@ struct pmu_events_map *perf_pmu__find_map(void)
>
> i = 0;
> for (;;) {
> + regex_t re;
> + regmatch_t pmatch[1];
> + int match;
> +
> map = &pmu_events_map[i++];
> if (!map->table) {
> map = NULL;
> break;
> }
>
> - if (!strcmp(map->cpuid, cpuid))
> + if (regcomp(&re, map->cpuid, 0) != 0) {
> + /* Warn unable to generate match particular string. */
> + pr_info("Invalid regular expression %s\n", map->cpuid);
> break;
> + }
> +
> + match = !regexec(&re, cpuid, 1, pmatch, REG_EXTENDED);
> + regfree(&re);
> + if (match) {
> + size_t match_len = (pmatch[0].rm_eo - pmatch[0].rm_so);
> +
> + /* Verify the entire string matched. */
> + if (match_len == strlen(cpuid))
> + break;
> + }
> }
> free(cpuid);
> return map;
> --
> 2.13.6
Powered by blists - more mailing lists