1. 24 Apr, 2023 5 commits
    • James Clark's avatar
      perf cs-etm: Validate options after applying them · 35c51f83
      James Clark authored
      Currently the cs_etm_set_option() function both validates and applies
      the config options. Because it's only called when they are added
      automatically, there are some paths where the user can apply the option
      on the command line and skip the validation. By moving it to the end it
      covers both cases.
      
      Also, options don't need to be re-applied anyway, Perf handles parsing
      and applying the config terms automatically.
      Signed-off-by: default avatarJames Clark <james.clark@arm.com>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
      Cc: Denis Nikitin <denik@google.com>
      Cc: Ian Rogers <irogers@google.com>
      Cc: Ingo Molnar <mingo@redhat.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: John Garry <john.g.garry@oracle.com>
      Cc: Leo Yan <leo.yan@linaro.org>
      Cc: Mark Rutland <mark.rutland@arm.com>
      Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
      Cc: Mike Leach <mike.leach@linaro.org>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
      Cc: Will Deacon <will@kernel.org>
      Cc: Yang Shi <shy828301@gmail.com>
      Cc: coresight@lists.linaro.org
      Cc: linux-arm-kernel@lists.infradead.org
      Link: https://lore.kernel.org/r/20230424134748.228137-5-james.clark@arm.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      35c51f83
    • James Clark's avatar
      perf cs-etm: Don't test full_auxtrace because it's always set · 3963d84b
      James Clark authored
      There is no path in cs-etm where this isn't true so it doesn't need to
      be tested. Also re-order the beginning of cs_etm_recording_options() so
      that nothing is done until the early exit is passed.
      Signed-off-by: default avatarJames Clark <james.clark@arm.com>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
      Cc: Denis Nikitin <denik@google.com>
      Cc: Ian Rogers <irogers@google.com>
      Cc: Ingo Molnar <mingo@redhat.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: John Garry <john.g.garry@oracle.com>
      Cc: Leo Yan <leo.yan@linaro.org>
      Cc: Mark Rutland <mark.rutland@arm.com>
      Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
      Cc: Mike Leach <mike.leach@linaro.org>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
      Cc: Will Deacon <will@kernel.org>
      Cc: Yang Shi <shy828301@gmail.com>
      Cc: coresight@lists.linaro.org
      Cc: linux-arm-kernel@lists.infradead.org
      Link: https://lore.kernel.org/r/20230424134748.228137-4-james.clark@arm.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      3963d84b
    • James Clark's avatar
      perf tools: Add util function for overriding user set config values · 6593f019
      James Clark authored
      There is some duplicated code to only override config values if they
      haven't already been set by the user so make a util function for this.
      Signed-off-by: default avatarJames Clark <james.clark@arm.com>
      Acked-by: default avatarAdrian Hunter <adrian.hunter@intel.com>
      Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
      Cc: Denis Nikitin <denik@google.com>
      Cc: Ian Rogers <irogers@google.com>
      Cc: Ingo Molnar <mingo@redhat.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: John Garry <john.g.garry@oracle.com>
      Cc: Leo Yan <leo.yan@linaro.org>
      Cc: Mark Rutland <mark.rutland@arm.com>
      Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
      Cc: Mike Leach <mike.leach@linaro.org>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
      Cc: Will Deacon <will@kernel.org>
      Cc: Yang Shi <shy828301@gmail.com>
      Cc: coresight@lists.linaro.org
      Cc: linux-arm-kernel@lists.infradead.org
      Link: https://lore.kernel.org/r/20230424134748.228137-3-james.clark@arm.com
      [ Moved evsel__set_config_if_unset() to util/pmu.c to avoid dragging stuff into the python binding ]
      Signed-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      6593f019
    • James Clark's avatar
      perf cs-etm: Fix timeless decode mode detection · 449067f3
      James Clark authored
      In this context, timeless refers to the trace data rather than the perf
      event data. But when detecting whether there are timestamps in the trace
      data or not, the presence of a timestamp flag on any perf event is used.
      
      Since commit f42c0ce5 ("perf record: Always get text_poke events
      with --kcore option") timestamps were added to a tracking event when
      --kcore is used which breaks this detection mechanism. Fix it by
      detecting if trace timestamps exist by looking at the ETM config flags.
      This would have always been a more accurate way of doing it anyway.
      
      This fixes the following error message when using --kcore with
      Coresight:
      
        $ perf record --kcore -e cs_etm// --per-thread
        $ perf report
        The perf.data/data data has no samples!
      
      Fixes: f42c0ce5 ("perf record: Always get text_poke events with --kcore option")
      Reported-by: default avatarYang Shi <shy828301@gmail.com>
      Signed-off-by: default avatarJames Clark <james.clark@arm.com>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
      Cc: Ian Rogers <irogers@google.com>
      Cc: Ingo Molnar <mingo@redhat.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: John Garry <john.g.garry@oracle.com>
      Cc: Leo Yan <leo.yan@linaro.org>
      Cc: Mark Rutland <mark.rutland@arm.com>
      Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
      Cc: Mike Leach <mike.leach@linaro.org>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
      Cc: Will Deacon <will@kernel.org>
      Cc: coresight@lists.linaro.org
      Cc: denik@google.com
      Cc: linux-arm-kernel@lists.infradead.org
      Link: https://lore.kernel.org/lkml/CAHbLzkrJQTrYBtPkf=jf3OpQ-yBcJe7XkvQstX9j2frz4WF-SQ@mail.gmail.com/
      Link: https://lore.kernel.org/r/20230424134748.228137-2-james.clark@arm.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      449067f3
    • Arnaldo Carvalho de Melo's avatar
      perf evsel: Introduce evsel__name_is() method to check if the evsel name is equal to a given string · ce1d3bc2
      Arnaldo Carvalho de Melo authored
      This makes the logic a bit clear by avoiding the !strcmp() pattern and
      also a way to intercept the pointer if we need to do extra validation on
      it or to do lazy setting of evsel->name via evsel__name(evsel).
      Reviewed-by: default avatar"Liang, Kan" <kan.liang@linux.intel.com>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Ian Rogers <irogers@google.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Link: https://lore.kernel.org/lkml/ZEGLM8VehJbS0gP2@kernel.orgSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      ce1d3bc2
  2. 20 Apr, 2023 2 commits
    • Ian Rogers's avatar
      libperf rc_check: Enable implicitly with sanitizers · 9be6ab18
      Ian Rogers authored
      If using leak sanitizer then implicitly enable reference count checking.
      Signed-off-by: default avatarIan Rogers <irogers@google.com>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
      Cc: Ingo Molnar <mingo@redhat.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: Mark Rutland <mark.rutland@arm.com>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Link: https://lore.kernel.org/r/20230420171812.561603-1-irogers@google.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      9be6ab18
    • Ian Rogers's avatar
      perf test: Fix maps use after put · edd4cab2
      Ian Rogers authored
      Fix a use after put reference count issue. maps is copied from leader,
      but the leader is put on line 79 and then maps is used to read the
      reference count below - so a use after put, with the put of maps
      happening within thread__put. Fix by reversing the order of puts so
      that the leader is put last.
      
      To explain the reference count checker, I wrote this up as a little
      example here:
      https://perf.wiki.kernel.org/index.php/Reference_Count_Checking
      
      Note, the bug was introduced by the committer and wasn't present in
      the original reference count patch set.
      
      Committer notes:
      
      Yes, the bug predated your patch and is detected by the reference count
      checking you contributed.
      
      This was just part of splitting up your series into smaller chunks, in
      this case either we fix the problem detected while developing this
      reference counting infrastructure before the patch introducing REFCNT_CHECKING
      or fix it later after the merged infrastructure, when built with
      EXTRA_CFLAGS="-DREFCNT_CHECKING=1" detects it when running 'perf test', which
      is what this patch does.
      Signed-off-by: default avatarIan Rogers <irogers@google.com>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: Mark Rutland <mark.rutland@arm.com>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Link: http://lore.kernel.org/lkml/20230420030430.489243-1-irogers@google.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      edd4cab2
  3. 19 Apr, 2023 9 commits
  4. 18 Apr, 2023 5 commits
  5. 17 Apr, 2023 7 commits
    • Ian Rogers's avatar
      perf namespaces: Add reference count checking · c35ce1d9
      Ian Rogers authored
      Add reference count checking controlled by REFCNT_CHECKING ifdef. The
      reference count checking interposes an allocated pointer between the
      reference counted struct on a get and frees the pointer on a put.
      Accesses after a put cause faults and use after free, missed puts are
      caughts as leaks and double puts are double frees.
      
      This checking helped resolve a memory leak and use after free:
      https://lore.kernel.org/linux-perf-users/CAP-5=fWZH20L4kv-BwVtGLwR=Em3AOOT+Q4QGivvQuYn5AsPRg@mail.gmail.com/Signed-off-by: default avatarIan Rogers <irogers@google.com>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
      Cc: Dmitriy Vyukov <dvyukov@google.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Riccardo Mancini <rickyman7@gmail.com>
      Cc: Stephane Eranian <eranian@google.com>
      Cc: Stephen Brennan <stephen.s.brennan@oracle.com>
      Link: https://lore.kernel.org/lkml/20230407230405.2931830-4-irogers@google.com
      [ Extracted from a larger patch ]
      Signed-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      c35ce1d9
    • Arnaldo Carvalho de Melo's avatar
      perf dso: Add dso__filename_with_chroot() to reduce number of accesses to dso->nsinfo members · 7031edac
      Arnaldo Carvalho de Melo authored
      We'll need to reference count dso->nsinfo, so reduce the number of
      direct accesses by having a shorter form of obtaining a filename with
      a chroot (namespace one).
      
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
      Cc: Dmitriy Vyukov <dvyukov@google.com>
      Cc: Ian Rogers <irogers@google.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Riccardo Mancini <rickyman7@gmail.com>
      Cc: Stephane Eranian <eranian@google.com>
      Cc: Stephen Brennan <stephen.s.brennan@oracle.com>
      Link: https://lore.kernel.org/lkml/ZD26ZlqSbgSyH5lX@kernel.org
      [ Used nsinfo__pid(dso->nsinfo), as it was already present ]
      Signed-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      7031edac
    • Ian Rogers's avatar
      perf cpumap: Add reference count checking · da885a0e
      Ian Rogers authored
      Enabled when REFCNT_CHECKING is defined. The change adds a memory
      allocated pointer that is interposed between the reference counted cpu
      map at a get and freed by a put. The pointer replaces the original
      perf_cpu_map struct, so use of the perf_cpu_map via APIs remains
      unchanged. Any use of the cpu map without the API requires two versions,
      handled via the RC_CHK_ACCESS macro.
      
      This change is intended to catch:
      
       - use after put: using a cpumap after you have put it will cause a
         segv.
       - unbalanced puts: two puts for a get will result in a double free
         that can be captured and reported by tools like address sanitizer,
         including with the associated stack traces of allocation and frees.
       - missing puts: if a put is missing then the get turns into a memory
         leak that can be reported by leak sanitizer, including the stack
         trace at the point the get occurs.
      
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
      Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
      Cc: Andi Kleen <ak@linux.intel.com>
      Cc: Andrew Morton <akpm@linux-foundation.org>
      Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
      Cc: Darren Hart <dvhart@infradead.org>
      Cc: Davidlohr Bueso <dave@stgolabs.net>
      Cc: Dmitriy Vyukov <dvyukov@google.com>
      Cc: Eric Dumazet <edumazet@google.com>
      Cc: German Gomez <german.gomez@arm.com>
      Cc: Hao Luo <haoluo@google.com>
      Cc: Ingo Molnar <mingo@redhat.com>
      Cc: James Clark <james.clark@arm.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: John Garry <john.g.garry@oracle.com>
      Cc: Kajol Jain <kjain@linux.ibm.com>
      Cc: Kan Liang <kan.liang@linux.intel.com>
      Cc: Leo Yan <leo.yan@linaro.org>
      Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
      Cc: Mark Rutland <mark.rutland@arm.com>
      Cc: Masami Hiramatsu <mhiramat@kernel.org>
      Cc: Miaoqian Lin <linmq006@gmail.com>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Cc: Riccardo Mancini <rickyman7@gmail.com>
      Cc: Shunsuke Nakamura <nakamura.shun@fujitsu.com>
      Cc: Song Liu <song@kernel.org>
      Cc: Stephen Brennan <stephen.s.brennan@oracle.com>
      Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
      Cc: Thomas Gleixner <tglx@linutronix.de>
      Cc: Thomas Richter <tmricht@linux.ibm.com>,
      Cc: Yury Norov <yury.norov@gmail.com>
      Link: https://lore.kernel.org/lkml/20230407230405.2931830-3-irogers@google.com
      [ Extracted from a larger patch ]
      Signed-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      da885a0e
    • Ian Rogers's avatar
      perf cpumap: Use perf_cpu_map__cpu(map, cpu) instead of accessing map->map[cpu] directly · 491b13c4
      Ian Rogers authored
      So that we can validate the 'map' instance wrt refcount checking.
      
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
      Cc: Dmitriy Vyukov <dvyukov@google.com>
      Cc: Ian Rogers <irogers@google.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Riccardo Mancini <rickyman7@gmail.com>
      Cc: Stephane Eranian <eranian@google.com>
      Cc: Stephen Brennan <stephen.s.brennan@oracle.com>
      Link: https://lore.kernel.org/lkml/20230407230405.2931830-3-irogers@google.com
      [ Extracted from a larger patch ]
      Signed-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      491b13c4
    • Arnaldo Carvalho de Melo's avatar
      perf cpumap: Remove initializations done in perf_cpu_map__alloc() · d57fd492
      Arnaldo Carvalho de Melo authored
      When extracting this patch from Ian's original patch I forgot to remove
      the setting of ->nr and ->refcnt, no need to do those initializations
      again as those are done in perf_cpu_map__alloc() already, duh.
      
      Cc: Ian Rogers <irogers@google.com>
      Fixes: 1f94479e ("libperf: Make perf_cpu_map__alloc() available as an internal function for tools/perf to use")
      Signed-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      d57fd492
    • Ian Rogers's avatar
      libperf: Add reference count checking macros · a9b867f6
      Ian Rogers authored
      The macros serve as a way to debug use of a reference counted struct.
      
      The macros add a memory allocated pointer that is interposed between
      the reference counted original struct at a get and freed by a put.
      
      The pointer replaces the original struct, so use of the struct name
      via APIs remains unchanged.
      Signed-off-by: default avatarIan Rogers <irogers@google.com>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
      Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
      Cc: Andi Kleen <ak@linux.intel.com>
      Cc: Andrew Morton <akpm@linux-foundation.org>
      Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
      Cc: Darren Hart <dvhart@infradead.org>
      Cc: Davidlohr Bueso <dave@stgolabs.net>
      Cc: Dmitriy Vyukov <dvyukov@google.com>
      Cc: Eric Dumazet <edumazet@google.com>
      Cc: German Gomez <german.gomez@arm.com>
      Cc: Hao Luo <haoluo@google.com>
      Cc: James Clark <james.clark@arm.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: John Garry <john.g.garry@oracle.com>
      Cc: Kajol Jain <kjain@linux.ibm.com>
      Cc: Kan Liang <kan.liang@linux.intel.com>
      Cc: Leo Yan <leo.yan@linaro.org>
      Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
      Cc: Mark Rutland <mark.rutland@arm.com>
      Cc: Masami Hiramatsu <mhiramat@kernel.org>
      Cc: Miaoqian Lin <linmq006@gmail.com>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Cc: Riccardo Mancini <rickyman7@gmail.com>
      Cc: Shunsuke Nakamura <nakamura.shun@fujitsu.com>
      Cc: Song Liu <song@kernel.org>
      Cc: Stephane Eranian <eranian@google.com>
      Cc: Stephen Brennan <stephen.s.brennan@oracle.com>
      Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
      Cc: Thomas Gleixner <tglx@linutronix.de>
      Cc: Thomas Richter <tmricht@linux.ibm.com>
      Cc: Yury Norov <yury.norov@gmail.com>
      Link: http://lore.kernel.org/lkml/20230407230405.2931830-2-irogers@google.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      a9b867f6
    • Arnaldo Carvalho de Melo's avatar
      libperf: Add perf_cpu_map__refcnt() interanl accessor to use in the maps test · 4121234a
      Arnaldo Carvalho de Melo authored
      To remove one more direct access to 'struct perf_cpu_map' so that we can
      intercept accesses to its instantiations and refcount check it to catch
      use after free, etc.
      
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
      Cc: Dmitriy Vyukov <dvyukov@google.com>
      Cc: Ian Rogers <irogers@google.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Riccardo Mancini <rickyman7@gmail.com>
      Cc: Stephane Eranian <eranian@google.com>
      Cc: Stephen Brennan <stephen.s.brennan@oracle.com>
      Link: https://lore.kernel.org/lkml/ZD1qdYjG+DL6KOfP@kernel.orgSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      4121234a
  6. 15 Apr, 2023 1 commit
    • Arnaldo Carvalho de Melo's avatar
      perf test: Simplify for_each_test() to avoid tripping on -Werror=array-bounds · 17354d15
      Arnaldo Carvalho de Melo authored
      When cross building on debian to the mips 32-bit arch we get these
      warnings:
      
        In function '__cmd_test',
            inlined from 'cmd_test' at tests/builtin-test.c:561:9:
        tests/builtin-test.c:260:66: error: array subscript 1 is outside array bounds of 'struct test_suite *[1]' [-Werror=array-bounds]
          260 |                 for (k = 0, t = tests[j][k]; tests[j][k]; k++, t = tests[j][k])
              |                                                                  ^
        tests/builtin-test.c:369:9: note: in expansion of macro 'for_each_test'
          369 |         for_each_test(j, k, t) {
              |         ^~~~~~~~~~~~~
        tests/builtin-test.c: In function 'cmd_test':
        tests/builtin-test.c:36:27: note: at offset 4 into object 'arch_tests' of size 4
           36 | struct test_suite *__weak arch_tests[] = {
              |                           ^~~~~~~~~~
        cc1: all warnings being treated as errors
      
      Switch to using a while(!sentinel) for the second level of the 'tests'
      array to avoid that compiler complaint.
      Signed-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      17354d15
  7. 13 Apr, 2023 11 commits