From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751867AbdKVT1F (ORCPT ); Wed, 22 Nov 2017 14:27:05 -0500 Received: from mail.kernel.org ([198.145.29.99]:42504 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751273AbdKVT1C (ORCPT ); Wed, 22 Nov 2017 14:27:02 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 94C0F21924 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=acme@kernel.org Date: Wed, 22 Nov 2017 16:26:59 -0300 From: Arnaldo Carvalho de Melo To: William Cohen Cc: peterz@infradead.org, mingo@redhat.com, mpetlan@redhat.com, alexander.shishkin@linux.intel.com, jolsa@redhat.com, namhyung@kernel.org, Ravi Bangoria , sukadev@linux.vnet.ibm.com, maddy@linux.vnet.ibm.com, shriyak@linux.vnet.ibm.com, Linux Kernel Mailing List Subject: Re: [PATCH] Use more flexible pattern matching for CPU identification for mapfile.csv Message-ID: <20171122192659.GB2383@kernel.org> References: <20171122160849.8400-1-wcohen@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171122160849.8400-1-wcohen@redhat.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.9.1 (2017-09-22) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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 > #include > #include > +#include > #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