Re: [PATCH v5 5/5] perf evlist: Improve default event for s390

[Ian Rogers]

On Tue, Mar 17, 2026 at 12:52 AM Thomas Richter <tmricht@xxxxxxxxxxxxx> wrote:
>
> On 3/17/26 06:53, Ian Rogers wrote:
> > Frame pointer callchains are not supported on s390 and dwarf
> > callchains are only supported on software events.
> >
> > Switch the default event from cycles to cpu-clock or task-clock on
> > s390 if callchains are enabled.
> >
> > If frame pointer callchains are requested on s390 show a
> > warning. Modify the '-g' option of `perf top` and `perf record` to
> > default to dwarf callchains on s390.
> >
> > Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> > ---
> >  tools/perf/builtin-record.c      |  8 ++++++--
> >  tools/perf/builtin-top.c         |  5 +++--
> >  tools/perf/tests/event_update.c  |  4 +++-
> >  tools/perf/tests/expand-cgroup.c |  4 +++-
> >  tools/perf/tests/perf-record.c   |  7 +++++--
> >  tools/perf/tests/topology.c      |  4 +++-
> >  tools/perf/util/evlist.c         | 32 +++++++++++++++++++++-----------
> >  tools/perf/util/evlist.h         |  2 +-
> >  tools/perf/util/evsel.c          |  5 +++++
> >  9 files changed, 50 insertions(+), 21 deletions(-)
> >
> > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > index af1fe6b7c65c..ef97c7a54088 100644
> > --- a/tools/perf/builtin-record.c
> > +++ b/tools/perf/builtin-record.c
> > @@ -55,6 +55,7 @@
> >  #include "asm/bug.h"
> >  #include "perf.h"
> >  #include "cputopo.h"
> > +#include "dwarf-regs.h"
> >
> >  #include <errno.h>
> >  #include <inttypes.h>
> > @@ -2986,7 +2987,9 @@ static int record_callchain_opt(const struct option *opt,
> >                               const char *arg __maybe_unused,
> >                               int unset)
> >  {
> > -     return record_opts__parse_callchain(opt->value, &callchain_param, "fp", unset);
> > +     return record_opts__parse_callchain(opt->value, &callchain_param,
> > +                                         EM_HOST != EM_S390 ? "fp" : "dwarf",
> > +                                         unset);
> >  }
> >
> >
> > @@ -4265,7 +4268,8 @@ int cmd_record(int argc, const char **argv)
> >               record.opts.tail_synthesize = true;
> >
> >       if (rec->evlist->core.nr_entries == 0) {
> > -             struct evlist *def_evlist = evlist__new_default();
> > +             struct evlist *def_evlist = evlist__new_default(&rec->opts.target,
> > +                                                             callchain_param.enabled);
> >
> >               if (!def_evlist)
> >                       goto out;
> > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> > index 2a949d956d0b..84211a78977e 100644
> > --- a/tools/perf/builtin-top.c
> > +++ b/tools/perf/builtin-top.c
> > @@ -56,6 +56,7 @@
> >  #include "util/debug.h"
> >  #include "util/ordered-events.h"
> >  #include "util/pfm.h"
> > +#include "dwarf-regs.h"
> >
> >  #include <assert.h>
> >  #include <elf.h>
> > @@ -1409,7 +1410,7 @@ parse_callchain_opt(const struct option *opt, const char *arg, int unset)
> >  static int
> >  callchain_opt(const struct option *opt, const char *arg __maybe_unused, int unset)
> >  {
> > -     return parse_callchain_opt(opt, "fp", unset);
> > +     return parse_callchain_opt(opt, EM_HOST != EM_S390 ? "fp" : "dwarf", unset);
> >  }
> >
> >
> > @@ -1695,7 +1696,7 @@ int cmd_top(int argc, const char **argv)
> >               goto out_delete_evlist;
> >
> >       if (!top.evlist->core.nr_entries) {
> > -             struct evlist *def_evlist = evlist__new_default();
> > +             struct evlist *def_evlist = evlist__new_default(target, callchain_param.enabled);
> >
> >               if (!def_evlist)
> >                       goto out_delete_evlist;
> > diff --git a/tools/perf/tests/event_update.c b/tools/perf/tests/event_update.c
> > index cb9e6de2e033..facc65e29f20 100644
> > --- a/tools/perf/tests/event_update.c
> > +++ b/tools/perf/tests/event_update.c
> > @@ -8,6 +8,7 @@
> >  #include "header.h"
> >  #include "machine.h"
> >  #include "util/synthetic-events.h"
> > +#include "target.h"
> >  #include "tool.h"
> >  #include "tests.h"
> >  #include "debug.h"
> > @@ -81,7 +82,8 @@ static int test__event_update(struct test_suite *test __maybe_unused, int subtes
> >  {
> >       struct evsel *evsel;
> >       struct event_name tmp;
> > -     struct evlist *evlist = evlist__new_default();
> > +     struct target target = {};
> > +     struct evlist *evlist = evlist__new_default(&target, /*sample_callchains=*/false);
> >
> >       TEST_ASSERT_VAL("failed to get evlist", evlist);
> >
> > diff --git a/tools/perf/tests/expand-cgroup.c b/tools/perf/tests/expand-cgroup.c
> > index c7b32a220ca1..dd547f2f77cc 100644
> > --- a/tools/perf/tests/expand-cgroup.c
> > +++ b/tools/perf/tests/expand-cgroup.c
> > @@ -8,6 +8,7 @@
> >  #include "parse-events.h"
> >  #include "pmu-events/pmu-events.h"
> >  #include "pfm.h"
> > +#include "target.h"
> >  #include <subcmd/parse-options.h>
> >  #include <stdio.h>
> >  #include <stdlib.h>
> > @@ -99,7 +100,8 @@ out:       for (i = 0; i < nr_events; i++)
> >  static int expand_default_events(void)
> >  {
> >       int ret;
> > -     struct evlist *evlist = evlist__new_default();
> > +     struct target target = {};
> > +     struct evlist *evlist = evlist__new_default(&target, /*sample_callchains=*/false);
> >
> >       TEST_ASSERT_VAL("failed to get evlist", evlist);
> >
> > diff --git a/tools/perf/tests/perf-record.c b/tools/perf/tests/perf-record.c
> > index efbd9cd60c63..c6e31ab8a6b8 100644
> > --- a/tools/perf/tests/perf-record.c
> > +++ b/tools/perf/tests/perf-record.c
> > @@ -84,8 +84,11 @@ static int test__PERF_RECORD(struct test_suite *test __maybe_unused, int subtest
> >       CPU_ZERO_S(cpu_mask_size, cpu_mask);
> >
> >       perf_sample__init(&sample, /*all=*/false);
> > -     if (evlist == NULL) /* Fallback for kernels lacking PERF_COUNT_SW_DUMMY */
> > -             evlist = evlist__new_default();
> > +     if (evlist == NULL) { /* Fallback for kernels lacking PERF_COUNT_SW_DUMMY */
> > +             struct target target = {};
> > +
> > +             evlist = evlist__new_default(&target, /*sample_callchains=*/false);
> > +     }
> >
> >       if (evlist == NULL) {
> >               pr_debug("Not enough memory to create evlist\n");
> > diff --git a/tools/perf/tests/topology.c b/tools/perf/tests/topology.c
> > index ec01150d208d..a34a7ab19a80 100644
> > --- a/tools/perf/tests/topology.c
> > +++ b/tools/perf/tests/topology.c
> > @@ -9,6 +9,7 @@
> >  #include "evlist.h"
> >  #include "debug.h"
> >  #include "pmus.h"
> > +#include "target.h"
> >  #include <linux/err.h>
> >
> >  #define TEMPL "/tmp/perf-test-XXXXXX"
> > @@ -37,11 +38,12 @@ static int session_write_header(char *path)
> >               .path = path,
> >               .mode = PERF_DATA_MODE_WRITE,
> >       };
> > +     struct target target = {};
> >
> >       session = perf_session__new(&data, NULL);
> >       TEST_ASSERT_VAL("can't get session", !IS_ERR(session));
> >
> > -     session->evlist = evlist__new_default();
> > +     session->evlist = evlist__new_default(&target, /*sample_callchains=*/false);
> >       TEST_ASSERT_VAL("can't get evlist", session->evlist);
> >       session->evlist->session = session;
> >
> > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> > index 591bdf0b3e2a..c702741a9173 100644
> > --- a/tools/perf/util/evlist.c
> > +++ b/tools/perf/util/evlist.c
> > @@ -13,6 +13,7 @@
> >  #include "util/mmap.h"
> >  #include "thread_map.h"
> >  #include "target.h"
> > +#include "dwarf-regs.h"
> >  #include "evlist.h"
> >  #include "evsel.h"
> >  #include "record.h"
> > @@ -98,38 +99,47 @@ struct evlist *evlist__new(void)
> >       return evlist;
> >  }
> >
> > -struct evlist *evlist__new_default(void)
> > +struct evlist *evlist__new_default(const struct target *target, bool sample_callchains)
> >  {
> >       struct evlist *evlist = evlist__new();
> >       bool can_profile_kernel;
> >       struct perf_pmu *pmu = NULL;
> > +     struct evsel *evsel;
> > +     char buf[256];
> > +     int err;
> >
> >       if (!evlist)
> >               return NULL;
> >
> >       can_profile_kernel = perf_event_paranoid_check(1);
> >
> > -     while ((pmu = perf_pmus__scan_core(pmu)) != NULL) {
> > -             char buf[256];
> > -             int err;
> > -
> > -             snprintf(buf, sizeof(buf), "%s/cycles/%s", pmu->name,
> > +     if (EM_HOST == EM_S390 && sample_callchains) {
> > +             snprintf(buf, sizeof(buf), "software/%s/%s",
> > +                      target__has_cpu(target) ? "cpu-clock" : "task-clock",
> >                        can_profile_kernel ? "P" : "Pu");
> >               err = parse_event(evlist, buf);
> > -             if (err) {
> > -                     evlist__delete(evlist);
> > -                     return NULL;
> > +             if (err)
> > +                     goto out_err;
> > +     } else {
> > +             while ((pmu = perf_pmus__scan_core(pmu)) != NULL) {
> > +                     snprintf(buf, sizeof(buf), "%s/cycles/%s", pmu->name,
> > +                             can_profile_kernel ? "P" : "Pu");
> > +                     err = parse_event(evlist, buf);
> > +                     if (err)
> > +                             goto out_err;
> >               }
> >       }
> >
> > +     /* If there is only 1 event a sample identifier isn't necessary. */
> >       if (evlist->core.nr_entries > 1) {
> > -             struct evsel *evsel;
> > -
> >               evlist__for_each_entry(evlist, evsel)
> >                       evsel__set_sample_id(evsel, /*can_sample_identifier=*/false);
> >       }
> >
> >       return evlist;
> > +out_err:
> > +     evlist__delete(evlist);
> > +     return NULL;
> >  }
> >
> >  struct evlist *evlist__new_dummy(void)
> > diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> > index d17c3b57a409..e507f5f20ef6 100644
> > --- a/tools/perf/util/evlist.h
> > +++ b/tools/perf/util/evlist.h
> > @@ -104,7 +104,7 @@ struct evsel_str_handler {
> >  };
> >
> >  struct evlist *evlist__new(void);
> > -struct evlist *evlist__new_default(void);
> > +struct evlist *evlist__new_default(const struct target *target, bool sample_callchains);
> >  struct evlist *evlist__new_dummy(void);
> >  void evlist__init(struct evlist *evlist, struct perf_cpu_map *cpus,
> >                 struct perf_thread_map *threads);
> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > index 54c8922a8e47..5a294595a677 100644
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> > @@ -1021,6 +1021,11 @@ static void __evsel__config_callchain(struct evsel *evsel, const struct record_o
> >       bool function = evsel__is_function_event(evsel);
> >       struct perf_event_attr *attr = &evsel->core.attr;
> >
> > +     if (EM_HOST == EM_S390 && param->record_mode == CALLCHAIN_FP) {
> > +             pr_warning_once(
> > +                     "Framepointer unwinding lacks kernel support. Use '--call-graph dwarf'\n");
> > +     }
> > +
> >       evsel__set_sample_bit(evsel, CALLCHAIN);
> >
> >       attr->sample_max_stack = param->max_stack;
>
> Great, here is the output on my LPAR. Thanks very much fpr addressing and fixing this!!!
>
> root in 🌐 b83lp69 in mirror-linux-next/tools/perf on  master [⇡] via C v15.2.1-gcc took 2s
> ❯ ./perf record  --call-graph dwarf  -- perf test -w noploop
> [ perf record: Woken up 133 times to write data ]
> [ perf record: Captured and wrote 32.928 MB perf.data (4039 samples) ]
>
> root in 🌐 b83lp69 in mirror-linux-next/tools/perf on  master [⇡] via C v15.2.1-gcc took 2s
> ❯ ./perf evlist
> software/task-clock/P
>
> root in 🌐 b83lp69 in mirror-linux-next/tools/perf on  master [⇡] via C v15.2.1-gcc
> ❯ ./perf record  -g -- perf test -w noploop
> [ perf record: Woken up 133 times to write data ]
> [ perf record: Captured and wrote 32.952 MB perf.data (4042 samples) ]
>
> root in 🌐 b83lp69 in mirror-linux-next/tools/perf on  master [⇡] via C v15.2.1-gcc took 2s
> ❯ ./perf evlist
> software/task-clock/P
>
> root in 🌐 b83lp69 in mirror-linux-next/tools/perf on  master [⇡] via C v15.2.1-gcc
> ❯ ./perf record  -- perf test -w noploop
> [ perf record: Woken up 2 times to write data ]
> [ perf record: Captured and wrote 0.179 MB perf.data (3974 samples) ]
>
> root in 🌐 b83lp69 in mirror-linux-next/tools/perf on  master [⇡] via C v15.2.1-gcc took 2s
> ❯ ./perf evlist
> cpum_cf/cycles/P
>
> root in 🌐 b83lp69 in mirror-linux-next/tools/perf on  master [⇡] via C v15.2.1-gcc
> ❯ ./perf record
> ^C[ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.250 MB perf.data (24 samples) ]
>
>
> root in 🌐 b83lp69 in mirror-linux-next/tools/perf on  master [⇡] via C v15.2.1-gcc took 3s
> ❯ ./perf evlist
> cpum_cf/cycles/P
> dummy:u
>
> root in 🌐 b83lp69 in mirror-linux-next/tools/perf on  master [⇡] via C v15.2.1-gcc
> ❯ ./perf record  -g
> ^C[ perf record: Woken up 8 times to write data ]
> [ perf record: Captured and wrote 5.186 MB perf.data (34255 samples) ]
>
>
> root in 🌐 b83lp69 in mirror-linux-next/tools/perf on  master [⇡] via C v15.2.1-gcc took 3s
> ❯ ./perf evlist
> software/cpu-clock/P
> dummy:u
>
>
> root in 🌐 b83lp69 in mirror-linux-next/tools/perf on  master [⇡] via C v15.2.1-gcc took 3s
>
> Tested-by:  Thomas Richter <tmricht@xxxxxxxxxxxxx>

Thanks Thomas! I'll add the tag into the next version. Why a new
version? I'm trying to appease sashiko that is rightly flagging more
issues:
https://sashiko.dev/#/patchset/20260317055334.760347-1-irogers%40google.com

Thanks,
Ian

> --
> Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
> --
> IBM Deutschland Research & Development GmbH
>
> Vorsitzender des Aufsichtsrats: Wolfgang Wendt
>
> Geschäftsführung: David Faller
>
> Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294