| 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
| ||
|
Message-ID: <ZypZpjh1bDDqpxn6@x1>
Date: Tue, 5 Nov 2024 14:45:10 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: 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>,
Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Kan Liang <kan.liang@...ux.intel.com>,
Athira Jajeev <atrajeev@...ux.vnet.ibm.com>,
James Clark <james.clark@...aro.org>,
Dominique Martinet <asmadeus@...ewreck.org>,
Yang Li <yang.lee@...ux.alibaba.com>,
Colin Ian King <colin.i.king@...il.com>,
Yang Jihong <yangjihong@...edance.com>,
"Steinar H. Gunderson" <sesse@...gle.com>,
Oliver Upton <oliver.upton@...ux.dev>,
Ilkka Koskinen <ilkka@...amperecomputing.com>,
Ze Gao <zegao2021@...il.com>, Weilin Wang <weilin.wang@...el.com>,
Ben Gainey <ben.gainey@....com>,
zhaimingbing <zhaimingbing@...s.chinamobile.com>,
Zixian Cai <fzczx123@...il.com>, Andi Kleen <ak@...ux.intel.com>,
Paran Lee <p4ranlee@...il.com>,
Thomas Falcon <thomas.falcon@...el.com>,
linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
"Steven Rostedt (Google)" <rostedt@...dmis.org>
Subject: Re: [PATCH v2 5/6] perf evsel: Allow evsel__newtp without
libtraceevent
On Sat, Nov 02, 2024 at 09:53:59AM -0700, Ian Rogers wrote:
> Switch from reading the tracepoint format to reading the id directly
> for the evsel config. This avoids the need to initialize
> libtraceevent, plugins, etc. It is sufficient for many tracepoint
> commands to work like:
> $ perf stat -e sched:sched_switch true
root@x1:~# perf check feature libtraceevent
libtraceevent: [ OFF ] # HAVE_LIBTRACEEVENT
root@x1:~# perf stat -e sched:sched_switch -I 1000
# time counts unit events
1.001071607 4,631 sched:sched_switch
2.004003548 5,138 sched:sched_switch
3.006847313 4,967 sched:sched_switch
^C 3.944095456 5,553 sched:sched_switch
root@x1:~#
Works:
Tested-by: Arnaldo Carvalho de Melo <acme@...hat.com>
> To populate evsel->tp_format, do lazy initialization using
> libtraceevent in the evsel__tp_format function (the sys and name are
> saved in evsel__newtp_idx for this purpose). Reading the id should be
> indicative of the format failing to load, but if not an error is
> reported in evsel__tp_format. This could happen for a tracepoint with
> a format that fails to parse.
>
> As tracepoints can be parsed without libtraceevent with this, remove
> the associated #ifdefs in parse-events.c.
>
> By only lazily parsing the tracepoint format information it is hoped
> this will help improve the performance of code using tracepoints but
> not the format information. It also cuts down on the build and ifdef
> logic.
>
> Signed-off-by: Ian Rogers <irogers@...gle.com>
> ---
> tools/perf/util/evsel.c | 103 ++++++++++++++++++++++++---------
> tools/perf/util/evsel.h | 14 ++---
> tools/perf/util/parse-events.c | 16 +----
> 3 files changed, 82 insertions(+), 51 deletions(-)
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index a95db7e900d5..56e19e32a852 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -543,54 +543,101 @@ struct evsel *evsel__clone(struct evsel *orig)
> return NULL;
> }
>
> +static int trace_event__id(const char *sys, const char *name)
> +{
> + char *tp_dir = get_events_file(sys);
> + char path[PATH_MAX];
> + int id, err;
> +
> + if (!tp_dir)
> + return -1;
> +
> + scnprintf(path, PATH_MAX, "%s/%s/id", tp_dir, name);
> + put_events_file(tp_dir);
> + err = filename__read_int(path, &id);
> + if (err)
> + return err;
> +
> + return id;
> +}
> +
> /*
> * 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, bool format)
> {
> + struct perf_event_attr attr = {
> + .type = PERF_TYPE_TRACEPOINT,
> + .sample_type = (PERF_SAMPLE_RAW | PERF_SAMPLE_TIME |
> + PERF_SAMPLE_CPU | PERF_SAMPLE_PERIOD),
> + };
> struct evsel *evsel = zalloc(perf_evsel__object.size);
> - int err = -ENOMEM;
> + int err = -ENOMEM, id = -1;
>
> - if (evsel == NULL) {
> + if (evsel == NULL)
> goto out_err;
> - } else {
> - struct perf_event_attr attr = {
> - .type = PERF_TYPE_TRACEPOINT,
> - .sample_type = (PERF_SAMPLE_RAW | PERF_SAMPLE_TIME |
> - PERF_SAMPLE_CPU | PERF_SAMPLE_PERIOD),
> - };
>
> - if (asprintf(&evsel->name, "%s:%s", sys, name) < 0)
> - goto out_free;
>
> - event_attr_init(&attr);
> + if (asprintf(&evsel->name, "%s:%s", sys, name) < 0)
> + goto out_free;
>
> - if (format) {
> - evsel->tp_format = trace_event__tp_format(sys, name);
> - if (IS_ERR(evsel->tp_format)) {
> - err = PTR_ERR(evsel->tp_format);
> - evsel->tp_format = NULL;
> - goto out_free;
> - }
> - attr.config = evsel->tp_format->id;
> - } else {
> - attr.config = (__u64) -1;
> - }
> +#ifdef HAVE_LIBTRACEEVENT
> + evsel->tp_sys = strdup(sys);
> + if (!evsel->tp_sys)
> + goto out_free;
>
> + evsel->tp_name = strdup(name);
> + if (!evsel->tp_name)
> + goto out_free;
> +#endif
>
> - attr.sample_period = 1;
> - evsel__init(evsel, &attr, idx);
> - }
> + event_attr_init(&attr);
>
> + if (format) {
> + id = trace_event__id(sys, name);
> + if (id < 0) {
> + err = id;
> + goto out_free;
> + }
> + }
> + attr.config = (__u64)id;
> + attr.sample_period = 1;
> + evsel__init(evsel, &attr, idx);
> return evsel;
>
> out_free:
> zfree(&evsel->name);
> +#ifdef HAVE_LIBTRACEEVENT
> + zfree(&evsel->tp_sys);
> + zfree(&evsel->tp_name);
> +#endif
> free(evsel);
> out_err:
> return ERR_PTR(err);
> }
> +
> +#ifdef HAVE_LIBTRACEEVENT
> +struct tep_event *evsel__tp_format(struct evsel *evsel)
> +{
> + struct tep_event *tp_format = evsel->tp_format;
> +
> + if (tp_format)
> + return tp_format;
> +
> + if (evsel->core.attr.type != PERF_TYPE_TRACEPOINT)
> + return NULL;
> +
> + tp_format = trace_event__tp_format(evsel->tp_sys, evsel->tp_name);
> + if (IS_ERR(tp_format)) {
> + int err = -PTR_ERR(evsel->tp_format);
> +
> + pr_err("Error getting tracepoint format '%s' '%s'(%d)\n",
> + evsel__name(evsel), strerror(err), err);
> + return NULL;
> + }
> + evsel->tp_format = tp_format;
> + return evsel->tp_format;
> +}
> #endif
>
> const char *const evsel__hw_names[PERF_COUNT_HW_MAX] = {
> @@ -1587,6 +1634,10 @@ void evsel__exit(struct evsel *evsel)
> perf_thread_map__put(evsel->core.threads);
> zfree(&evsel->group_name);
> zfree(&evsel->name);
> +#ifdef HAVE_LIBTRACEEVENT
> + zfree(&evsel->tp_sys);
> + zfree(&evsel->tp_name);
> +#endif
> zfree(&evsel->filter);
> zfree(&evsel->group_pmu_name);
> zfree(&evsel->unit);
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index c3e53d320bf5..93b6244ec302 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -59,6 +59,8 @@ struct evsel {
> char *group_name;
> const char *group_pmu_name;
> #ifdef HAVE_LIBTRACEEVENT
> + char *tp_sys;
> + char *tp_name;
> struct tep_event *tp_format;
> #endif
> char *filter;
> @@ -247,25 +249,17 @@ int copy_config_terms(struct list_head *dst, struct list_head *src);
> 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, bool format);
> -
> /*
> * Returns pointer with encoded error via <linux/err.h> interface.
> */
> +struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx, bool format);
> static inline struct evsel *evsel__newtp(const char *sys, const char *name)
> {
> return evsel__newtp_idx(sys, name, 0, true);
> }
>
> -static inline struct tep_event *evsel__tp_format(struct evsel *evsel)
> -{
> - return evsel->tp_format;
> -}
> -#endif
> -
> #ifdef HAVE_LIBTRACEEVENT
> -struct tep_event *event_format__new(const char *sys, const char *name);
> +struct tep_event *evsel__tp_format(struct evsel *evsel);
> #endif
>
> void evsel__init(struct evsel *evsel, struct perf_event_attr *attr, int idx);
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index afeb8d815bbf..7fc1c36ef2a4 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -489,7 +489,6 @@ int parse_events_add_cache(struct list_head *list, int *idx, const char *name,
> return found_supported ? 0 : -EINVAL;
> }
>
> -#ifdef HAVE_LIBTRACEEVENT
> static void tracepoint_error(struct parse_events_error *e, int err,
> const char *sys, const char *name, int column)
> {
> @@ -644,7 +643,6 @@ static int add_tracepoint_multi_sys(struct parse_events_state *parse_state,
> closedir(events_dir);
> return ret;
> }
> -#endif /* HAVE_LIBTRACEEVENT */
>
> size_t default_breakpoint_len(void)
> {
> @@ -1066,7 +1064,6 @@ static int config_term_pmu(struct perf_event_attr *attr,
> return config_term_common(attr, term, err);
> }
>
> -#ifdef HAVE_LIBTRACEEVENT
> static int config_term_tracepoint(struct perf_event_attr *attr,
> struct parse_events_term *term,
> struct parse_events_error *err)
> @@ -1111,7 +1108,6 @@ static int config_term_tracepoint(struct perf_event_attr *attr,
>
> return 0;
> }
> -#endif
>
> static int config_attr(struct perf_event_attr *attr,
> const struct parse_events_terms *head,
> @@ -1303,7 +1299,7 @@ int parse_events_add_tracepoint(struct parse_events_state *parse_state,
> struct parse_events_terms *head_config, void *loc_)
> {
> YYLTYPE *loc = loc_;
> -#ifdef HAVE_LIBTRACEEVENT
> +
> if (head_config) {
> struct perf_event_attr attr;
>
> @@ -1318,16 +1314,6 @@ int parse_events_add_tracepoint(struct parse_events_state *parse_state,
> else
> return add_tracepoint_event(parse_state, list, sys, event,
> err, head_config, loc);
> -#else
> - (void)parse_state;
> - (void)list;
> - (void)sys;
> - (void)event;
> - (void)head_config;
> - parse_events_error__handle(err, loc->first_column, strdup("unsupported tracepoint"),
> - strdup("libtraceevent is necessary for tracepoint support"));
> - return -1;
> -#endif
> }
>
> static int __parse_events_add_numeric(struct parse_events_state *parse_state,
> --
> 2.47.0.199.ga7371fff76-goog
>
Powered by blists - more mailing lists