[GIT PULL 00/12] perf/urgent fixes

[GIT PULL 00/12] perf/urgent fixes

[Arnaldo Carvalho de Melo]

Hi Ingo,

	Please consider pulling,

- Arnaldo

Test results at the end of this message, as usual.

The following changes since commit c042f7e9bb6ad9429ea0f2c9138dc06413198967:

  Merge tag 'perf-urgent-for-mingo-4.17-20180420' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent (2018-04-21 09:38:33 +0200)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-urgent-for-mingo-4.17-20180425

for you to fetch changes up to 80ee8c588afde077cb0439e15129579a267916c4:

  perf stat: Fix duplicate PMU name for interval print (2018-04-24 16:12:00 -0300)

----------------------------------------------------------------
perf/urgent fixes:

perf stat:

- Keep the '/' event modifier separator in fallback, for example when
  fallbacking from 'cpu/cpu-cycles/' to user level only, where it should
  become 'cpu/cpu-cycles/u' and not 'cpu/cpu-cycles/:u' (Jiri Olsa)

- Fix PMU events parsing rule, improving error reporting for
  invalid events (Jiri Olsa)

- Disable write_backward and other event attributes for !group
  events in a group, fixing, for instance this group: '{cycles,msr/aperf/}:S'
  that has leader sampling (:S) and where just the 'cycles',
  the leader event, should have the write_backward attribute
  set, in this case it all fails because the PMU where 'msr/aperf/'
  lives doesn't accepts write_backward style sampling (Jiri Olsa)

- Only fall back group read for leader (Kan Liang)

- Fix core PMU alias list for X86 platform (Kan Liang)

- Print out hint for mixed PMU group error (Kan Liang)

- Fix duplicate PMU name for interval print (Kan Liang)

Core:

- Set main kernel end address properly when reading kernel and
  module maps (Namhyung Kim)

perf mem:

- Fix incorrect entries and add missing man options (Sangwon Hong)

s/390:

- Remove s390 specific strcmp_cpuid_cmp function (Thomas Richter)

- Adapt 'perf test' case record+probe_libc_inet_pton.sh for s390

- Fix s390 undefined record__auxtrace_init() return value in
  'perf record' (Thomas Richter)

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

----------------------------------------------------------------
Jiri Olsa (3):
      perf stat: Keep the / modifier separator in fallback
      perf pmu: Fix pmu events parsing rule
      perf evsel: Disable write_backward for leader sampling group events

Kan Liang (4):
      perf pmu: Fix core PMU alias list for X86 platform
      perf stat: Print out hint for mixed PMU group error
      perf evsel: Only fall back group read for leader
      perf stat: Fix duplicate PMU name for interval print

Namhyung Kim (1):
      perf machine: Set main kernel end address properly

Sangwon Hong (1):
      perf mem: Document incorrect and missing options

Thomas Richter (3):
      perf list: Remove s390 specific strcmp_cpuid_cmp function
      perf test: Adapt test case record+probe_libc_inet_pton.sh for s390
      perf record: Fix s390 undefined record__auxtrace_init() return value

 tools/perf/Documentation/perf-mem.txt              | 41 +++++++++++++++-------
 tools/perf/arch/s390/util/auxtrace.c               |  1 +
 tools/perf/arch/s390/util/header.c                 | 18 ----------
 tools/perf/builtin-stat.c                          | 40 +++++++++++++++++++--
 tools/perf/pmu-events/arch/s390/mapfile.csv        | 10 +++---
 tools/perf/tests/attr/test-record-group-sampling   |  3 ++
 .../tests/shell/record+probe_libc_inet_pton.sh     |  6 ++--
 tools/perf/util/evsel.c                            | 18 +++++++---
 tools/perf/util/evsel.h                            |  1 +
 tools/perf/util/machine.c                          | 30 +++++++++-------
 tools/perf/util/parse-events.y                     |  8 ++---
 tools/perf/util/pmu.c                              | 22 +++++-------
 12 files changed, 123 insertions(+), 75 deletions(-)

Test results:

The first ones are container (docker) based builds of tools/perf with
and without libelf support.  Where clang is available, it is also used
to build perf with/without libelf, and building with LIBCLANGLLVM=1
(built-in clang) with gcc and clang when clang and its devel libraries
are installed.

The objtool and samples/bpf/ builds are disabled now that I'm switching from
using the sources in a local volume to fetching them from a http server to
build it inside the container, to make it easier to build in a container cluster.
Those will come back later.

Several are cross builds, the ones with -x-ARCH and the android one, and those
may not have all the features built, due to lack of multi-arch devel packages,
available and being used so far on just a few, like
debian:experimental-x-{arm64,mipsel}.

The 'perf test' one will perform a variety of tests exercising
tools/perf/util/, tools/lib/{bpf,traceevent,etc}, as well as run perf commands
with a variety of command line event specifications to then intercept the
sys_perf_event syscall to check that the perf_event_attr fields are set up as
expected, among a variety of other unit tests.

Then there is the 'make -C tools/perf build-test' ones, that build tools/perf/
with a variety of feature sets, exercising the build with an incomplete set of
features as well as with a complete one. It is planned to have it run on each
of the containers mentioned above, using some container orchestration
infrastructure. Get in contact if interested in helping having this in place.

  # dm
   1 alpine:3.4                    : Ok   gcc (Alpine 5.3.0) 5.3.0
   2 alpine:3.5                    : Ok   gcc (Alpine 6.2.1) 6.2.1 20160822
   3 alpine:3.6                    : Ok   gcc (Alpine 6.3.0) 6.3.0
   4 alpine:3.7                    : Ok   gcc (Alpine 6.4.0) 6.4.0
   5 alpine:edge                   : Ok   gcc (Alpine 6.4.0) 6.4.0
   6 amazonlinux:1                 : Ok   gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-11)
   7 amazonlinux:2                 : Ok   gcc (GCC) 7.3.1 20180303 (Red Hat 7.3.1-5)
   8 android-ndk:r12b-arm          : Ok   arm-linux-androideabi-gcc (GCC) 4.9.x 20150123 (prerelease)
   9 android-ndk:r15c-arm          : Ok   arm-linux-androideabi-gcc (GCC) 4.9.x 20150123 (prerelease)
  10 centos:5                      : Ok   gcc (GCC) 4.1.2 20080704 (Red Hat 4.1.2-55)
  11 centos:6                      : Ok   gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-18)
  12 centos:7                      : Ok   gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-16)
  13 debian:7                      : Ok   gcc (Debian 4.7.2-5) 4.7.2
  14 debian:8                      : Ok   gcc (Debian 4.9.2-10+deb8u1) 4.9.2
  15 debian:9                      : Ok   gcc (Debian 6.3.0-18+deb9u1) 6.3.0 20170516
  16 debian:experimental           : Ok   gcc (Debian 7.3.0-15) 7.3.0
  17 debian:experimental-x-arm64   : Ok   aarch64-linux-gnu-gcc (Debian 7.3.0-15) 7.3.0
  18 debian:experimental-x-mips    : Ok   mips-linux-gnu-gcc (Debian 7.3.0-12) 7.3.0
  19 debian:experimental-x-mips64  : Ok   mips64-linux-gnuabi64-gcc (Debian 7.3.0-12) 7.3.0
  20 debian:experimental-x-mipsel  : Ok   mipsel-linux-gnu-gcc (Debian 7.3.0-12) 7.3.0
  21 fedora:20                     : Ok   gcc (GCC) 4.8.3 20140911 (Red Hat 4.8.3-7)
  22 fedora:21                     : Ok   gcc (GCC) 4.9.2 20150212 (Red Hat 4.9.2-6)
  23 fedora:22                     : Ok   gcc (GCC) 5.3.1 20160406 (Red Hat 5.3.1-6)
  24 fedora:23                     : Ok   gcc (GCC) 5.3.1 20160406 (Red Hat 5.3.1-6)
  25 fedora:24                     : Ok   gcc (GCC) 6.3.1 20161221 (Red Hat 6.3.1-1)
  26 fedora:24-x-ARC-uClibc        : Ok   arc-linux-gcc (ARCompact ISA Linux uClibc toolchain 2017.09-rc2) 7.1.1 20170710
  27 fedora:25                     : Ok   gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1)
  28 fedora:26                     : Ok   gcc (GCC) 7.3.1 20180130 (Red Hat 7.3.1-2)
  29 fedora:27                     : Ok   gcc (GCC) 7.3.1 20180303 (Red Hat 7.3.1-5)
  30 fedora:28                     : Ok   gcc (GCC) 8.0.1 20180324 (Red Hat 8.0.1-0.20)
  31 fedora:rawhide                : Ok   gcc (GCC) 8.0.1 20180324 (Red Hat 8.0.1-0.20)
  32 gentoo-stage3-amd64:latest    : Ok   gcc (Gentoo 6.4.0-r1 p1.3) 6.4.0
  33 mageia:5                      : Ok   gcc (GCC) 4.9.2
  34 mageia:6                      : Ok   gcc (Mageia 5.5.0-1.mga6) 5.5.0
  35 opensuse:42.1                 : Ok   gcc (SUSE Linux) 4.8.5
  36 opensuse:42.2                 : Ok   gcc (SUSE Linux) 4.8.5
  37 opensuse:42.3                 : Ok   gcc (SUSE Linux) 4.8.5
  38 opensuse:tumbleweed           : Ok   gcc (SUSE Linux) 7.3.1 20180323 [gcc-7-branch revision 258812]
  39 oraclelinux:6                 : Ok   gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-18.0.7)
  40 oraclelinux:7                 : Ok   gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-28.0.1)
  41 ubuntu:12.04.5                : Ok   gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3
  42 ubuntu:14.04.4                : Ok   gcc (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4
  43 ubuntu:14.04.4-x-linaro-arm64 : Ok   aarch64-linux-gnu-gcc (Linaro GCC 5.5-2017.10) 5.5.0
  44 ubuntu:16.04                  : Ok   gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
  45 ubuntu:16.04-x-arm            : Ok   arm-linux-gnueabihf-gcc (Ubuntu/Linaro 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
  46 ubuntu:16.04-x-arm64          : Ok   aarch64-linux-gnu-gcc (Ubuntu/Linaro 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
  47 ubuntu:16.04-x-powerpc        : Ok   powerpc-linux-gnu-gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
  48 ubuntu:16.04-x-powerpc64      : Ok   powerpc64-linux-gnu-gcc (Ubuntu/IBM 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
  49 ubuntu:16.04-x-powerpc64el    : Ok   powerpc64le-linux-gnu-gcc (Ubuntu/IBM 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
  50 ubuntu:16.04-x-s390           : Ok   s390x-linux-gnu-gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
  51 ubuntu:16.10                  : Ok   gcc (Ubuntu 6.2.0-5ubuntu12) 6.2.0 20161005
  52 ubuntu:17.04                  : Ok   gcc (Ubuntu 6.3.0-12ubuntu2) 6.3.0 20170406
  53 ubuntu:17.10                  : Ok   gcc (Ubuntu 7.2.0-8ubuntu3.2) 7.2.0
  54 ubuntu:18.04                  : Ok   gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0

  # uname -a
  Linux jouet 4.17.0-rc1-00024-g7e4e440a91c8-dirty #16 SMP Wed Apr 18 11:21:02 -03 2018 x86_64 x86_64 x86_64 GNU/Linux
  # perf test
   1: vmlinux symtab matches kallsyms                       : Ok
   2: Detect openat syscall event                           : Ok
   3: Detect openat syscall event on all cpus               : Ok
   4: Read samples using the mmap interface                 : Ok
   5: Test data source output                               : Ok
   6: Parse event definition strings                        : Ok
   7: Simple expression parser                              : Ok
   8: PERF_RECORD_* events & perf_sample fields             : Ok
   9: Parse perf pmu format                                 : Ok
  10: DSO data read                                         : Ok
  11: DSO data cache                                        : Ok
  12: DSO data reopen                                       : Ok
  13: Roundtrip evsel->name                                 : Ok
  14: Parse sched tracepoints fields                        : Ok
  15: syscalls:sys_enter_openat event fields                : Ok
  16: Setup struct perf_event_attr                          : Ok
  17: Match and link multiple hists                         : Ok
  18: 'import perf' in python                               : Ok
  19: Breakpoint overflow signal handler                    : Ok
  20: Breakpoint overflow sampling                          : Ok
  21: Breakpoint accounting                                 : Ok
  22: Number of exit events of a simple workload            : Ok
  23: Software clock events period values                   : Ok
  24: Object code reading                                   : Ok
  25: Sample parsing                                        : Ok
  26: Use a dummy software event to keep tracking           : Ok
  27: Parse with no sample_id_all bit set                   : Ok
  28: Filter hist entries                                   : Ok
  29: Lookup mmap thread                                    : Ok
  30: Share thread mg                                       : Ok
  31: Sort output of hist entries                           : Ok
  32: Cumulate child hist entries                           : Ok
  33: Track with sched_switch                               : Ok
  34: Filter fds with revents mask in a fdarray             : Ok
  35: Add fd to a fdarray, making it autogrow               : Ok
  36: kmod_path__parse                                      : Ok
  37: Thread map                                            : Ok
  38: LLVM search and compile                               :
  38.1: Basic BPF llvm compile                              : Ok
  38.2: kbuild searching                                    : Ok
  38.3: Compile source for BPF prologue generation          : Ok
  38.4: Compile source for BPF relocation                   : Ok
  39: Session topology                                      : Ok
  40: BPF filter                                            :
  40.1: Basic BPF filtering                                 : Ok
  40.2: BPF pinning                                         : Ok
  40.3: BPF prologue generation                             : Ok
  40.4: BPF relocation checker                              : Ok
  41: Synthesize thread map                                 : Ok
  42: Remove thread map                                     : Ok
  43: Synthesize cpu map                                    : Ok
  44: Synthesize stat config                                : Ok
  45: Synthesize stat                                       : Ok
  46: Synthesize stat round                                 : Ok
  47: Synthesize attr update                                : Ok
  48: Event times                                           : Ok
  49: Read backward ring buffer                             : Ok
  50: Print cpu map                                         : Ok
  51: Probe SDT events                                      : Ok
  52: is_printable_array                                    : Ok
  53: Print bitmap                                          : Ok
  54: perf hooks                                            : Ok
  55: builtin clang support                                 : Skip (not compiled in)
  56: unit_number__scnprintf                                : Ok
  57: mem2node                                              : Ok
  58: x86 rdpmc                                             : Ok
  59: Convert perf time to TSC                              : Ok
  60: DWARF unwind                                          : Ok
  61: x86 instruction decoder - new instructions            : Ok
  62: Use vfs_getname probe to get syscall args filenames   : Ok
  63: Check open filename arg using perf trace + vfs_getname: Ok
  64: probe libc's inet_pton & backtrace it with ping       : Ok
  65: Add vfs_getname probe to get syscall args filenames   : Ok
  #

  $ make -C tools/perf build-test
  make: Entering directory '/home/acme/git/perf/tools/perf'
  - tarpkg: ./tests/perf-targz-src-pkg .
                  make_no_ui_O: make NO_NEWT=1 NO_SLANG=1 NO_GTK2=1
         make_with_clangllvm_O: make LIBCLANGLLVM=1
                make_no_newt_O: make NO_NEWT=1
             make_no_scripts_O: make NO_LIBPYTHON=1 NO_LIBPERL=1
                   make_help_O: make help
              make_no_libelf_O: make NO_LIBELF=1
             make_no_libnuma_O: make NO_LIBNUMA=1
        make_with_babeltrace_O: make LIBBABELTRACE=1
   make_install_prefix_slash_O: make install prefix=/tmp/krava/
             make_util_map_o_O: make util/map.o
            make_no_libaudit_O: make NO_LIBAUDIT=1
       make_util_pmu_bison_o_O: make util/pmu-bison.o
              make_no_libbpf_O: make NO_LIBBPF=1
                make_install_O: make install
               make_no_slang_O: make NO_SLANG=1
                   make_tags_O: make tags
           make_no_libbionic_O: make NO_LIBBIONIC=1
                  make_debug_O: make DEBUG=1
              make_clean_all_O: make clean all
  make_no_libdw_dwarf_unwind_O: make NO_LIBDW_DWARF_UNWIND=1
             make_no_libperl_O: make NO_LIBPERL=1
         make_install_prefix_O: make install prefix=/tmp/krava
            make_install_bin_O: make install-bin
            make_no_auxtrace_O: make NO_AUXTRACE=1
                    make_doc_O: make doc
                make_minimal_O: make NO_LIBPERL=1 NO_LIBPYTHON=1 NO_NEWT=1 NO_GTK2=1 NO_DEMANGLE=1 NO_LIBELF=1 NO_LIBUNWIND=1 NO_BACKTRACE=1 NO_LIBNUMA=1 NO_LIBAUDIT=1 NO_LIBBIONIC=1 NO_LIBDW_DWARF_UNWIND=1 NO_AUXTRACE=1 NO_LIBBPF=1 NO_LIBCRYPTO=1 NO_SDT=1 NO_JVMTI=1
                 make_perf_o_O: make perf.o
                   make_pure_O: make
           make_no_backtrace_O: make NO_BACKTRACE=1
                 make_static_O: make LDFLAGS=-static
            make_no_demangle_O: make NO_DEMANGLE=1
           make_no_libpython_O: make NO_LIBPYTHON=1
                make_no_gtk2_O: make NO_GTK2=1
           make_no_libunwind_O: make NO_LIBUNWIND=1
  OK
  make: Leaving directory '/home/acme/git/perf/tools/perf'
  $ 

[PATCH 01/12] perf machine: Set main kernel end address properly

[Arnaldo Carvalho de Melo]

From: Namhyung Kim <namhyung@kernel.org>

map_groups__fixup_end() was called to set the end addresses of kernel
and module maps.  But now since machine__create_modules() sets the end
address of modules properly, the only remaining piece is the kernel map.

We can set it with adjacent module's address directly instead of calling
map_groups__fixup_end().  If there's no module after the kernel map, the
end address will be ~0ULL.

Since it also changes the start address of the kernel map, it needs to
re-insert the map to the kmaps in order to keep a correct ordering.  Kim
reported that it caused problems on ARM64.

Reported-by: Kim Phillips <kim.phillips@arm.com>
Tested-by: Kim Phillips <kim.phillips@arm.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: kernel-team@lge.com
Link: http://lkml.kernel.org/r/20180419235915.GA19067@sejong
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/machine.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 2eca8478e24f..32d50492505d 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1019,13 +1019,6 @@ int machine__load_vmlinux_path(struct machine *machine, enum map_type type)
 	return ret;
 }
 
-static void map_groups__fixup_end(struct map_groups *mg)
-{
-	int i;
-	for (i = 0; i < MAP__NR_TYPES; ++i)
-		__map_groups__fixup_end(mg, i);
-}
-
 static char *get_kernel_version(const char *root_dir)
 {
 	char version[PATH_MAX];
@@ -1233,6 +1226,7 @@ int machine__create_kernel_maps(struct machine *machine)
 {
 	struct dso *kernel = machine__get_kernel(machine);
 	const char *name = NULL;
+	struct map *map;
 	u64 addr = 0;
 	int ret;
 
@@ -1259,13 +1253,25 @@ int machine__create_kernel_maps(struct machine *machine)
 			machine__destroy_kernel_maps(machine);
 			return -1;
 		}
-		machine__set_kernel_mmap(machine, addr, 0);
+
+		/* we have a real start address now, so re-order the kmaps */
+		map = machine__kernel_map(machine);
+
+		map__get(map);
+		map_groups__remove(&machine->kmaps, map);
+
+		/* assume it's the last in the kmaps */
+		machine__set_kernel_mmap(machine, addr, ~0ULL);
+
+		map_groups__insert(&machine->kmaps, map);
+		map__put(map);
 	}
 
-	/*
-	 * Now that we have all the maps created, just set the ->end of them:
-	 */
-	map_groups__fixup_end(&machine->kmaps);
+	/* update end address of the kernel map using adjacent module address */
+	map = map__next(machine__kernel_map(machine));
+	if (map)
+		machine__set_kernel_mmap(machine, addr, map->start);
+
 	return 0;
 }
 
-- 
2.14.3

[PATCH 02/12] perf list: Remove s390 specific strcmp_cpuid_cmp function

[Arnaldo Carvalho de Melo]

From: Thomas Richter <tmricht@linux.ibm.com>

Make the type field in pmu-events/arch/s390/mapfile.cvs more generic to
match the created cpuid string for s390.

The pattern also checks for the counter first version number and counter
second version number ([13]\.[1-5]) and the authorization field which
follows.

These numbers do not exist in the cpuid identification string when perf
commands are executed on a z/VM environment (which does not support CPU
counter measurement facility).

CPUID string for LPAR:
   cpuid : IBM,3906,704,M03,3.5,002f
CPUID string for z/VM:
   cpuid : IBM,2964,702,N96

This allows the removal of s390 specific cpuid compare code and uses the
common compare function with its regular expression matching algorithm.

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Reviewed-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Link: http://lkml.kernel.org/r/20180423081745.3672-1-tmricht@linux.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/s390/util/header.c          | 18 ------------------
 tools/perf/pmu-events/arch/s390/mapfile.csv | 10 +++++-----
 tools/perf/util/pmu.c                       |  2 +-
 3 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/tools/perf/arch/s390/util/header.c b/tools/perf/arch/s390/util/header.c
index a4c30f1c70be..163b92f33998 100644
--- a/tools/perf/arch/s390/util/header.c
+++ b/tools/perf/arch/s390/util/header.c
@@ -146,21 +146,3 @@ char *get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
 		zfree(&buf);
 	return buf;
 }
-
-/*
- * Compare the cpuid string returned by get_cpuid() function
- * with the name generated by the jevents file read from
- * pmu-events/arch/s390/mapfile.csv.
- *
- * Parameter mapcpuid is the cpuid as stored in the
- * pmu-events/arch/s390/mapfile.csv. This is just the type number.
- * Parameter cpuid is the cpuid returned by function get_cpuid().
- */
-int strcmp_cpuid_str(const char *mapcpuid, const char *cpuid)
-{
-	char *cp = strchr(cpuid, ',');
-
-	if (cp == NULL)
-		return -1;
-	return strncmp(cp + 1, mapcpuid, strlen(mapcpuid));
-}
diff --git a/tools/perf/pmu-events/arch/s390/mapfile.csv b/tools/perf/pmu-events/arch/s390/mapfile.csv
index ca7682748a4b..78bcf7f8e206 100644
--- a/tools/perf/pmu-events/arch/s390/mapfile.csv
+++ b/tools/perf/pmu-events/arch/s390/mapfile.csv
@@ -1,6 +1,6 @@
 Family-model,Version,Filename,EventType
-209[78],1,cf_z10,core
-281[78],1,cf_z196,core
-282[78],1,cf_zec12,core
-296[45],1,cf_z13,core
-3906,3,cf_z14,core
+^IBM.209[78].*[13]\.[1-5].[[:xdigit:]]+$,1,cf_z10,core
+^IBM.281[78].*[13]\.[1-5].[[:xdigit:]]+$,1,cf_z196,core
+^IBM.282[78].*[13]\.[1-5].[[:xdigit:]]+$,1,cf_zec12,core
+^IBM.296[45].*[13]\.[1-5].[[:xdigit:]]+$,1,cf_z13,core
+^IBM.390[67].*[13]\.[1-5].[[:xdigit:]]+$,3,cf_z14,core
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 61a5e5027338..af4bedf4cf98 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -586,7 +586,7 @@ char * __weak get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
  * cpuid string generated on this platform.
  * Otherwise return non-zero.
  */
-int __weak strcmp_cpuid_str(const char *mapcpuid, const char *cpuid)
+int strcmp_cpuid_str(const char *mapcpuid, const char *cpuid)
 {
 	regex_t re;
 	regmatch_t pmatch[1];
-- 
2.14.3

[PATCH 03/12] perf test: Adapt test case record+probe_libc_inet_pton.sh for s390

[Arnaldo Carvalho de Melo]

From: Thomas Richter <tmricht@linux.ibm.com>

perf test case 58 (record+probe_libc_inet_pton.sh) executed on s390x
using kernel 4.16.0rc3 displays this result:

 # perf trace --no-syscalls -e probe_libc:inet_pton/call-graph=dwarf/ ping -6 -c 1 ::1
     probe_libc:inet_pton: (3ffa0240448)
	      __GI___inet_pton (/usr/lib64/libc-2.26.so)
	      gaih_inet (inlined)
	      __GI_getaddrinfo (inlined)
	      main (/usr/bin/ping)
	      __libc_start_main (/usr/lib64/libc-2.26.so)
	     _start (/usr/bin/ping)

After I installed kernel 4.16.0 the same tests uses commands:

 # perf record -e probe_libc:inet_pton/call-graph=dwarf/
      -o /tmp/perf.data.abc ping -6 -c 1 ::1
 # perf script -i /tmp/perf.data.abc

and displays:

 ping 39048 [006] 84230.381198: probe_libc:inet_pton: (3ffa0240448)
	       140448 __GI___inet_pton (/usr/lib64/libc-2.26.so)
	       fbde1 gaih_inet (inlined)
	       fe2b9 __GI_getaddrinfo (inlined)
	        398d main (/usr/bin/ping)

Nothing else changed including glibc elfutils and other libraries picked
up by the build.

The entries for __libc_start_main and _start are missing.

I bisected missing __libc_start_main and _start to commit

Fixes: 3d20c6246690 ("perf unwind: Unwind with libdw doesn't take symfs into account")

When I undo this commit I get this call stack on s390:
 [root@s35lp76 perf]# ./perf script  -i /tmp/perf.data.abc
 ping 39048 [006] 84230.381198: probe_libc:inet_pton: (3ffa0240448)
	140448 __GI___inet_pton (/usr/lib64/libc-2.26.so)
	 fbde1 gaih_inet (inlined)
	 fe2b9 __GI_getaddrinfo (inlined)
	  398d main (/usr/bin/ping)
	 22fbd __libc_start_main (/usr/lib64/libc-2.26.so)
	  457b _start (/usr/bin/ping)

Looks like dwarf functions dwfl_xxx create different call back stack
trace when using file /usr/lib/debug/usr/bin/ping-20161105-7.fc27.s390x.debug
instead of file /usr/bin/ping.

Fix this test case on s390 and do not expect any call back stack entry
after the main() function. Also be more robust and accept a leading
__GI_ prefix in front of getaddrinfo.

On x86 this test case shows the same call stack using both kernel
versions 4.16.0rc3 and 4.16.0 and also stops at main:

  [root@f27 perf]# ./perf script -i /tmp/perf.data.tmr
  ping  4446 [000]   172.027088: probe_libc:inet_pton: (7fdfa08c93c0)
	             1393c0 __GI___inet_pton (/usr/lib64/libc-2.26.so)
	              fe60d getaddrinfo (/usr/lib64/libc-2.26.so)
	               2f40 main (/usr/bin/ping)
  [root@f27 perf]#

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Reviewed-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Martin Vuille <jpmv27@aim.com>
Link: http://lkml.kernel.org/r/20180423082428.7930-1-tmricht@linux.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/shell/record+probe_libc_inet_pton.sh | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/perf/tests/shell/record+probe_libc_inet_pton.sh b/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
index 1ecc1f0ff84a..016882dbbc16 100755
--- a/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
+++ b/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
@@ -19,12 +19,10 @@ trace_libc_inet_pton_backtrace() {
 	expected[1]=".*inet_pton[[:space:]]\($libc\)$"
 	case "$(uname -m)" in
 	s390x)
-		eventattr='call-graph=dwarf'
+		eventattr='call-graph=dwarf,max-stack=4'
 		expected[2]="gaih_inet.*[[:space:]]\($libc|inlined\)$"
-		expected[3]="__GI_getaddrinfo[[:space:]]\($libc|inlined\)$"
+		expected[3]="(__GI_)?getaddrinfo[[:space:]]\($libc|inlined\)$"
 		expected[4]="main[[:space:]]\(.*/bin/ping.*\)$"
-		expected[5]="__libc_start_main[[:space:]]\($libc\)$"
-		expected[6]="_start[[:space:]]\(.*/bin/ping.*\)$"
 		;;
 	*)
 		eventattr='max-stack=3'
-- 
2.14.3

[PATCH 04/12] perf stat: Keep the / modifier separator in fallback

[Arnaldo Carvalho de Melo]

From: Jiri Olsa <jolsa@kernel.org>

The 'perf stat' fallback for EACCES error sets the exclude_kernel
perf_event_attr and tries perf_event_open() again with it. In addition,
it also changes the name of the event to reflect that change by adding
the 'u' modifier.

But it does not take into account the '/' separator, so the event name
can end up mangled, like: (note the '/:' characters)

  $ perf stat -e cpu/cpu-cycles/ kill
  ...
             386,832      cpu/cpu-cycles/:u

Adding the code to check on the '/' separator and set the following
correct event name:

  $ perf stat -e cpu/cpu-cycles/ kill
  ...
             388,548      cpu/cpu-cycles/u

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20180423090823.32309-4-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evsel.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 3e87486c28fe..7eb1e9850abf 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2754,8 +2754,14 @@ bool perf_evsel__fallback(struct perf_evsel *evsel, int err,
 		   (paranoid = perf_event_paranoid()) > 1) {
 		const char *name = perf_evsel__name(evsel);
 		char *new_name;
+		const char *sep = ":";
 
-		if (asprintf(&new_name, "%s%su", name, strchr(name, ':') ? "" : ":") < 0)
+		/* Is there already the separator in the name. */
+		if (strchr(name, '/') ||
+		    strchr(name, ':'))
+			sep = "";
+
+		if (asprintf(&new_name, "%s%su", name, sep) < 0)
 			return false;
 
 		if (evsel->name)
-- 
2.14.3

[PATCH 05/12] perf pmu: Fix pmu events parsing rule

[Arnaldo Carvalho de Melo]

From: Jiri Olsa <jolsa@kernel.org>

Currently all the event parsing fails end up in the event_pmu rule, and
display misleading help like:

  $ perf stat -e inst kill
  event syntax error: 'inst'
                       \___ Cannot find PMU `inst'. Missing kernel support?
  ...

The reason is that the event_pmu is too strong and match also single
string. Changing it to force the '/' separators to be part of the rule,
and getting the proper error now:

  $ perf stat -e inst kill
  event syntax error: 'inst'
                       \___ parser error
  Run 'perf list' for a list of valid events
  ...

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Reported-by: Ingo Molnar <mingo@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20180423090823.32309-5-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.y | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 7afeb80cc39e..d14464c42714 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -224,15 +224,15 @@ event_def: event_pmu |
 	   event_bpf_file
 
 event_pmu:
-PE_NAME opt_event_config
+PE_NAME '/' event_config '/'
 {
 	struct list_head *list, *orig_terms, *terms;
 
-	if (parse_events_copy_term_list($2, &orig_terms))
+	if (parse_events_copy_term_list($3, &orig_terms))
 		YYABORT;
 
 	ALLOC_LIST(list);
-	if (parse_events_add_pmu(_parse_state, list, $1, $2, false)) {
+	if (parse_events_add_pmu(_parse_state, list, $1, $3, false)) {
 		struct perf_pmu *pmu = NULL;
 		int ok = 0;
 		char *pattern;
@@ -262,7 +262,7 @@ PE_NAME opt_event_config
 		if (!ok)
 			YYABORT;
 	}
-	parse_events_terms__delete($2);
+	parse_events_terms__delete($3);
 	parse_events_terms__delete(orig_terms);
 	$$ = list;
 }
-- 
2.14.3

[PATCH 06/12] perf evsel: Disable write_backward for leader sampling group events

[Arnaldo Carvalho de Melo]

From: Jiri Olsa <jolsa@kernel.org>

.. and other related fields that do not need to be enabled
for events that have sampling leader.

It fixes the perf top usage Ingo reported broken:

  # perf top -e '{cycles,msr/aperf/}:S'

The 'msr/aperf/' event is configured for write_back sampling, which is
not allowed by the MSR PMU, so it fails to create the event.

Adjusting related attr test.

Reported-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20180423090823.32309-6-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/attr/test-record-group-sampling | 3 +++
 tools/perf/util/evsel.c                          | 7 +++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/perf/tests/attr/test-record-group-sampling b/tools/perf/tests/attr/test-record-group-sampling
index f906b793196f..8a33ca4f9e1f 100644
--- a/tools/perf/tests/attr/test-record-group-sampling
+++ b/tools/perf/tests/attr/test-record-group-sampling
@@ -35,3 +35,6 @@ inherit=0
 # sampling disabled
 sample_freq=0
 sample_period=0
+freq=0
+write_backward=0
+sample_id_all=0
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 7eb1e9850abf..26bdeecc0452 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -930,8 +930,11 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts,
 	 * than leader in case leader 'leads' the sampling.
 	 */
 	if ((leader != evsel) && leader->sample_read) {
-		attr->sample_freq   = 0;
-		attr->sample_period = 0;
+		attr->freq           = 0;
+		attr->sample_freq    = 0;
+		attr->sample_period  = 0;
+		attr->write_backward = 0;
+		attr->sample_id_all  = 0;
 	}
 
 	if (opts->no_samples)
-- 
2.14.3

[PATCH 07/12] perf mem: Document incorrect and missing options

[Arnaldo Carvalho de Melo]

From: Sangwon Hong <qpakzk@gmail.com>

Several options were incorrectly described, some lacked describing
required arguments while others were simply not documented, fix it.

Signed-off-by: Sangwon Hong <qpakzk@gmail.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Taeung Song <treeze.taeung@gmail.com>
Link: http://lkml.kernel.org/r/1524382146-19609-1-git-send-email-qpakzk@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-mem.txt | 41 +++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/tools/perf/Documentation/perf-mem.txt b/tools/perf/Documentation/perf-mem.txt
index 8806ed5f3802..f8d2167cf3e7 100644
--- a/tools/perf/Documentation/perf-mem.txt
+++ b/tools/perf/Documentation/perf-mem.txt
@@ -28,29 +28,46 @@ OPTIONS
 <command>...::
 	Any command you can specify in a shell.
 
+-i::
+--input=<file>::
+	Input file name.
+
 -f::
 --force::
 	Don't do ownership validation
 
 -t::
---type=::
+--type=<type>::
 	Select the memory operation type: load or store (default: load,store)
 
 -D::
---dump-raw-samples=::
+--dump-raw-samples::
 	Dump the raw decoded samples on the screen in a format that is easy to parse with
 	one sample per line.
 
 -x::
---field-separator::
+--field-separator=<separator>::
 	Specify the field separator used when dump raw samples (-D option). By default,
 	The separator is the space character.
 
 -C::
---cpu-list::
-	Restrict dump of raw samples to those provided via this option. Note that the same
-	option can be passed in record mode. It will be interpreted the same way as perf
-	record.
+--cpu=<cpu>::
+	Monitor only on the list of CPUs provided. Multiple CPUs can be provided as a
+        comma-separated list with no space: 0,1. Ranges of CPUs are specified with -: 0-2. Default
+        is to monitor all CPUS.
+-U::
+--hide-unresolved::
+	Only display entries resolved to a symbol.
+
+-p::
+--phys-data::
+	Record/Report sample physical addresses
+
+RECORD OPTIONS
+--------------
+-e::
+--event <event>::
+	Event selector. Use 'perf mem record -e list' to list available events.
 
 -K::
 --all-kernel::
@@ -60,12 +77,12 @@ OPTIONS
 --all-user::
 	Configure all used events to run in user space.
 
---ldload::
-	Specify desired latency for loads event.
+-v::
+--verbose::
+	Be more verbose (show counter open errors, etc)
 
--p::
---phys-data::
-	Record/Report sample physical addresses
+--ldlat <n>::
+	Specify desired latency for loads event.
 
 In addition, for report all perf report options are valid, and for record
 all perf record options.
-- 
2.14.3

[PATCH 08/12] perf record: Fix s390 undefined record__auxtrace_init() return value

[Arnaldo Carvalho de Melo]

From: Thomas Richter <tmricht@linux.ibm.com>

Command 'perf record' calls:

  cmd_report()
    record__auxtrace_init()
       auxtrace_record__init()

On s390 function auxtrace_record__init() returns random return value due
to missing initialization.

This sometime causes 'perf record' to exit immediately without error
message and creating a perf.data file.

Fix this by setting error the return code to zero before returning from
platform specific functions which may not set the error code in call
cases.

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Link: http://lkml.kernel.org/r/20180423142940.21143-1-tmricht@linux.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/s390/util/auxtrace.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/arch/s390/util/auxtrace.c b/tools/perf/arch/s390/util/auxtrace.c
index 6cb48e4cffd9..3afe8256eff2 100644
--- a/tools/perf/arch/s390/util/auxtrace.c
+++ b/tools/perf/arch/s390/util/auxtrace.c
@@ -87,6 +87,7 @@ struct auxtrace_record *auxtrace_record__init(struct perf_evlist *evlist,
 	struct perf_evsel *pos;
 	int diagnose = 0;
 
+	*err = 0;
 	if (evlist->nr_entries == 0)
 		return NULL;
 
-- 
2.14.3

[PATCH 09/12] perf pmu: Fix core PMU alias list for X86 platform

[Arnaldo Carvalho de Melo]

From: Kan Liang <kan.liang@linux.intel.com>

When counting uncore event with alias, core event is mistakenly
involved, for example:

  perf stat --no-merge -e "unc_m_cas_count.all" -C0  sleep 1

  Performance counter stats for 'CPU(s) 0':

                 0      unc_m_cas_count.all [uncore_imc_4]
                 0      unc_m_cas_count.all [uncore_imc_2]
                 0      unc_m_cas_count.all [uncore_imc_0]
           153,640      unc_m_cas_count.all [cpu]
                 0      unc_m_cas_count.all [uncore_imc_5]
            25,026      unc_m_cas_count.all [uncore_imc_3]
                 0      unc_m_cas_count.all [uncore_imc_1]

       1.001447890 seconds time elapsed

The reason is that current implementation doesn't check PMU name of a
event when adding its alias into the alias list for core PMU. The
uncore event aliases are mistakenly added.

This bug was introduced in:
  commit 14b22ae028de ("perf pmu: Add helper function is_pmu_core to
  detect PMU CORE devices")

Checking the PMU name for all PMUs on X86 and other architectures except
ARM.
There is no behavior change for ARM.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Agustin Vega-Frias <agustinv@codeaurora.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Shaokun Zhang <zhangshaokun@hisilicon.com>
Cc: Will Deacon <will.deacon@arm.com>
Fixes: 14b22ae028de ("perf pmu: Add helper function is_pmu_core to detect PMU CORE devices")
Link: http://lkml.kernel.org/r/1524594014-79243-1-git-send-email-kan.liang@linux.intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/pmu.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index af4bedf4cf98..d2fb597c9a8c 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -539,9 +539,10 @@ static bool pmu_is_uncore(const char *name)
 
 /*
  *  PMU CORE devices have different name other than cpu in sysfs on some
- *  platforms. looking for possible sysfs files to identify as core device.
+ *  platforms.
+ *  Looking for possible sysfs files to identify the arm core device.
  */
-static int is_pmu_core(const char *name)
+static int is_arm_pmu_core(const char *name)
 {
 	struct stat st;
 	char path[PATH_MAX];
@@ -550,12 +551,6 @@ static int is_pmu_core(const char *name)
 	if (!sysfs)
 		return 0;
 
-	/* Look for cpu sysfs (x86 and others) */
-	scnprintf(path, PATH_MAX, "%s/bus/event_source/devices/cpu", sysfs);
-	if ((stat(path, &st) == 0) &&
-			(strncmp(name, "cpu", strlen("cpu")) == 0))
-		return 1;
-
 	/* Look for cpu sysfs (specific to arm) */
 	scnprintf(path, PATH_MAX, "%s/bus/event_source/devices/%s/cpus",
 				sysfs, name);
@@ -668,6 +663,7 @@ static void pmu_add_cpu_aliases(struct list_head *head, struct perf_pmu *pmu)
 	struct pmu_events_map *map;
 	struct pmu_event *pe;
 	const char *name = pmu->name;
+	const char *pname;
 
 	map = perf_pmu__find_map(pmu);
 	if (!map)
@@ -686,11 +682,9 @@ static void pmu_add_cpu_aliases(struct list_head *head, struct perf_pmu *pmu)
 			break;
 		}
 
-		if (!is_pmu_core(name)) {
-			/* check for uncore devices */
-			if (pe->pmu == NULL)
-				continue;
-			if (strncmp(pe->pmu, name, strlen(pe->pmu)))
+		if (!is_arm_pmu_core(name)) {
+			pname = pe->pmu ? pe->pmu : "cpu";
+			if (strncmp(pname, name, strlen(pname)))
 				continue;
 		}
 
-- 
2.14.3

[PATCH 10/12] perf stat: Print out hint for mixed PMU group error

[Arnaldo Carvalho de Melo]

From: Kan Liang <kan.liang@linux.intel.com>

Perf doesn't support mixed events from different PMUs (except software
event) in a group. For this case, only "<not counted>" or "<not
supported>" are printed out. There is no hint which guides users to fix
the issue.

Checking the PMU type of events to determine if they are from the same
PMU. There may be false alarm for the checking. E.g. the core PMU has
different PMU type. But it should not happen often.

The false alarm can also be tolerated, because:

- It only happens on error path.
- It just provides a possible solution for the issue.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: Agustin Vega-Frias <agustinv@codeaurora.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Shaokun Zhang <zhangshaokun@hisilicon.com>
Cc: Will Deacon <will.deacon@arm.com>
Link: http://lkml.kernel.org/r/1524594014-79243-2-git-send-email-kan.liang@linux.intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-stat.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 147a27e8c937..30e6b374e095 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -172,6 +172,7 @@ static bool			interval_count;
 static const char		*output_name;
 static int			output_fd;
 static int			print_free_counters_hint;
+static int			print_mixed_hw_group_error;
 
 struct perf_stat {
 	bool			 record;
@@ -1126,6 +1127,30 @@ static void abs_printout(int id, int nr, struct perf_evsel *evsel, double avg)
 		fprintf(output, "%s%s", csv_sep, evsel->cgrp->name);
 }
 
+static bool is_mixed_hw_group(struct perf_evsel *counter)
+{
+	struct perf_evlist *evlist = counter->evlist;
+	u32 pmu_type = counter->attr.type;
+	struct perf_evsel *pos;
+
+	if (counter->nr_members < 2)
+		return false;
+
+	evlist__for_each_entry(evlist, pos) {
+		/* software events can be part of any hardware group */
+		if (pos->attr.type == PERF_TYPE_SOFTWARE)
+			continue;
+		if (pmu_type == PERF_TYPE_SOFTWARE) {
+			pmu_type = pos->attr.type;
+			continue;
+		}
+		if (pmu_type != pos->attr.type)
+			return true;
+	}
+
+	return false;
+}
+
 static void printout(int id, int nr, struct perf_evsel *counter, double uval,
 		     char *prefix, u64 run, u64 ena, double noise,
 		     struct runtime_stat *st)
@@ -1178,8 +1203,11 @@ static void printout(int id, int nr, struct perf_evsel *counter, double uval,
 			counter->supported ? CNTR_NOT_COUNTED : CNTR_NOT_SUPPORTED,
 			csv_sep);
 
-		if (counter->supported)
+		if (counter->supported) {
 			print_free_counters_hint = 1;
+			if (is_mixed_hw_group(counter))
+				print_mixed_hw_group_error = 1;
+		}
 
 		fprintf(stat_config.output, "%-*s%s",
 			csv_output ? 0 : unit_width,
@@ -1757,6 +1785,11 @@ static void print_footer(void)
 "	echo 0 > /proc/sys/kernel/nmi_watchdog\n"
 "	perf stat ...\n"
 "	echo 1 > /proc/sys/kernel/nmi_watchdog\n");
+
+	if (print_mixed_hw_group_error)
+		fprintf(output,
+			"The events in group usually have to be from "
+			"the same PMU. Try reorganizing the group.\n");
 }
 
 static void print_counters(struct timespec *ts, int argc, const char **argv)
-- 
2.14.3

[PATCH 11/12] perf evsel: Only fall back group read for leader

[Arnaldo Carvalho de Melo]

From: Kan Liang <kan.liang@linux.intel.com>

Perf doesn't support mixed events from different PMUs (except software
event) in a group. The perf stat should output <not counted>/<not
supported> for all events, but it doesn't. For example,

  perf stat -e '{cycles,uncore_imc_5/umask=0xF,event=0x4/,instructions}'
       <not counted>      cycles
       <not supported>    uncore_imc_5/umask=0xF,event=0x4/
           1,024,300      instructions

If perf fails to open an event, it doesn't error out directly. It will
disable some features and retry, until the event is opened or all
features are disabled. The disabled features will not be re-enabled. The
group read is one of these features.

For the example as above, the IMC event and the leader event "cycles"
are from different PMUs. Opening the IMC event must fail. The group read
feature must be disabled for IMC event and the followed event
"instructions". The "instructions" event has the same PMU as the leader
"cycles". It can be opened successfully. Since the group read feature
has been disabled, the "instructions" event will be read as a single
event, which definitely has a value.

The group read fallback is still useful for the case which kernel
doesn't support group read. It is good enough to be handled only by the
leader.

For the fallback request from members, it must be caused by an error.
The fallback only breaks the semantics of group.  Limit the group read
fallback only for the leader.

Committer testing:

On a broadwell t450s notebook:

Before:

  # perf stat -e '{cycles,unc_cbo_cache_lookup.read_i,instructions}' sleep 1

  Performance counter stats for 'sleep 1':

     <not counted>      cycles
   <not supported>      unc_cbo_cache_lookup.read_i
           818,206      instructions

       1.003170887 seconds time elapsed

  Some events weren't counted. Try disabling the NMI watchdog:
	echo 0 > /proc/sys/kernel/nmi_watchdog
	perf stat ...
	echo 1 > /proc/sys/kernel/nmi_watchdog

After:

  # perf stat -e '{cycles,unc_cbo_cache_lookup.read_i,instructions}' sleep 1

  Performance counter stats for 'sleep 1':

     <not counted>      cycles
   <not supported>      unc_cbo_cache_lookup.read_i
     <not counted>      instructions

       1.001380511 seconds time elapsed

  Some events weren't counted. Try disabling the NMI watchdog:
	echo 0 > /proc/sys/kernel/nmi_watchdog
	perf stat ...
	echo 1 > /proc/sys/kernel/nmi_watchdog
  #

Reported-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Agustin Vega-Frias <agustinv@codeaurora.org>
Cc: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Shaokun Zhang <zhangshaokun@hisilicon.com>
Cc: Will Deacon <will.deacon@arm.com>
Fixes:  82bf311e15d2 ("perf stat: Use group read for event groups")
Link: http://lkml.kernel.org/r/1524594014-79243-3-git-send-email-kan.liang@linux.intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evsel.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 26bdeecc0452..4cd2cf93f726 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1925,7 +1925,8 @@ int perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
 		goto fallback_missing_features;
 	} else if (!perf_missing_features.group_read &&
 		    evsel->attr.inherit &&
-		   (evsel->attr.read_format & PERF_FORMAT_GROUP)) {
+		   (evsel->attr.read_format & PERF_FORMAT_GROUP) &&
+		   perf_evsel__is_group_leader(evsel)) {
 		perf_missing_features.group_read = true;
 		pr_debug2("switching off group read\n");
 		goto fallback_missing_features;
-- 
2.14.3

[PATCH 12/12] perf stat: Fix duplicate PMU name for interval print

[Arnaldo Carvalho de Melo]

From: Kan Liang <kan.liang@linux.intel.com>

PMU name is printed repeatedly for interval print, for example:

  perf stat --no-merge -e 'unc_m_clockticks' -a -I 1000
  #           time             counts unit events
     1.001053069        243,702,144      unc_m_clockticks [uncore_imc_4]
     1.001053069        244,268,304      unc_m_clockticks [uncore_imc_2]
     1.001053069        244,427,386      unc_m_clockticks [uncore_imc_0]
     1.001053069        244,583,760      unc_m_clockticks [uncore_imc_5]
     1.001053069        244,738,971      unc_m_clockticks [uncore_imc_3]
     1.001053069        244,880,309      unc_m_clockticks [uncore_imc_1]
     2.002024821        240,818,200      unc_m_clockticks [uncore_imc_4] [uncore_imc_4]
     2.002024821        240,767,812      unc_m_clockticks [uncore_imc_2] [uncore_imc_2]
     2.002024821        240,764,215      unc_m_clockticks [uncore_imc_0] [uncore_imc_0]
     2.002024821        240,759,504      unc_m_clockticks [uncore_imc_5] [uncore_imc_5]
     2.002024821        240,755,992      unc_m_clockticks [uncore_imc_3] [uncore_imc_3]
     2.002024821        240,750,403      unc_m_clockticks [uncore_imc_1] [uncore_imc_1]

For each print, the PMU name is unconditionally appended to the
counter->name.

Need to check the counter->name first. If the PMU name is already
appended, do nothing.

Committer notes:

Add and use perf_evsel->uniquified_name bool instead of doing the more
expensive strstr(event->name, pmu->name).

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Agustin Vega-Frias <agustinv@codeaurora.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Shaokun Zhang <zhangshaokun@hisilicon.com>
Cc: Will Deacon <will.deacon@arm.com>
Fixes: 8c5421c016a4 ("perf pmu: Display pmu name when printing unmerged events in stat")
Link: http://lkml.kernel.org/r/1524594014-79243-5-git-send-email-kan.liang@linux.intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-stat.c | 5 ++++-
 tools/perf/util/evsel.h   | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 30e6b374e095..f17dc601b0f3 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1284,7 +1284,8 @@ static void uniquify_event_name(struct perf_evsel *counter)
 	char *new_name;
 	char *config;
 
-	if (!counter->pmu_name || !strncmp(counter->name, counter->pmu_name,
+	if (counter->uniquified_name ||
+	    !counter->pmu_name || !strncmp(counter->name, counter->pmu_name,
 					   strlen(counter->pmu_name)))
 		return;
 
@@ -1302,6 +1303,8 @@ static void uniquify_event_name(struct perf_evsel *counter)
 			counter->name = new_name;
 		}
 	}
+
+	counter->uniquified_name = true;
 }
 
 static void collect_all_aliases(struct perf_evsel *counter,
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index d3ee3af618ef..92ec009a292d 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -115,6 +115,7 @@ struct perf_evsel {
 	unsigned int		sample_size;
 	int			id_pos;
 	int			is_pos;
+	bool			uniquified_name;
 	bool			snapshot;
 	bool 			supported;
 	bool 			needs_swap;
-- 
2.14.3

Re: [GIT PULL 00/12] perf/urgent fixes

[Ingo Molnar]

* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Hi Ingo,
> 
> 	Please consider pulling,
> 
> - Arnaldo
> 
> Test results at the end of this message, as usual.
> 
> The following changes since commit c042f7e9bb6ad9429ea0f2c9138dc06413198967:
> 
>   Merge tag 'perf-urgent-for-mingo-4.17-20180420' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent (2018-04-21 09:38:33 +0200)
> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-urgent-for-mingo-4.17-20180425
> 
> for you to fetch changes up to 80ee8c588afde077cb0439e15129579a267916c4:
> 
>   perf stat: Fix duplicate PMU name for interval print (2018-04-24 16:12:00 -0300)
> 
> ----------------------------------------------------------------
> perf/urgent fixes:
> 
> perf stat:
> 
> - Keep the '/' event modifier separator in fallback, for example when
>   fallbacking from 'cpu/cpu-cycles/' to user level only, where it should
>   become 'cpu/cpu-cycles/u' and not 'cpu/cpu-cycles/:u' (Jiri Olsa)
> 
> - Fix PMU events parsing rule, improving error reporting for
>   invalid events (Jiri Olsa)
> 
> - Disable write_backward and other event attributes for !group
>   events in a group, fixing, for instance this group: '{cycles,msr/aperf/}:S'
>   that has leader sampling (:S) and where just the 'cycles',
>   the leader event, should have the write_backward attribute
>   set, in this case it all fails because the PMU where 'msr/aperf/'
>   lives doesn't accepts write_backward style sampling (Jiri Olsa)
> 
> - Only fall back group read for leader (Kan Liang)
> 
> - Fix core PMU alias list for X86 platform (Kan Liang)
> 
> - Print out hint for mixed PMU group error (Kan Liang)
> 
> - Fix duplicate PMU name for interval print (Kan Liang)
> 
> Core:
> 
> - Set main kernel end address properly when reading kernel and
>   module maps (Namhyung Kim)
> 
> perf mem:
> 
> - Fix incorrect entries and add missing man options (Sangwon Hong)
> 
> s/390:
> 
> - Remove s390 specific strcmp_cpuid_cmp function (Thomas Richter)
> 
> - Adapt 'perf test' case record+probe_libc_inet_pton.sh for s390
> 
> - Fix s390 undefined record__auxtrace_init() return value in
>   'perf record' (Thomas Richter)
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Jiri Olsa (3):
>       perf stat: Keep the / modifier separator in fallback
>       perf pmu: Fix pmu events parsing rule
>       perf evsel: Disable write_backward for leader sampling group events
> 
> Kan Liang (4):
>       perf pmu: Fix core PMU alias list for X86 platform
>       perf stat: Print out hint for mixed PMU group error
>       perf evsel: Only fall back group read for leader
>       perf stat: Fix duplicate PMU name for interval print
> 
> Namhyung Kim (1):
>       perf machine: Set main kernel end address properly
> 
> Sangwon Hong (1):
>       perf mem: Document incorrect and missing options
> 
> Thomas Richter (3):
>       perf list: Remove s390 specific strcmp_cpuid_cmp function
>       perf test: Adapt test case record+probe_libc_inet_pton.sh for s390
>       perf record: Fix s390 undefined record__auxtrace_init() return value
> 
>  tools/perf/Documentation/perf-mem.txt              | 41 +++++++++++++++-------
>  tools/perf/arch/s390/util/auxtrace.c               |  1 +
>  tools/perf/arch/s390/util/header.c                 | 18 ----------
>  tools/perf/builtin-stat.c                          | 40 +++++++++++++++++++--
>  tools/perf/pmu-events/arch/s390/mapfile.csv        | 10 +++---
>  tools/perf/tests/attr/test-record-group-sampling   |  3 ++
>  .../tests/shell/record+probe_libc_inet_pton.sh     |  6 ++--
>  tools/perf/util/evsel.c                            | 18 +++++++---
>  tools/perf/util/evsel.h                            |  1 +
>  tools/perf/util/machine.c                          | 30 +++++++++-------
>  tools/perf/util/parse-events.y                     |  8 ++---
>  tools/perf/util/pmu.c                              | 22 +++++-------
>  12 files changed, 123 insertions(+), 75 deletions(-)

Pulled, thanks a lot Arnaldo!

	Ingo

Re: [PATCH 05/12] perf pmu: Fix pmu events parsing rule

[Adrian Hunter]

Hi

This breaks Intel PT i.e.

$ perf record -e intel_pt//u uname
event syntax error: 'intel_pt//u'
                             \___ parser error
Run 'perf list' for a list of valid events

 Usage: perf record [<options>] [<command>]
    or: perf record [<options>] -- <command> [<options>]

    -e, --event <event>   event selector. use 'perf list' to list available events

See below for cause.

On 25/04/18 19:00, Arnaldo Carvalho de Melo wrote:
> From: Jiri Olsa <jolsa@kernel.org>
> 
> Currently all the event parsing fails end up in the event_pmu rule, and
> display misleading help like:
> 
>   $ perf stat -e inst kill
>   event syntax error: 'inst'
>                        \___ Cannot find PMU `inst'. Missing kernel support?
>   ...
> 
> The reason is that the event_pmu is too strong and match also single
> string. Changing it to force the '/' separators to be part of the rule,
> and getting the proper error now:
> 
>   $ perf stat -e inst kill
>   event syntax error: 'inst'
>                        \___ parser error
>   Run 'perf list' for a list of valid events
>   ...
> 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> Reported-by: Ingo Molnar <mingo@kernel.org>
> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Link: http://lkml.kernel.org/r/20180423090823.32309-5-jolsa@kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/util/parse-events.y | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> index 7afeb80cc39e..d14464c42714 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -224,15 +224,15 @@ event_def: event_pmu |
>  	   event_bpf_file
>  
>  event_pmu:
> -PE_NAME opt_event_config
> +PE_NAME '/' event_config '/'

These are not equivalent because opt_event_config allows '//'
but event_config cannot be an empty string.

>  {
>  	struct list_head *list, *orig_terms, *terms;
>  
> -	if (parse_events_copy_term_list($2, &orig_terms))
> +	if (parse_events_copy_term_list($3, &orig_terms))
>  		YYABORT;
>  
>  	ALLOC_LIST(list);
> -	if (parse_events_add_pmu(_parse_state, list, $1, $2, false)) {
> +	if (parse_events_add_pmu(_parse_state, list, $1, $3, false)) {
>  		struct perf_pmu *pmu = NULL;
>  		int ok = 0;
>  		char *pattern;
> @@ -262,7 +262,7 @@ PE_NAME opt_event_config
>  		if (!ok)
>  			YYABORT;
>  	}
> -	parse_events_terms__delete($2);
> +	parse_events_terms__delete($3);
>  	parse_events_terms__delete(orig_terms);
>  	$$ = list;
>  }
> 

Re: [PATCH 05/12] perf pmu: Fix pmu events parsing rule

[Jiri Olsa]

On Thu, May 03, 2018 at 11:25:16AM +0300, Adrian Hunter wrote:
> Hi
> 
> This breaks Intel PT i.e.
> 
> $ perf record -e intel_pt//u uname
> event syntax error: 'intel_pt//u'
>                              \___ parser error
> Run 'perf list' for a list of valid events
> 
>  Usage: perf record [<options>] [<command>]
>     or: perf record [<options>] -- <command> [<options>]
> 
>     -e, --event <event>   event selector. use 'perf list' to list available events
> 
> See below for cause.
> 
> On 25/04/18 19:00, Arnaldo Carvalho de Melo wrote:
> > From: Jiri Olsa <jolsa@kernel.org>
> > 
> > Currently all the event parsing fails end up in the event_pmu rule, and
> > display misleading help like:
> > 
> >   $ perf stat -e inst kill
> >   event syntax error: 'inst'
> >                        \___ Cannot find PMU `inst'. Missing kernel support?
> >   ...
> > 
> > The reason is that the event_pmu is too strong and match also single
> > string. Changing it to force the '/' separators to be part of the rule,
> > and getting the proper error now:
> > 
> >   $ perf stat -e inst kill
> >   event syntax error: 'inst'
> >                        \___ parser error
> >   Run 'perf list' for a list of valid events
> >   ...
> > 
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > Reported-by: Ingo Molnar <mingo@kernel.org>
> > Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Cc: David Ahern <dsahern@gmail.com>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Link: http://lkml.kernel.org/r/20180423090823.32309-5-jolsa@kernel.org
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > ---
> >  tools/perf/util/parse-events.y | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> > index 7afeb80cc39e..d14464c42714 100644
> > --- a/tools/perf/util/parse-events.y
> > +++ b/tools/perf/util/parse-events.y
> > @@ -224,15 +224,15 @@ event_def: event_pmu |
> >  	   event_bpf_file
> >  
> >  event_pmu:
> > -PE_NAME opt_event_config
> > +PE_NAME '/' event_config '/'
> 
> These are not equivalent because opt_event_config allows '//'
> but event_config cannot be an empty string.

yep, overlooked this one, how about patch below

jirka

---
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index d14464c42714..1ed2befeca8a 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -523,6 +523,10 @@ event_term
 	list_add_tail(&term->list, head);
 	$$ = head;
 }
+|
+{
+	$$ = NULL;
+}
 
 event_term:
 PE_NAME '=' PE_NAME

Re: [PATCH 05/12] perf pmu: Fix pmu events parsing rule

[Adrian Hunter]

On 03/05/18 13:37, Jiri Olsa wrote:
> On Thu, May 03, 2018 at 11:25:16AM +0300, Adrian Hunter wrote:
>> Hi
>>
>> This breaks Intel PT i.e.
>>
>> $ perf record -e intel_pt//u uname
>> event syntax error: 'intel_pt//u'
>>                              \___ parser error
>> Run 'perf list' for a list of valid events
>>
>>  Usage: perf record [<options>] [<command>]
>>     or: perf record [<options>] -- <command> [<options>]
>>
>>     -e, --event <event>   event selector. use 'perf list' to list available events
>>
>> See below for cause.
>>
>> On 25/04/18 19:00, Arnaldo Carvalho de Melo wrote:
>>> From: Jiri Olsa <jolsa@kernel.org>
>>>
>>> Currently all the event parsing fails end up in the event_pmu rule, and
>>> display misleading help like:
>>>
>>>   $ perf stat -e inst kill
>>>   event syntax error: 'inst'
>>>                        \___ Cannot find PMU `inst'. Missing kernel support?
>>>   ...
>>>
>>> The reason is that the event_pmu is too strong and match also single
>>> string. Changing it to force the '/' separators to be part of the rule,
>>> and getting the proper error now:
>>>
>>>   $ perf stat -e inst kill
>>>   event syntax error: 'inst'
>>>                        \___ parser error
>>>   Run 'perf list' for a list of valid events
>>>   ...
>>>
>>> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>>> Reported-by: Ingo Molnar <mingo@kernel.org>
>>> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>>> Cc: David Ahern <dsahern@gmail.com>
>>> Cc: Namhyung Kim <namhyung@kernel.org>
>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>> Link: http://lkml.kernel.org/r/20180423090823.32309-5-jolsa@kernel.org
>>> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>>> ---
>>>  tools/perf/util/parse-events.y | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
>>> index 7afeb80cc39e..d14464c42714 100644
>>> --- a/tools/perf/util/parse-events.y
>>> +++ b/tools/perf/util/parse-events.y
>>> @@ -224,15 +224,15 @@ event_def: event_pmu |
>>>  	   event_bpf_file
>>>  
>>>  event_pmu:
>>> -PE_NAME opt_event_config
>>> +PE_NAME '/' event_config '/'
>>
>> These are not equivalent because opt_event_config allows '//'
>> but event_config cannot be an empty string.
> 
> yep, overlooked this one, how about patch below

Seems to work but gives build warnings:

util/parse-events.y: warning: 1 shift/reduce conflict [-Wconflicts-sr]
util/parse-events.y: warning: 1 reduce/reduce conflict [-Wconflicts-rr]


> 
> jirka
> 
> ---
> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> index d14464c42714..1ed2befeca8a 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -523,6 +523,10 @@ event_term
>  	list_add_tail(&term->list, head);
>  	$$ = head;
>  }
> +|
> +{
> +	$$ = NULL;
> +}
>  
>  event_term:
>  PE_NAME '=' PE_NAME
> 

Re: [PATCH 05/12] perf pmu: Fix pmu events parsing rule

[Jiri Olsa]

On Thu, May 03, 2018 at 02:38:15PM +0300, Adrian Hunter wrote:
> On 03/05/18 13:37, Jiri Olsa wrote:
> > On Thu, May 03, 2018 at 11:25:16AM +0300, Adrian Hunter wrote:
> >> Hi
> >>
> >> This breaks Intel PT i.e.
> >>
> >> $ perf record -e intel_pt//u uname
> >> event syntax error: 'intel_pt//u'
> >>                              \___ parser error
> >> Run 'perf list' for a list of valid events
> >>
> >>  Usage: perf record [<options>] [<command>]
> >>     or: perf record [<options>] -- <command> [<options>]
> >>
> >>     -e, --event <event>   event selector. use 'perf list' to list available events
> >>
> >> See below for cause.
> >>
> >> On 25/04/18 19:00, Arnaldo Carvalho de Melo wrote:
> >>> From: Jiri Olsa <jolsa@kernel.org>
> >>>
> >>> Currently all the event parsing fails end up in the event_pmu rule, and
> >>> display misleading help like:
> >>>
> >>>   $ perf stat -e inst kill
> >>>   event syntax error: 'inst'
> >>>                        \___ Cannot find PMU `inst'. Missing kernel support?
> >>>   ...
> >>>
> >>> The reason is that the event_pmu is too strong and match also single
> >>> string. Changing it to force the '/' separators to be part of the rule,
> >>> and getting the proper error now:
> >>>
> >>>   $ perf stat -e inst kill
> >>>   event syntax error: 'inst'
> >>>                        \___ parser error
> >>>   Run 'perf list' for a list of valid events
> >>>   ...
> >>>
> >>> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> >>> Reported-by: Ingo Molnar <mingo@kernel.org>
> >>> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> >>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> >>> Cc: David Ahern <dsahern@gmail.com>
> >>> Cc: Namhyung Kim <namhyung@kernel.org>
> >>> Cc: Peter Zijlstra <peterz@infradead.org>
> >>> Link: http://lkml.kernel.org/r/20180423090823.32309-5-jolsa@kernel.org
> >>> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> >>> ---
> >>>  tools/perf/util/parse-events.y | 8 ++++----
> >>>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> >>> index 7afeb80cc39e..d14464c42714 100644
> >>> --- a/tools/perf/util/parse-events.y
> >>> +++ b/tools/perf/util/parse-events.y
> >>> @@ -224,15 +224,15 @@ event_def: event_pmu |
> >>>  	   event_bpf_file
> >>>  
> >>>  event_pmu:
> >>> -PE_NAME opt_event_config
> >>> +PE_NAME '/' event_config '/'
> >>
> >> These are not equivalent because opt_event_config allows '//'
> >> but event_config cannot be an empty string.
> > 
> > yep, overlooked this one, how about patch below
> 
> Seems to work but gives build warnings:
> 
> util/parse-events.y: warning: 1 shift/reduce conflict [-Wconflicts-sr]
> util/parse-events.y: warning: 1 reduce/reduce conflict [-Wconflicts-rr]

hm, I wonder why my bison was silent.. there's conflict
with opt_event_config reduction to empty, I'll resend

thanks,
jirka

Re: [PATCH 05/12] perf pmu: Fix pmu events parsing rule

[Jiri Olsa]

On Thu, May 03, 2018 at 02:38:15PM +0300, Adrian Hunter wrote:

SNIP

> >>> index 7afeb80cc39e..d14464c42714 100644
> >>> --- a/tools/perf/util/parse-events.y
> >>> +++ b/tools/perf/util/parse-events.y
> >>> @@ -224,15 +224,15 @@ event_def: event_pmu |
> >>>  	   event_bpf_file
> >>>  
> >>>  event_pmu:
> >>> -PE_NAME opt_event_config
> >>> +PE_NAME '/' event_config '/'
> >>
> >> These are not equivalent because opt_event_config allows '//'
> >> but event_config cannot be an empty string.
> > 
> > yep, overlooked this one, how about patch below
> 
> Seems to work but gives build warnings:
> 
> util/parse-events.y: warning: 1 shift/reduce conflict [-Wconflicts-sr]
> util/parse-events.y: warning: 1 reduce/reduce conflict [-Wconflicts-rr]

I had to make an explicit rule, so I moved the original rule code
into separate function.. hence the patch is little bigger ;-)

cc-ing Kan Liang, who recently changed this code,

I'll still have to rebase this one his recent change,
once it gets in, but should be easy

jirka


---
 tools/perf/util/parse-events.c | 43 +++++++++++++++++++++++++++++++++++
 tools/perf/util/parse-events.h |  3 +++
 tools/perf/util/parse-events.y | 51 +++++++++++++-----------------------------
 3 files changed, 62 insertions(+), 35 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 2fb0272146d8..33e3fc147f35 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -8,6 +8,7 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <sys/param.h>
+#include <fnmatch.h>
 #include "term.h"
 #include "../perf.h"
 #include "evlist.h"
@@ -1287,6 +1288,48 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 	return evsel ? 0 : -ENOMEM;
 }
 
+int parse_events_pmu(struct parse_events_state *parse_state,
+		     struct list_head *list, char *name,
+		     struct list_head *head_config)
+{
+	struct list_head *orig_terms;
+	struct perf_pmu *pmu = NULL;
+	char *pattern = NULL;
+	int ok = 0;
+
+	if (!parse_events_add_pmu(parse_state, list, name, head_config, false))
+		return 0;
+
+	if (parse_events_copy_term_list(head_config, &orig_terms))
+		return -1;
+
+	if (asprintf(&pattern, "%s*", name) < 0)
+		goto out;
+
+	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+		struct list_head *terms;
+		char *tmp = pmu->name;
+
+		if (!strncmp(tmp, "uncore_", 7) &&
+		    strncmp(tmp, "uncore_", 7))
+			tmp += 7;
+		if (!fnmatch(pattern, tmp, 0)) {
+			if (parse_events_copy_term_list(orig_terms, &terms)) {
+				ok = 0;
+				goto out;
+			}
+			if (!parse_events_add_pmu(parse_state, list, pmu->name, terms, true))
+				ok++;
+			parse_events_terms__delete(terms);
+		}
+	}
+
+out:
+	parse_events_terms__delete(orig_terms);
+	free(pattern);
+	return ok ? 0 : -1;
+}
+
 int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 			       char *str, struct list_head **listp)
 {
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 5015cfd58277..743586213ee4 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -168,6 +168,9 @@ int parse_events_add_breakpoint(struct list_head *list, int *idx,
 int parse_events_add_pmu(struct parse_events_state *parse_state,
 			 struct list_head *list, char *name,
 			 struct list_head *head_config, bool auto_merge_stats);
+int parse_events_pmu(struct parse_events_state *parse_state,
+		     struct list_head *list, char *name,
+		     struct list_head *head_config);
 
 int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 			       char *str,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index d14464c42714..a93f8c660395 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -8,7 +8,6 @@
 
 #define YYDEBUG 1
 
-#include <fnmatch.h>
 #include <linux/compiler.h>
 #include <linux/list.h>
 #include <linux/types.h>
@@ -226,44 +225,26 @@ event_def: event_pmu |
 event_pmu:
 PE_NAME '/' event_config '/'
 {
-	struct list_head *list, *orig_terms, *terms;
+	struct list_head *list;
+
+	ALLOC_LIST(list);
 
-	if (parse_events_copy_term_list($3, &orig_terms))
+	if (parse_events_pmu(_parse_state, list, $1, $3))
 		YYABORT;
 
-	ALLOC_LIST(list);
-	if (parse_events_add_pmu(_parse_state, list, $1, $3, false)) {
-		struct perf_pmu *pmu = NULL;
-		int ok = 0;
-		char *pattern;
-
-		if (asprintf(&pattern, "%s*", $1) < 0)
-			YYABORT;
-
-		while ((pmu = perf_pmu__scan(pmu)) != NULL) {
-			char *name = pmu->name;
-
-			if (!strncmp(name, "uncore_", 7) &&
-			    strncmp($1, "uncore_", 7))
-				name += 7;
-			if (!fnmatch(pattern, name, 0)) {
-				if (parse_events_copy_term_list(orig_terms, &terms)) {
-					free(pattern);
-					YYABORT;
-				}
-				if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, true))
-					ok++;
-				parse_events_terms__delete(terms);
-			}
-		}
-
-		free(pattern);
-
-		if (!ok)
-			YYABORT;
-	}
 	parse_events_terms__delete($3);
-	parse_events_terms__delete(orig_terms);
+	$$ = list;
+}
+|
+PE_NAME '/' '/'
+{
+	struct list_head *list;
+
+	ALLOC_LIST(list);
+
+	if (parse_events_pmu(_parse_state, list, $1, NULL))
+		YYABORT;
+
 	$$ = list;
 }
 |
-- 
2.13.6

Re: [PATCH 05/12] perf pmu: Fix pmu events parsing rule

[Andi Kleen]

Jiri Olsa <jolsa@redhat.com> writes:

Please fix this quickly, PT is currently totally non functional in Linus
mainline.

-Andi

[PATCH] perf tools: Fix parser for empty pmu terms case

[Jiri Olsa]

On Sat, May 05, 2018 at 08:43:11PM -0700, Andi Kleen wrote:
> Jiri Olsa <jolsa@redhat.com> writes:
> 
> Please fix this quickly, PT is currently totally non functional in Linus
> mainline.

attached.. Kan, could you please test it wrt your latest changes?

thanks,
jirka


---
Adrian reported broken event parsing for Intel PT:

  $ perf record -e intel_pt//u uname
  event syntax error: 'intel_pt//u'
                               \___ parser error
  Run 'perf list' for a list of valid events

It's caused by recent change in parsing grammar
(see Fixes: for commit).

Adding special rule with empty terms config to handle
the reported case and moving the common rule code into
new parse_events_pmu function.

Fixes: 9a4a931ce847 ("perf pmu: Fix pmu events parsing rule")
Reported-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Link: http://lkml.kernel.org/n/tip-uorb0azuem7b7ydace7cf6vc@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/parse-events.c | 46 +++++++++++++++++++++++++++++++++++++
 tools/perf/util/parse-events.h |  3 +++
 tools/perf/util/parse-events.y | 51 +++++++++++++-----------------------------
 3 files changed, 65 insertions(+), 35 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 3576aaaa9e4c..7cf326a8effe 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -8,6 +8,7 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <sys/param.h>
+#include <fnmatch.h>
 #include "term.h"
 #include "../perf.h"
 #include "evlist.h"
@@ -1294,6 +1295,51 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 	return evsel ? 0 : -ENOMEM;
 }
 
+int parse_events_pmu(struct parse_events_state *parse_state,
+		     struct list_head *list, char *name,
+		     struct list_head *head_config)
+{
+	struct list_head *orig_terms;
+	struct perf_pmu *pmu = NULL;
+	char *pattern = NULL;
+	int ok = 0;
+
+	if (!parse_events_add_pmu(parse_state, list, name,
+				  head_config, false, false))
+		return 0;
+
+	if (parse_events_copy_term_list(head_config, &orig_terms))
+		return -1;
+
+	if (asprintf(&pattern, "%s*", name) < 0)
+		goto out;
+
+	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+		char *tmp = pmu->name;
+
+		if (!strncmp(tmp, "uncore_", 7) &&
+		    strncmp(name, "uncore_", 7))
+			tmp += 7;
+		if (!fnmatch(pattern, tmp, 0)) {
+			struct list_head *terms;
+
+			if (parse_events_copy_term_list(orig_terms, &terms)) {
+				ok = 0;
+				goto out;
+			}
+			if (!parse_events_add_pmu(parse_state, list, pmu->name,
+						  terms, true, false))
+				ok++;
+			parse_events_terms__delete(terms);
+		}
+	}
+
+out:
+	parse_events_terms__delete(orig_terms);
+	free(pattern);
+	return ok ? 0 : -1;
+}
+
 int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 			       char *str, struct list_head **listp)
 {
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 4473dac27aee..553fcaf5d23e 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -170,6 +170,9 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 			 struct list_head *head_config,
 			 bool auto_merge_stats,
 			 bool use_alias);
+int parse_events_pmu(struct parse_events_state *parse_state,
+		     struct list_head *list, char *name,
+		     struct list_head *head_config);
 
 int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 			       char *str,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 47f6399a309a..d44599bece43 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -8,7 +8,6 @@
 
 #define YYDEBUG 1
 
-#include <fnmatch.h>
 #include <linux/compiler.h>
 #include <linux/list.h>
 #include <linux/types.h>
@@ -226,44 +225,26 @@ event_def: event_pmu |
 event_pmu:
 PE_NAME '/' event_config '/'
 {
-	struct list_head *list, *orig_terms, *terms;
+	struct list_head *list;
+
+	ALLOC_LIST(list);
 
-	if (parse_events_copy_term_list($3, &orig_terms))
+	if (parse_events_pmu(_parse_state, list, $1, $3))
 		YYABORT;
 
-	ALLOC_LIST(list);
-	if (parse_events_add_pmu(_parse_state, list, $1, $3, false, false)) {
-		struct perf_pmu *pmu = NULL;
-		int ok = 0;
-		char *pattern;
-
-		if (asprintf(&pattern, "%s*", $1) < 0)
-			YYABORT;
-
-		while ((pmu = perf_pmu__scan(pmu)) != NULL) {
-			char *name = pmu->name;
-
-			if (!strncmp(name, "uncore_", 7) &&
-			    strncmp($1, "uncore_", 7))
-				name += 7;
-			if (!fnmatch(pattern, name, 0)) {
-				if (parse_events_copy_term_list(orig_terms, &terms)) {
-					free(pattern);
-					YYABORT;
-				}
-				if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, true, false))
-					ok++;
-				parse_events_terms__delete(terms);
-			}
-		}
-
-		free(pattern);
-
-		if (!ok)
-			YYABORT;
-	}
 	parse_events_terms__delete($3);
-	parse_events_terms__delete(orig_terms);
+	$$ = list;
+}
+|
+PE_NAME '/' '/'
+{
+	struct list_head *list;
+
+	ALLOC_LIST(list);
+
+	if (parse_events_pmu(_parse_state, list, $1, NULL))
+		YYABORT;
+
 	$$ = list;
 }
 |
-- 
2.13.6

Re: [PATCH] perf tools: Fix parser for empty pmu terms case

[Liang, Kan]

On 5/6/2018 10:28 AM, Jiri Olsa wrote:
> On Sat, May 05, 2018 at 08:43:11PM -0700, Andi Kleen wrote:
>> Jiri Olsa <jolsa@redhat.com> writes:
>>
>> Please fix this quickly, PT is currently totally non functional in Linus
>> mainline.
> 
> attached.. Kan, could you please test it wrt your latest changes?
> 
> thanks,
> jirka
> 
> 
> ---
> Adrian reported broken event parsing for Intel PT:
> 
>    $ perf record -e intel_pt//u uname
>    event syntax error: 'intel_pt//u'
>                                 \___ parser error
>    Run 'perf list' for a list of valid events
> 
> It's caused by recent change in parsing grammar
> (see Fixes: for commit).
> 
> Adding special rule with empty terms config to handle
> the reported case and moving the common rule code into
> new parse_events_pmu function.
> 
> Fixes: 9a4a931ce847 ("perf pmu: Fix pmu events parsing rule")
> Reported-by: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Andi Kleen <ak@linux.intel.com>
> Link: http://lkml.kernel.org/n/tip-uorb0azuem7b7ydace7cf6vc@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>

The patch looks good to me.
I tested my latest change and Intel PT. Both looks good.

./perf stat -e '{unc_m_cas_count.all,unc_m_clockticks}' -a -I 1000   # 
         time             counts unit events
      1.001556622             95,686      unc_m_cas_count.all
      1.001556622      1,900,544,149      unc_m_clockticks
      2.002722048            159,598      unc_m_cas_count.all
      2.002722048      2,339,028,614      unc_m_clockticks

./perf record -e intel_pt//u
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.044 MB perf.data ]


Tested-by: Kan Liang <kan.liang@linux.intel.com>

> ---
>   tools/perf/util/parse-events.c | 46 +++++++++++++++++++++++++++++++++++++
>   tools/perf/util/parse-events.h |  3 +++
>   tools/perf/util/parse-events.y | 51 +++++++++++++-----------------------------
>   3 files changed, 65 insertions(+), 35 deletions(-)
> 
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 3576aaaa9e4c..7cf326a8effe 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -8,6 +8,7 @@
>   #include <sys/stat.h>
>   #include <fcntl.h>
>   #include <sys/param.h>
> +#include <fnmatch.h>
>   #include "term.h"
>   #include "../perf.h"
>   #include "evlist.h"
> @@ -1294,6 +1295,51 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
>   	return evsel ? 0 : -ENOMEM;
>   }
>   
> +int parse_events_pmu(struct parse_events_state *parse_state,
> +		     struct list_head *list, char *name,
> +		     struct list_head *head_config)
> +{
> +	struct list_head *orig_terms;
> +	struct perf_pmu *pmu = NULL;
> +	char *pattern = NULL;
> +	int ok = 0;
> +
> +	if (!parse_events_add_pmu(parse_state, list, name,
> +				  head_config, false, false))
> +		return 0;
> +
> +	if (parse_events_copy_term_list(head_config, &orig_terms))
> +		return -1;
> +
> +	if (asprintf(&pattern, "%s*", name) < 0)
> +		goto out;
> +
> +	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
> +		char *tmp = pmu->name;
> +
> +		if (!strncmp(tmp, "uncore_", 7) &&
> +		    strncmp(name, "uncore_", 7))
> +			tmp += 7;
> +		if (!fnmatch(pattern, tmp, 0)) {
> +			struct list_head *terms;
> +
> +			if (parse_events_copy_term_list(orig_terms, &terms)) {
> +				ok = 0;
> +				goto out;
> +			}
> +			if (!parse_events_add_pmu(parse_state, list, pmu->name,
> +						  terms, true, false))
> +				ok++;
> +			parse_events_terms__delete(terms);
> +		}
> +	}
> +
> +out:
> +	parse_events_terms__delete(orig_terms);
> +	free(pattern);
> +	return ok ? 0 : -1;
> +}
> +
>   int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
>   			       char *str, struct list_head **listp)
>   {
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index 4473dac27aee..553fcaf5d23e 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -170,6 +170,9 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
>   			 struct list_head *head_config,
>   			 bool auto_merge_stats,
>   			 bool use_alias);
> +int parse_events_pmu(struct parse_events_state *parse_state,
> +		     struct list_head *list, char *name,
> +		     struct list_head *head_config);
>   
>   int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
>   			       char *str,
> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> index 47f6399a309a..d44599bece43 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -8,7 +8,6 @@
>   
>   #define YYDEBUG 1
>   
> -#include <fnmatch.h>
>   #include <linux/compiler.h>
>   #include <linux/list.h>
>   #include <linux/types.h>
> @@ -226,44 +225,26 @@ event_def: event_pmu |
>   event_pmu:
>   PE_NAME '/' event_config '/'
>   {
> -	struct list_head *list, *orig_terms, *terms;
> +	struct list_head *list;
> +
> +	ALLOC_LIST(list);
>   
> -	if (parse_events_copy_term_list($3, &orig_terms))
> +	if (parse_events_pmu(_parse_state, list, $1, $3))
>   		YYABORT;
>   
> -	ALLOC_LIST(list);
> -	if (parse_events_add_pmu(_parse_state, list, $1, $3, false, false)) {
> -		struct perf_pmu *pmu = NULL;
> -		int ok = 0;
> -		char *pattern;
> -
> -		if (asprintf(&pattern, "%s*", $1) < 0)
> -			YYABORT;
> -
> -		while ((pmu = perf_pmu__scan(pmu)) != NULL) {
> -			char *name = pmu->name;
> -
> -			if (!strncmp(name, "uncore_", 7) &&
> -			    strncmp($1, "uncore_", 7))
> -				name += 7;
> -			if (!fnmatch(pattern, name, 0)) {
> -				if (parse_events_copy_term_list(orig_terms, &terms)) {
> -					free(pattern);
> -					YYABORT;
> -				}
> -				if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, true, false))
> -					ok++;
> -				parse_events_terms__delete(terms);
> -			}
> -		}
> -
> -		free(pattern);
> -
> -		if (!ok)
> -			YYABORT;
> -	}
>   	parse_events_terms__delete($3);
> -	parse_events_terms__delete(orig_terms);
> +	$$ = list;
> +}
> +|
> +PE_NAME '/' '/'
> +{
> +	struct list_head *list;
> +
> +	ALLOC_LIST(list);
> +
> +	if (parse_events_pmu(_parse_state, list, $1, NULL))
> +		YYABORT;
> +
>   	$$ = list;
>   }
>   |
> 

Re: [PATCH] perf tools: Fix parser for empty pmu terms case

[Adrian Hunter]

On 06/05/18 17:28, Jiri Olsa wrote:
> On Sat, May 05, 2018 at 08:43:11PM -0700, Andi Kleen wrote:
>> Jiri Olsa <jolsa@redhat.com> writes:
>>
>> Please fix this quickly, PT is currently totally non functional in Linus
>> mainline.
> 
> attached.. Kan, could you please test it wrt your latest changes?
> 
> thanks,
> jirka
> 
> 
> ---
> Adrian reported broken event parsing for Intel PT:
> 
>   $ perf record -e intel_pt//u uname
>   event syntax error: 'intel_pt//u'
>                                \___ parser error
>   Run 'perf list' for a list of valid events
> 
> It's caused by recent change in parsing grammar
> (see Fixes: for commit).
> 
> Adding special rule with empty terms config to handle
> the reported case and moving the common rule code into
> new parse_events_pmu function.

Hi

Can you explain why that is needed instead of just changing the grammar e.g.

diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 7afeb80cc39e..28806e0c7950 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -73,6 +73,7 @@ static void inc_group_count(struct list_head *list,
 %type <num> value_sym
 %type <head> event_config
 %type <head> opt_event_config
+%type <head> opt_pmu_config
 %type <term> event_term
 %type <head> event_pmu
 %type <head> event_legacy_symbol
@@ -224,7 +225,7 @@ event_def: event_pmu |
 	   event_bpf_file

 event_pmu:
-PE_NAME opt_event_config
+PE_NAME opt_pmu_config
 {
 	struct list_head *list, *orig_terms, *terms;

@@ -496,6 +497,17 @@ opt_event_config:
 	$$ = NULL;
 }

+opt_pmu_config:
+'/' event_config '/'
+{
+	$$ = $2;
+}
+|
+'/' '/'
+{
+	$$ = NULL;
+}
+
 start_terms: event_config
 {
 	struct parse_events_state *parse_state = _parse_state;



> 
> Fixes: 9a4a931ce847 ("perf pmu: Fix pmu events parsing rule")
> Reported-by: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Andi Kleen <ak@linux.intel.com>
> Link: http://lkml.kernel.org/n/tip-uorb0azuem7b7ydace7cf6vc@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/parse-events.c | 46 +++++++++++++++++++++++++++++++++++++
>  tools/perf/util/parse-events.h |  3 +++
>  tools/perf/util/parse-events.y | 51 +++++++++++++-----------------------------
>  3 files changed, 65 insertions(+), 35 deletions(-)
> 
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 3576aaaa9e4c..7cf326a8effe 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -8,6 +8,7 @@
>  #include <sys/stat.h>
>  #include <fcntl.h>
>  #include <sys/param.h>
> +#include <fnmatch.h>
>  #include "term.h"
>  #include "../perf.h"
>  #include "evlist.h"
> @@ -1294,6 +1295,51 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
>  	return evsel ? 0 : -ENOMEM;
>  }
>  
> +int parse_events_pmu(struct parse_events_state *parse_state,
> +		     struct list_head *list, char *name,
> +		     struct list_head *head_config)
> +{
> +	struct list_head *orig_terms;
> +	struct perf_pmu *pmu = NULL;
> +	char *pattern = NULL;
> +	int ok = 0;
> +
> +	if (!parse_events_add_pmu(parse_state, list, name,
> +				  head_config, false, false))
> +		return 0;
> +
> +	if (parse_events_copy_term_list(head_config, &orig_terms))
> +		return -1;
> +
> +	if (asprintf(&pattern, "%s*", name) < 0)
> +		goto out;
> +
> +	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
> +		char *tmp = pmu->name;
> +
> +		if (!strncmp(tmp, "uncore_", 7) &&
> +		    strncmp(name, "uncore_", 7))
> +			tmp += 7;
> +		if (!fnmatch(pattern, tmp, 0)) {
> +			struct list_head *terms;
> +
> +			if (parse_events_copy_term_list(orig_terms, &terms)) {
> +				ok = 0;
> +				goto out;
> +			}
> +			if (!parse_events_add_pmu(parse_state, list, pmu->name,
> +						  terms, true, false))
> +				ok++;
> +			parse_events_terms__delete(terms);
> +		}
> +	}
> +
> +out:
> +	parse_events_terms__delete(orig_terms);
> +	free(pattern);
> +	return ok ? 0 : -1;
> +}
> +
>  int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
>  			       char *str, struct list_head **listp)
>  {
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index 4473dac27aee..553fcaf5d23e 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -170,6 +170,9 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
>  			 struct list_head *head_config,
>  			 bool auto_merge_stats,
>  			 bool use_alias);
> +int parse_events_pmu(struct parse_events_state *parse_state,
> +		     struct list_head *list, char *name,
> +		     struct list_head *head_config);
>  
>  int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
>  			       char *str,
> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> index 47f6399a309a..d44599bece43 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -8,7 +8,6 @@
>  
>  #define YYDEBUG 1
>  
> -#include <fnmatch.h>
>  #include <linux/compiler.h>
>  #include <linux/list.h>
>  #include <linux/types.h>
> @@ -226,44 +225,26 @@ event_def: event_pmu |
>  event_pmu:
>  PE_NAME '/' event_config '/'
>  {
> -	struct list_head *list, *orig_terms, *terms;
> +	struct list_head *list;
> +
> +	ALLOC_LIST(list);
>  
> -	if (parse_events_copy_term_list($3, &orig_terms))
> +	if (parse_events_pmu(_parse_state, list, $1, $3))
>  		YYABORT;
>  
> -	ALLOC_LIST(list);
> -	if (parse_events_add_pmu(_parse_state, list, $1, $3, false, false)) {
> -		struct perf_pmu *pmu = NULL;
> -		int ok = 0;
> -		char *pattern;
> -
> -		if (asprintf(&pattern, "%s*", $1) < 0)
> -			YYABORT;
> -
> -		while ((pmu = perf_pmu__scan(pmu)) != NULL) {
> -			char *name = pmu->name;
> -
> -			if (!strncmp(name, "uncore_", 7) &&
> -			    strncmp($1, "uncore_", 7))
> -				name += 7;
> -			if (!fnmatch(pattern, name, 0)) {
> -				if (parse_events_copy_term_list(orig_terms, &terms)) {
> -					free(pattern);
> -					YYABORT;
> -				}
> -				if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, true, false))
> -					ok++;
> -				parse_events_terms__delete(terms);
> -			}
> -		}
> -
> -		free(pattern);
> -
> -		if (!ok)
> -			YYABORT;
> -	}
>  	parse_events_terms__delete($3);
> -	parse_events_terms__delete(orig_terms);
> +	$$ = list;
> +}
> +|
> +PE_NAME '/' '/'
> +{
> +	struct list_head *list;
> +
> +	ALLOC_LIST(list);
> +
> +	if (parse_events_pmu(_parse_state, list, $1, NULL))
> +		YYABORT;
> +
>  	$$ = list;
>  }
>  |
> 

Re: [PATCH] perf tools: Fix parser for empty pmu terms case

[Jiri Olsa]

On Mon, May 07, 2018 at 10:21:42AM +0300, Adrian Hunter wrote:
> On 06/05/18 17:28, Jiri Olsa wrote:
> > On Sat, May 05, 2018 at 08:43:11PM -0700, Andi Kleen wrote:
> >> Jiri Olsa <jolsa@redhat.com> writes:
> >>
> >> Please fix this quickly, PT is currently totally non functional in Linus
> >> mainline.
> > 
> > attached.. Kan, could you please test it wrt your latest changes?
> > 
> > thanks,
> > jirka
> > 
> > 
> > ---
> > Adrian reported broken event parsing for Intel PT:
> > 
> >   $ perf record -e intel_pt//u uname
> >   event syntax error: 'intel_pt//u'
> >                                \___ parser error
> >   Run 'perf list' for a list of valid events
> > 
> > It's caused by recent change in parsing grammar
> > (see Fixes: for commit).
> > 
> > Adding special rule with empty terms config to handle
> > the reported case and moving the common rule code into
> > new parse_events_pmu function.
> 
> Hi
> 
> Can you explain why that is needed instead of just changing the grammar e.g.

well, for one, mine is working ;-)

[jolsa@krava perf]$ sudo ./perf record -e intel_pt//u uname
event syntax error: 'intel_pt//u'
                              \___ parser error
Run 'perf list' for a list of valid events

 Usage: perf record [<options>] [<command>]
    or: perf record [<options>] -- <command> [<options>]

    -e, --event <event>   event selector. use 'perf list' to list available events


but yep, it's better solution.. with extra changes like below

jirka


---
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 47f6399a309a..155d2570274f 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -73,6 +73,7 @@ static void inc_group_count(struct list_head *list,
 %type <num> value_sym
 %type <head> event_config
 %type <head> opt_event_config
+%type <head> opt_pmu_config
 %type <term> event_term
 %type <head> event_pmu
 %type <head> event_legacy_symbol
@@ -224,15 +225,15 @@ event_def: event_pmu |
 	   event_bpf_file
 
 event_pmu:
-PE_NAME '/' event_config '/'
+PE_NAME opt_pmu_config
 {
 	struct list_head *list, *orig_terms, *terms;
 
-	if (parse_events_copy_term_list($3, &orig_terms))
+	if (parse_events_copy_term_list($2, &orig_terms))
 		YYABORT;
 
 	ALLOC_LIST(list);
-	if (parse_events_add_pmu(_parse_state, list, $1, $3, false, false)) {
+	if (parse_events_add_pmu(_parse_state, list, $1, $2, false, false)) {
 		struct perf_pmu *pmu = NULL;
 		int ok = 0;
 		char *pattern;
@@ -262,7 +263,7 @@ PE_NAME '/' event_config '/'
 		if (!ok)
 			YYABORT;
 	}
-	parse_events_terms__delete($3);
+	parse_events_terms__delete($2);
 	parse_events_terms__delete(orig_terms);
 	$$ = list;
 }
@@ -496,6 +497,17 @@ opt_event_config:
 	$$ = NULL;
 }
 
+opt_pmu_config:
+'/' event_config '/'
+{
+	$$ = $2;
+}
+|
+'/' '/'
+{
+	$$ = NULL;
+}
+
 start_terms: event_config
 {
 	struct parse_events_state *parse_state = _parse_state;

Re: [PATCH] perf tools: Fix parser for empty pmu terms case

[Arnaldo Carvalho de Melo]

Em Mon, May 07, 2018 at 10:12:03AM +0200, Jiri Olsa escreveu:
> On Mon, May 07, 2018 at 10:21:42AM +0300, Adrian Hunter wrote:
> > On 06/05/18 17:28, Jiri Olsa wrote:
> > > On Sat, May 05, 2018 at 08:43:11PM -0700, Andi Kleen wrote:
> > >> Jiri Olsa <jolsa@redhat.com> writes:
> > >>
> > >> Please fix this quickly, PT is currently totally non functional in Linus
> > >> mainline.
> > > 
> > > attached.. Kan, could you please test it wrt your latest changes?
> > > 
> > > thanks,
> > > jirka
> > > 
> > > 
> > > ---
> > > Adrian reported broken event parsing for Intel PT:
> > > 
> > >   $ perf record -e intel_pt//u uname
> > >   event syntax error: 'intel_pt//u'
> > >                                \___ parser error
> > >   Run 'perf list' for a list of valid events
> > > 
> > > It's caused by recent change in parsing grammar
> > > (see Fixes: for commit).
> > > 
> > > Adding special rule with empty terms config to handle
> > > the reported case and moving the common rule code into
> > > new parse_events_pmu function.
> > 
> > Hi
> > 
> > Can you explain why that is needed instead of just changing the grammar e.g.
> 
> well, for one, mine is working ;-)
> 
> [jolsa@krava perf]$ sudo ./perf record -e intel_pt//u uname
> event syntax error: 'intel_pt//u'
>                               \___ parser error
> Run 'perf list' for a list of valid events
> 
>  Usage: perf record [<options>] [<command>]
>     or: perf record [<options>] -- <command> [<options>]
> 
>     -e, --event <event>   event selector. use 'perf list' to list available events
> 
> 
> but yep, it's better solution.. with extra changes like below

So, can I get a final patch, with an Ack from Adrian, please?

- Arnaldo
 
> jirka
> 
> 
> ---
> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> index 47f6399a309a..155d2570274f 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -73,6 +73,7 @@ static void inc_group_count(struct list_head *list,
>  %type <num> value_sym
>  %type <head> event_config
>  %type <head> opt_event_config
> +%type <head> opt_pmu_config
>  %type <term> event_term
>  %type <head> event_pmu
>  %type <head> event_legacy_symbol
> @@ -224,15 +225,15 @@ event_def: event_pmu |
>  	   event_bpf_file
>  
>  event_pmu:
> -PE_NAME '/' event_config '/'
> +PE_NAME opt_pmu_config
>  {
>  	struct list_head *list, *orig_terms, *terms;
>  
> -	if (parse_events_copy_term_list($3, &orig_terms))
> +	if (parse_events_copy_term_list($2, &orig_terms))
>  		YYABORT;
>  
>  	ALLOC_LIST(list);
> -	if (parse_events_add_pmu(_parse_state, list, $1, $3, false, false)) {
> +	if (parse_events_add_pmu(_parse_state, list, $1, $2, false, false)) {
>  		struct perf_pmu *pmu = NULL;
>  		int ok = 0;
>  		char *pattern;
> @@ -262,7 +263,7 @@ PE_NAME '/' event_config '/'
>  		if (!ok)
>  			YYABORT;
>  	}
> -	parse_events_terms__delete($3);
> +	parse_events_terms__delete($2);
>  	parse_events_terms__delete(orig_terms);
>  	$$ = list;
>  }
> @@ -496,6 +497,17 @@ opt_event_config:
>  	$$ = NULL;
>  }
>  
> +opt_pmu_config:
> +'/' event_config '/'
> +{
> +	$$ = $2;
> +}
> +|
> +'/' '/'
> +{
> +	$$ = NULL;
> +}
> +
>  start_terms: event_config
>  {
>  	struct parse_events_state *parse_state = _parse_state;

Re: [PATCH 05/12] perf pmu: Fix pmu events parsing rule

[Arnaldo Carvalho de Melo]

Em Sat, May 05, 2018 at 08:43:11PM -0700, Andi Kleen escreveu:
> Jiri Olsa <jolsa@redhat.com> writes:
> 
> Please fix this quickly, PT is currently totally non functional in Linus
> mainline.

Ok, so I'm reverting this patch, the previous situation was just a
misleading error message, so it can wait for the discussion about the
parser fixes to come to a conclusion and a proper patch to be submitted.

- Arnaldo

Re: [PATCH 05/12] perf pmu: Fix pmu events parsing rule

[Jiri Olsa]

On Mon, May 07, 2018 at 03:37:49PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sat, May 05, 2018 at 08:43:11PM -0700, Andi Kleen escreveu:
> > Jiri Olsa <jolsa@redhat.com> writes:
> > 
> > Please fix this quickly, PT is currently totally non functional in Linus
> > mainline.
> 
> Ok, so I'm reverting this patch, the previous situation was just a
> misleading error message, so it can wait for the discussion about the
> parser fixes to come to a conclusion and a proper patch to be submitted.

ok, I was waiting for some feedback on the new patch.. I'll merge
it with the one you reverted now

thanks,
jirka

Re: [PATCH 05/12] perf pmu: Fix pmu events parsing rule

[Arnaldo Carvalho de Melo]

Em Mon, May 07, 2018 at 03:37:49PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Sat, May 05, 2018 at 08:43:11PM -0700, Andi Kleen escreveu:
> > Jiri Olsa <jolsa@redhat.com> writes:
> > 
> > Please fix this quickly, PT is currently totally non functional in Linus
> > mainline.
> 
> Ok, so I'm reverting this patch, the previous situation was just a
> misleading error message, so it can wait for the discussion about the
> parser fixes to come to a conclusion and a proper patch to be submitted.

... and I'm adding this to my perf/core branch, so that we notice this
faster in the future:

diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 18b06444f230..6829dd416a99 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -1309,6 +1309,14 @@ static int test__checkevent_config_cache(struct perf_evlist *evlist)
 	return 0;
 }
 
+static int test__intel_pt(struct perf_evlist *evlist)
+{
+	struct perf_evsel *evsel = perf_evlist__first(evlist);
+
+	TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "intel_pt//u") == 0);
+	return 0;
+}
+
 static int count_tracepoints(void)
 {
 	struct dirent *events_ent;
@@ -1637,6 +1645,11 @@ static struct evlist_test test__events[] = {
 		.check = test__checkevent_config_cache,
 		.id    = 51,
 	},
+	{
+		.name  = "intel_pt//u",
+		.check = test__intel_pt,
+		.id    = 52,
+	},
 };
 
 static struct evlist_test test__events_pmu[] = {

Re: [PATCH 05/12] perf pmu: Fix pmu events parsing rule

[Arnaldo Carvalho de Melo]

Em Mon, May 07, 2018 at 09:16:53PM +0200, Jiri Olsa escreveu:
> On Mon, May 07, 2018 at 03:37:49PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Sat, May 05, 2018 at 08:43:11PM -0700, Andi Kleen escreveu:
> > > Jiri Olsa <jolsa@redhat.com> writes:
> > > 
> > > Please fix this quickly, PT is currently totally non functional in Linus
> > > mainline.
> > 
> > Ok, so I'm reverting this patch, the previous situation was just a
> > misleading error message, so it can wait for the discussion about the
> > parser fixes to come to a conclusion and a proper patch to be submitted.
> 
> ok, I was waiting for some feedback on the new patch.. I'll merge
> it with the one you reverted now

Ok!

- Arnaldo

Re: [PATCH 05/12] perf pmu: Fix pmu events parsing rule

[Jiri Olsa]

On Mon, May 07, 2018 at 04:24:08PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, May 07, 2018 at 03:37:49PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Sat, May 05, 2018 at 08:43:11PM -0700, Andi Kleen escreveu:
> > > Jiri Olsa <jolsa@redhat.com> writes:
> > > 
> > > Please fix this quickly, PT is currently totally non functional in Linus
> > > mainline.
> > 
> > Ok, so I'm reverting this patch, the previous situation was just a
> > misleading error message, so it can wait for the discussion about the
> > parser fixes to come to a conclusion and a proper patch to be submitted.
> 
> ... and I'm adding this to my perf/core branch, so that we notice this
> faster in the future:

sry, overlooked this one.. good idea, looks ok to me, ack

jirka

> 
> diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> index 18b06444f230..6829dd416a99 100644
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -1309,6 +1309,14 @@ static int test__checkevent_config_cache(struct perf_evlist *evlist)
>  	return 0;
>  }
>  
> +static int test__intel_pt(struct perf_evlist *evlist)
> +{
> +	struct perf_evsel *evsel = perf_evlist__first(evlist);
> +
> +	TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "intel_pt//u") == 0);
> +	return 0;
> +}
> +
>  static int count_tracepoints(void)
>  {
>  	struct dirent *events_ent;
> @@ -1637,6 +1645,11 @@ static struct evlist_test test__events[] = {
>  		.check = test__checkevent_config_cache,
>  		.id    = 51,
>  	},
> +	{
> +		.name  = "intel_pt//u",
> +		.check = test__intel_pt,
> +		.id    = 52,
> +	},
>  };
>  
>  static struct evlist_test test__events_pmu[] = {