[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fVuDXjWq85G3Q0RNQjbQE2Y7Sh+JSK_g+dBK9zEj=vjUg@mail.gmail.com>
Date: Wed, 8 May 2024 14:42:31 -0700
From: Ian Rogers <irogers@...gle.com>
To: Dominique Martinet <asmadeus@...ewreck.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, Jiri Olsa <jolsa@...nel.org>,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Adrian Hunter <adrian.hunter@...el.com>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] perf parse-events: Add new 'fake_tp' parameter for tests
On Sun, May 5, 2024 at 4:14 AM Dominique Martinet
<asmadeus@...ewreck.org> wrote:
>
> The next commit will allow tracepoints starting with digits, but most
> systems do not have any available by default so tests should skip the
> actual "check if it exists in /sys/kernel/debug/tracing" step.
>
> In order to do that, add a new boolean flag specifying if we should
> actually "format" the probe or not.
>
> Originally-by: Jiri Olsa <jolsa@...nel.org>
> Signed-off-by: Dominique Martinet <asmadeus@...ewreck.org>
Reviewed-by: Ian Rogers <irogers@...gle.com>
Thanks,
Ian
> ---
> tools/perf/tests/parse-events.c | 6 ++++--
> tools/perf/tests/pmu-events.c | 2 +-
> tools/perf/util/evlist.c | 3 ++-
> tools/perf/util/evsel.c | 20 +++++++++++++-------
> tools/perf/util/evsel.h | 4 ++--
> tools/perf/util/metricgroup.c | 3 ++-
> tools/perf/util/parse-events.c | 9 ++++++---
> tools/perf/util/parse-events.h | 6 ++++--
> 8 files changed, 34 insertions(+), 19 deletions(-)
>
> diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> index feb5727584d1..ef056e8740fe 100644
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -2504,7 +2504,8 @@ static int test_event(const struct evlist_test *e)
> return TEST_FAIL;
> }
> parse_events_error__init(&err);
> - ret = parse_events(evlist, e->name, &err);
> + ret = __parse_events(evlist, e->name, /*pmu_filter=*/NULL, &err, /*fake_pmu=*/NULL,
> + /*warn_if_reordered=*/true, /*fake_tp=*/true);
> if (ret) {
> pr_debug("failed to parse event '%s', err %d\n", e->name, ret);
> parse_events_error__print(&err, e->name);
> @@ -2532,7 +2533,8 @@ static int test_event_fake_pmu(const char *str)
>
> parse_events_error__init(&err);
> ret = __parse_events(evlist, str, /*pmu_filter=*/NULL, &err,
> - &perf_pmu__fake, /*warn_if_reordered=*/true);
> + &perf_pmu__fake, /*warn_if_reordered=*/true,
> + /*fake_tp=*/true);
> if (ret) {
> pr_debug("failed to parse event '%s', err %d\n",
> str, ret);
> diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
> index 47a7c3277540..0a1308d84e9e 100644
> --- a/tools/perf/tests/pmu-events.c
> +++ b/tools/perf/tests/pmu-events.c
> @@ -842,7 +842,7 @@ static int check_parse_id(const char *id, struct parse_events_error *error,
> *cur = '/';
>
> ret = __parse_events(evlist, dup, /*pmu_filter=*/NULL, error, fake_pmu,
> - /*warn_if_reordered=*/true);
> + /*warn_if_reordered=*/true, /*fake_tp=*/false);
> free(dup);
>
> evlist__delete(evlist);
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 55a300a0977b..3a719edafc7a 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -298,7 +298,8 @@ struct evsel *evlist__add_aux_dummy(struct evlist *evlist, bool system_wide)
> #ifdef HAVE_LIBTRACEEVENT
> struct evsel *evlist__add_sched_switch(struct evlist *evlist, bool system_wide)
> {
> - struct evsel *evsel = evsel__newtp_idx("sched", "sched_switch", 0);
> + struct evsel *evsel = evsel__newtp_idx("sched", "sched_switch", 0,
> + /*format=*/true);
>
> if (IS_ERR(evsel))
> return evsel;
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 3536404e9447..4f818ab6b662 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -452,7 +452,7 @@ struct evsel *evsel__clone(struct evsel *orig)
> * Returns pointer with encoded error via <linux/err.h> interface.
> */
> #ifdef HAVE_LIBTRACEEVENT
> -struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx)
> +struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx, bool format)
> {
> struct evsel *evsel = zalloc(perf_evsel__object.size);
> int err = -ENOMEM;
> @@ -469,14 +469,20 @@ struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx)
> if (asprintf(&evsel->name, "%s:%s", sys, name) < 0)
> goto out_free;
>
> - evsel->tp_format = trace_event__tp_format(sys, name);
> - if (IS_ERR(evsel->tp_format)) {
> - err = PTR_ERR(evsel->tp_format);
> - goto out_free;
> + event_attr_init(&attr);
> +
> + if (format) {
> + evsel->tp_format = trace_event__tp_format(sys, name);
> + if (IS_ERR(evsel->tp_format)) {
> + err = PTR_ERR(evsel->tp_format);
> + goto out_free;
> + }
> + attr.config = evsel->tp_format->id;
> + } else {
> + attr.config = (__u64) -1;
> }
>
> - event_attr_init(&attr);
> - attr.config = evsel->tp_format->id;
> +
> attr.sample_period = 1;
> evsel__init(evsel, &attr, idx);
> }
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 517cff431de2..375a38e15cd9 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -234,14 +234,14 @@ void free_config_terms(struct list_head *config_terms);
>
>
> #ifdef HAVE_LIBTRACEEVENT
> -struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx);
> +struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx, bool format);
>
> /*
> * Returns pointer with encoded error via <linux/err.h> interface.
> */
> static inline struct evsel *evsel__newtp(const char *sys, const char *name)
> {
> - return evsel__newtp_idx(sys, name, 0);
> + return evsel__newtp_idx(sys, name, 0, true);
> }
> #endif
>
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 79ef6095ab28..c21f05f8f255 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -1502,7 +1502,8 @@ static int parse_ids(bool metric_no_merge, struct perf_pmu *fake_pmu,
> pr_debug("Parsing metric events '%s'\n", events.buf);
> parse_events_error__init(&parse_error);
> ret = __parse_events(parsed_evlist, events.buf, /*pmu_filter=*/NULL,
> - &parse_error, fake_pmu, /*warn_if_reordered=*/false);
> + &parse_error, fake_pmu, /*warn_if_reordered=*/false,
> + /*fake_tp=*/false);
> if (ret) {
> parse_events_error__print(&parse_error, events.buf);
> goto err_out;
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 6e8cba03f0ac..04508e07569d 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -526,7 +526,8 @@ static int add_tracepoint(struct parse_events_state *parse_state,
> struct parse_events_terms *head_config, void *loc_)
> {
> YYLTYPE *loc = loc_;
> - struct evsel *evsel = evsel__newtp_idx(sys_name, evt_name, parse_state->idx++);
> + struct evsel *evsel = evsel__newtp_idx(sys_name, evt_name, parse_state->idx++,
> + !parse_state->fake_tp);
>
> if (IS_ERR(evsel)) {
> tracepoint_error(err, PTR_ERR(evsel), sys_name, evt_name, loc->first_column);
> @@ -2126,7 +2127,7 @@ static int parse_events__sort_events_and_fix_groups(struct list_head *list)
>
> int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filter,
> struct parse_events_error *err, struct perf_pmu *fake_pmu,
> - bool warn_if_reordered)
> + bool warn_if_reordered, bool fake_tp)
> {
> struct parse_events_state parse_state = {
> .list = LIST_HEAD_INIT(parse_state.list),
> @@ -2134,6 +2135,7 @@ int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filte
> .error = err,
> .stoken = PE_START_EVENTS,
> .fake_pmu = fake_pmu,
> + .fake_tp = fake_tp,
> .pmu_filter = pmu_filter,
> .match_legacy_cache_terms = true,
> };
> @@ -2343,7 +2345,8 @@ int parse_events_option(const struct option *opt, const char *str,
>
> parse_events_error__init(&err);
> ret = __parse_events(*args->evlistp, str, args->pmu_filter, &err,
> - /*fake_pmu=*/NULL, /*warn_if_reordered=*/true);
> + /*fake_pmu=*/NULL, /*warn_if_reordered=*/true,
> + /*fake_tp=*/false);
>
> if (ret) {
> parse_events_error__print(&err, str);
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index fd55a154ceff..b985a821546b 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -32,14 +32,14 @@ int parse_events_option_new_evlist(const struct option *opt, const char *str, in
> __attribute__((nonnull(1, 2, 4)))
> int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filter,
> struct parse_events_error *error, struct perf_pmu *fake_pmu,
> - bool warn_if_reordered);
> + bool warn_if_reordered, bool fake_tp);
>
> __attribute__((nonnull(1, 2, 3)))
> static inline int parse_events(struct evlist *evlist, const char *str,
> struct parse_events_error *err)
> {
> return __parse_events(evlist, str, /*pmu_filter=*/NULL, err, /*fake_pmu=*/NULL,
> - /*warn_if_reordered=*/true);
> + /*warn_if_reordered=*/true, /*fake_tp=*/false);
> }
>
> int parse_event(struct evlist *evlist, const char *str);
> @@ -152,6 +152,8 @@ struct parse_events_state {
> int stoken;
> /* Special fake PMU marker for testing. */
> struct perf_pmu *fake_pmu;
> + /* Skip actual tracepoint processing for testing. */
> + bool fake_tp;
> /* If non-null, when wildcard matching only match the given PMU. */
> const char *pmu_filter;
> /* Should PE_LEGACY_NAME tokens be generated for config terms? */
>
> --
> 2.44.0
>
Powered by blists - more mailing lists