linux-kernel - Re: [PATCH v3 2/7] perf report: Add parallelism sort key

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACT4Y+Z1hFt6ttPPdoDSxw-BDjBhnhxDAunHQ4do2o56Ne3BDQ@mail.gmail.com>
Date: Wed, 29 Jan 2025 08:18:34 +0100
From: Dmitry Vyukov <dvyukov@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: irogers@...gle.com, linux-perf-users@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Arnaldo Carvalho de Melo <acme@...nel.org>
Subject: Re: [PATCH v3 2/7] perf report: Add parallelism sort key

On Wed, 29 Jan 2025 at 05:42, Namhyung Kim <namhyung@...nel.org> wrote:
>
> On Mon, Jan 27, 2025 at 10:58:49AM +0100, Dmitry Vyukov wrote:
> > Show parallelism level in profiles if requested by user.
> >
> > Signed-off-by: Dmitry Vyukov <dvyukov@...gle.com>
> > Cc: Namhyung Kim <namhyung@...nel.org>
> > Cc: Arnaldo Carvalho de Melo <acme@...nel.org>
> > Cc: Ian Rogers <irogers@...gle.com>
> > Cc: linux-perf-users@...r.kernel.org
> > Cc: linux-kernel@...r.kernel.org
> > ---
> >  tools/perf/builtin-report.c | 11 +++++++++++
> >  tools/perf/util/hist.c      |  2 ++
> >  tools/perf/util/hist.h      |  3 +++
> >  tools/perf/util/session.c   | 12 ++++++++++++
> >  tools/perf/util/session.h   |  1 +
> >  tools/perf/util/sort.c      | 23 +++++++++++++++++++++++
> >  tools/perf/util/sort.h      |  1 +
> >  7 files changed, 53 insertions(+)
> >
> > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> > index 0d9bd090eda71..14d49f0625881 100644
> > --- a/tools/perf/builtin-report.c
> > +++ b/tools/perf/builtin-report.c
> > @@ -1720,6 +1720,17 @@ int cmd_report(int argc, const char **argv)
> >               symbol_conf.annotate_data_sample = true;
> >       }
> >
> > +     if (report.disable_order || !perf_session__has_switch_events(session)) {
> > +             if ((sort_order && strstr(sort_order, "parallelism")) ||
> > +                             (field_order && strstr(field_order, "parallelism"))) {
> > +                     if (report.disable_order)
> > +                             ui__error("Use of parallelism is incompatible with --disable-order.\n");
> > +                     else
> > +                             ui__error("Use of parallelism requires --switch-events during record.\n");
> > +                     return -1;
> > +             }
> > +     }
> > +
> >       if (sort_order && strstr(sort_order, "ipc")) {
> >               parse_options_usage(report_usage, options, "s", 1);
> >               goto error;
> > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> > index 0f30f843c566d..cafd693568189 100644
> > --- a/tools/perf/util/hist.c
> > +++ b/tools/perf/util/hist.c
> > @@ -207,6 +207,7 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
> >
> >       hists__new_col_len(hists, HISTC_CGROUP, 6);
> >       hists__new_col_len(hists, HISTC_CGROUP_ID, 20);
> > +     hists__new_col_len(hists, HISTC_PARALLELISM, 11);
> >       hists__new_col_len(hists, HISTC_CPU, 3);
> >       hists__new_col_len(hists, HISTC_SOCKET, 6);
> >       hists__new_col_len(hists, HISTC_MEM_LOCKED, 6);
> > @@ -741,6 +742,7 @@ __hists__add_entry(struct hists *hists,
> >               .ip      = al->addr,
> >               .level   = al->level,
> >               .code_page_size = sample->code_page_size,
> > +             .parallelism    = al->parallelism,
> >               .stat = {
> >                       .nr_events = 1,
> >                       .period = sample->period,
> > diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> > index 46c8373e31465..a6e662d77dc24 100644
> > --- a/tools/perf/util/hist.h
> > +++ b/tools/perf/util/hist.h
> > @@ -42,6 +42,7 @@ enum hist_column {
> >       HISTC_CGROUP_ID,
> >       HISTC_CGROUP,
> >       HISTC_PARENT,
> > +     HISTC_PARALLELISM,
> >       HISTC_CPU,
> >       HISTC_SOCKET,
> >       HISTC_SRCLINE,
> > @@ -228,6 +229,7 @@ struct hist_entry {
> >       u64                     transaction;
> >       s32                     socket;
> >       s32                     cpu;
> > +     int                     parallelism;
>
> Can you make it u16 and move to around cpumode to remove paddings?

It generally should be the same size as cpu/socket/etc. And these are
32-bits throughout the codebase (the previous fields). Are there any
checks that MAX_NR_CPUS<64K?


> Thanks,
> Namhyung
>
>
> >       u64                     code_page_size;
> >       u64                     weight;
> >       u64                     ins_lat;
> > @@ -580,6 +582,7 @@ bool perf_hpp__is_thread_entry(struct perf_hpp_fmt *fmt);
> >  bool perf_hpp__is_comm_entry(struct perf_hpp_fmt *fmt);
> >  bool perf_hpp__is_dso_entry(struct perf_hpp_fmt *fmt);
> >  bool perf_hpp__is_sym_entry(struct perf_hpp_fmt *fmt);
> > +bool perf_hpp__is_parallelism_entry(struct perf_hpp_fmt *fmt);
> >
> >  struct perf_hpp_fmt *perf_hpp_fmt__dup(struct perf_hpp_fmt *fmt);