All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ian Rogers <irogers@google.com>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	Jiri Olsa <jolsa@redhat.com>, Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Stephane Eranian <eranian@google.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/3] perf list: Add 'pfm' to list libpfm4 events
Date: Tue, 22 Sep 2020 13:50:05 -0700	[thread overview]
Message-ID: <CAP-5=fVT1SMB2G91SS28UVwXggZmYAGaqOPp2ozPoA=Yf0P3Ww@mail.gmail.com> (raw)
In-Reply-To: <20200909055849.469612-3-namhyung@kernel.org>

On Tue, Sep 8, 2020 at 10:59 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Print libpfm4 events with 'perf list pfm' command like others.
> When libpfm4 support is not enabled, it'd print nothing.
> Also it support glob pattern matching for event name.
>
>   $ perf list pfm
>
>   List of pre-defined events (to be used in --pfm-events):
>
>   ix86arch:
>     UNHALTED_CORE_CYCLES
>       [count core clock cycles whenever the clock signal ...
>     INSTRUCTION_RETIRED
>       [count the number of instructions at retirement. ...
>     ...
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index 10ab5e40a34f..167868053fe0 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -14,6 +14,7 @@
>  #include "util/pmu.h"
>  #include "util/debug.h"
>  #include "util/metricgroup.h"
> +#include "util/pfm.h"
>  #include <subcmd/pager.h>
>  #include <subcmd/parse-options.h>
>  #include <stdio.h>
> @@ -42,7 +43,7 @@ int cmd_list(int argc, const char **argv)
>                 OPT_END()
>         };
>         const char * const list_usage[] = {
> -               "perf list [<options>] [hw|sw|cache|tracepoint|pmu|sdt|metric|metricgroup|event_glob]",
> +               "perf list [<options>] [hw|sw|cache|tracepoint|pmu|sdt|metric|metricgroup|pfm|event_glob]",

Should this be "#ifdef HAVE_LIBPFM" to avoid advertising pfm events
when support isn't compiled in?

Thanks,
Ian

>                 NULL
>         };
>
> @@ -53,7 +54,7 @@ int cmd_list(int argc, const char **argv)
>
>         setup_pager();
>
> -       if (!raw_dump && pager_in_use())
> +       if (!raw_dump && pager_in_use() && (argc != 1 || strcmp(argv[0], "pfm")))
>                 printf("\nList of pre-defined events (to be used in -e):\n\n");
>
>         if (argc == 0) {
> @@ -89,6 +90,8 @@ int cmd_list(int argc, const char **argv)
>                         metricgroup__print(true, false, NULL, raw_dump, details_flag);
>                 else if (strcmp(argv[i], "metricgroup") == 0 || strcmp(argv[i], "metricgroups") == 0)
>                         metricgroup__print(false, true, NULL, raw_dump, details_flag);
> +               else if (strcmp(argv[i], "pfm") == 0)
> +                       print_libpfm_events(NULL, raw_dump, long_desc_flag);
>                 else if ((sep = strchr(argv[i], ':')) != NULL) {
>                         int sep_idx;
>
> @@ -120,6 +123,7 @@ int cmd_list(int argc, const char **argv)
>                         print_tracepoint_events(NULL, s, raw_dump);
>                         print_sdt_events(NULL, s, raw_dump);
>                         metricgroup__print(true, true, s, raw_dump, details_flag);
> +                       print_libpfm_events(s, raw_dump, long_desc_flag);
>                         free(s);
>                 }
>         }
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 772f1057647f..ae8ab930a792 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -2593,7 +2593,7 @@ static struct option __record_options[] = {
>                      "number of threads to run for event synthesis"),
>  #ifdef HAVE_LIBPFM
>         OPT_CALLBACK(0, "pfm-events", &record.evlist, "event",
> -               "libpfm4 event selector. use 'perf list' to list available events",
> +               "libpfm4 event selector. use 'perf list pfm' to list available events",
>                 parse_libpfm_events_option),
>  #endif
>         OPT_CALLBACK(0, "control", &record.opts, "fd:ctl-fd[,ack-fd]",
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 483a28ef4ec4..a672d2b68e8a 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -1166,7 +1166,7 @@ static struct option stat_options[] = {
>                     "threads of same physical core"),
>  #ifdef HAVE_LIBPFM
>         OPT_CALLBACK(0, "pfm-events", &evsel_list, "event",
> -               "libpfm4 event selector. use 'perf list' to list available events",
> +               "libpfm4 event selector. use 'perf list pfm' to list available events",
>                 parse_libpfm_events_option),
>  #endif
>         OPT_CALLBACK(0, "control", &stat_config, "fd:ctl-fd[,ack-fd]",
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 7c64134472c7..d6adc7d34210 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -1578,7 +1578,7 @@ int cmd_top(int argc, const char **argv)
>                     "Enable LBR callgraph stitching approach"),
>  #ifdef HAVE_LIBPFM
>         OPT_CALLBACK(0, "pfm-events", &top.evlist, "event",
> -               "libpfm4 event selector. use 'perf list' to list available events",
> +               "libpfm4 event selector. use 'perf list pfm' to list available events",
>                 parse_libpfm_events_option),
>  #endif
>         OPTS_EVSWITCH(&top.evswitch),
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index c4d2394e2b2d..2d426a4f3bc7 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -2899,7 +2899,7 @@ void print_events(const char *event_glob, bool name_only, bool quiet_flag,
>
>         metricgroup__print(true, true, NULL, name_only, details_flag);
>
> -       print_libpfm_events(name_only, long_desc);
> +       print_libpfm_events(NULL, name_only, long_desc);
>  }
>
>  int parse_events__is_hardcoded_term(struct parse_events_term *term)
> diff --git a/tools/perf/util/pfm.c b/tools/perf/util/pfm.c
> index d735acb6c29c..26ae2c8c0932 100644
> --- a/tools/perf/util/pfm.c
> +++ b/tools/perf/util/pfm.c
> @@ -12,6 +12,7 @@
>  #include "util/parse-events.h"
>  #include "util/pmu.h"
>  #include "util/pfm.h"
> +#include "util/string2.h"
>
>  #include <string.h>
>  #include <linux/kernel.h>
> @@ -227,7 +228,7 @@ print_libpfm_events_raw(pfm_pmu_info_t *pinfo, pfm_event_info_t *info)
>                 printf("%s::%s\n", pinfo->name, info->name);
>  }
>
> -void print_libpfm_events(bool name_only, bool long_desc)
> +void print_libpfm_events(const char *event_glob, bool name_only, bool long_desc)
>  {
>         pfm_event_info_t info;
>         pfm_pmu_info_t pinfo;
> @@ -265,6 +266,9 @@ void print_libpfm_events(bool name_only, bool long_desc)
>                         if (ret != PFM_SUCCESS)
>                                 continue;
>
> +                       if (event_glob && !strglobmatch_nocase(info.name, event_glob))
> +                               continue;
> +
>                         if (!name_only && !printed_pmu) {
>                                 printf("%s:\n", pinfo.name);
>                                 printed_pmu = true;
> diff --git a/tools/perf/util/pfm.h b/tools/perf/util/pfm.h
> index 7d70dda87012..036e2d97b260 100644
> --- a/tools/perf/util/pfm.h
> +++ b/tools/perf/util/pfm.h
> @@ -13,7 +13,7 @@
>  int parse_libpfm_events_option(const struct option *opt, const char *str,
>                         int unset);
>
> -void print_libpfm_events(bool name_only, bool long_desc);
> +void print_libpfm_events(const char *event_glob, bool name_only, bool long_desc);
>
>  #else
>  #include <linux/compiler.h>
> @@ -26,7 +26,8 @@ static inline int parse_libpfm_events_option(
>         return 0;
>  }
>
> -static inline void print_libpfm_events(bool name_only __maybe_unused,
> +static inline void print_libpfm_events(const char *event_glob __maybe_unused,
> +                                      bool name_only __maybe_unused,
>                                        bool long_desc __maybe_unused)
>  {
>  }
> --
> 2.28.0.526.ge36021eeef-goog
>

  parent reply	other threads:[~2020-09-22 20:50 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-09  5:58 [PATCH 1/3] perf list: Remove dead code in argument check Namhyung Kim
2020-09-09  5:58 ` [PATCH 2/3] perf list: Do not print 'Metric Groups:' unnecessarily Namhyung Kim
2020-09-09 12:29   ` Arnaldo Carvalho de Melo
2020-09-09  5:58 ` [PATCH 3/3] perf list: Add 'pfm' to list libpfm4 events Namhyung Kim
2020-09-21  6:34   ` Namhyung Kim
2020-09-22 20:42   ` Jiri Olsa
2020-09-22 22:42     ` Namhyung Kim
2020-09-23  5:10       ` Jiri Olsa
2020-09-22 20:50   ` Ian Rogers [this message]
2020-09-22 22:45     ` Namhyung Kim
2020-09-09 12:27 ` [PATCH 1/3] perf list: Remove dead code in argument check Arnaldo Carvalho de Melo
2020-09-09 12:58   ` Namhyung Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAP-5=fVT1SMB2G91SS28UVwXggZmYAGaqOPp2ozPoA=Yf0P3Ww@mail.gmail.com' \
    --to=irogers@google.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=eranian@google.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.