Re: [PATCH 5/6] perf: probes: move ftrace README parsing logic into trace-event-parse.c
From: Masami Hiramatsu
Date: Tue Mar 07 2017 - 10:51:49 EST
On Tue, 7 Mar 2017 16:17:40 +0530
"Naveen N. Rao" <naveen.n.rao@xxxxxxxxxxxxxxxxxx> wrote:
> probe-file.c needs libelf, but scanning ftrace README does not require
> that. As such, move the ftrace README scanning logic out of probe-file.c
> and into trace-event-parse.c.
As far as I can see, there is no reason to push this out from probe-file.c
because anyway this API using code requires libelf. Without this, I can
still build perf with NO_LIBELF=1. So I wouldn't like to pick this.
(I think we can drop this from this series)
Thank you,
>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@xxxxxxxxxxxxxxxxxx>
> ---
> tools/perf/util/probe-file.c | 87 +++---------------------------------
> tools/perf/util/probe-file.h | 2 -
> tools/perf/util/trace-event-parse.c | 89 +++++++++++++++++++++++++++++++++++++
> tools/perf/util/trace-event.h | 4 ++
> 4 files changed, 99 insertions(+), 83 deletions(-)
>
> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> index 1542cd0d6799..ff872fa30cdb 100644
> --- a/tools/perf/util/probe-file.c
> +++ b/tools/perf/util/probe-file.c
> @@ -26,6 +26,7 @@
> #include <api/fs/tracing_path.h>
> #include "probe-event.h"
> #include "probe-file.h"
> +#include "trace-event.h"
> #include "session.h"
>
> #define MAX_CMDLEN 256
> @@ -70,33 +71,17 @@ static void print_both_open_warning(int kerr, int uerr)
> }
> }
>
> -int open_trace_file(const char *trace_file, bool readwrite)
> -{
> - char buf[PATH_MAX];
> - int ret;
> -
> - ret = e_snprintf(buf, PATH_MAX, "%s/%s",
> - tracing_path, trace_file);
> - if (ret >= 0) {
> - pr_debug("Opening %s write=%d\n", buf, readwrite);
> - if (readwrite && !probe_event_dry_run)
> - ret = open(buf, O_RDWR | O_APPEND, 0);
> - else
> - ret = open(buf, O_RDONLY, 0);
> -
> - if (ret < 0)
> - ret = -errno;
> - }
> - return ret;
> -}
> -
> static int open_kprobe_events(bool readwrite)
> {
> + if (probe_event_dry_run)
> + readwrite = false;
> return open_trace_file("kprobe_events", readwrite);
> }
>
> static int open_uprobe_events(bool readwrite)
> {
> + if (probe_event_dry_run)
> + readwrite = false;
> return open_trace_file("uprobe_events", readwrite);
> }
>
> @@ -877,72 +862,12 @@ int probe_cache__show_all_caches(struct strfilter *filter)
> return 0;
> }
>
> -enum ftrace_readme {
> - FTRACE_README_PROBE_TYPE_X = 0,
> - FTRACE_README_KRETPROBE_OFFSET,
> - FTRACE_README_END,
> -};
> -
> -static struct {
> - const char *pattern;
> - bool avail;
> -} ftrace_readme_table[] = {
> -#define DEFINE_TYPE(idx, pat) \
> - [idx] = {.pattern = pat, .avail = false}
> - DEFINE_TYPE(FTRACE_README_PROBE_TYPE_X, "*type: * x8/16/32/64,*"),
> - DEFINE_TYPE(FTRACE_README_KRETPROBE_OFFSET, "*place (kretprobe): *"),
> -};
> -
> -static bool scan_ftrace_readme(enum ftrace_readme type)
> -{
> - int fd;
> - FILE *fp;
> - char *buf = NULL;
> - size_t len = 0;
> - bool ret = false;
> - static bool scanned = false;
> -
> - if (scanned)
> - goto result;
> -
> - fd = open_trace_file("README", false);
> - if (fd < 0)
> - return ret;
> -
> - fp = fdopen(fd, "r");
> - if (!fp) {
> - close(fd);
> - return ret;
> - }
> -
> - while (getline(&buf, &len, fp) > 0)
> - for (enum ftrace_readme i = 0; i < FTRACE_README_END; i++)
> - if (!ftrace_readme_table[i].avail)
> - ftrace_readme_table[i].avail =
> - strglobmatch(buf, ftrace_readme_table[i].pattern);
> - scanned = true;
> -
> - fclose(fp);
> - free(buf);
> -
> -result:
> - if (type >= FTRACE_README_END)
> - return false;
> -
> - return ftrace_readme_table[type].avail;
> -}
> -
> bool probe_type_is_available(enum probe_type type)
> {
> if (type >= PROBE_TYPE_END)
> return false;
> else if (type == PROBE_TYPE_X)
> - return scan_ftrace_readme(FTRACE_README_PROBE_TYPE_X);
> + return probe_type_x_is_supported();
>
> return true;
> }
> -
> -bool kretprobe_offset_is_supported(void)
> -{
> - return scan_ftrace_readme(FTRACE_README_KRETPROBE_OFFSET);
> -}
> diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
> index dbf95a00864a..eba44c3e9dca 100644
> --- a/tools/perf/util/probe-file.h
> +++ b/tools/perf/util/probe-file.h
> @@ -35,7 +35,6 @@ enum probe_type {
>
> /* probe-file.c depends on libelf */
> #ifdef HAVE_LIBELF_SUPPORT
> -int open_trace_file(const char *trace_file, bool readwrite);
> int probe_file__open(int flag);
> int probe_file__open_both(int *kfd, int *ufd, int flag);
> struct strlist *probe_file__get_namelist(int fd);
> @@ -65,7 +64,6 @@ struct probe_cache_entry *probe_cache__find_by_name(struct probe_cache *pcache,
> const char *group, const char *event);
> int probe_cache__show_all_caches(struct strfilter *filter);
> bool probe_type_is_available(enum probe_type type);
> -bool kretprobe_offset_is_supported(void);
> #else /* ! HAVE_LIBELF_SUPPORT */
> static inline struct probe_cache *probe_cache__new(const char *tgt __maybe_unused)
> {
> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
> index de0078e21408..77697c446cbd 100644
> --- a/tools/perf/util/trace-event-parse.c
> +++ b/tools/perf/util/trace-event-parse.c
> @@ -23,10 +23,12 @@
> #include <string.h>
> #include <ctype.h>
> #include <errno.h>
> +#include <api/fs/tracing_path.h>
>
> #include "../perf.h"
> #include "util.h"
> #include "trace-event.h"
> +#include "debug.h"
>
> static int get_common_field(struct scripting_context *context,
> int *offset, int *size, const char *type)
> @@ -254,3 +256,90 @@ unsigned long long eval_flag(const char *flag)
>
> return 0;
> }
> +
> +int open_trace_file(const char *trace_file, bool readwrite)
> +{
> + char buf[PATH_MAX];
> + int ret;
> +
> + ret = snprintf(buf, PATH_MAX, "%s/%s",
> + tracing_path, trace_file);
> + if (ret >= PATH_MAX)
> + ret = -E2BIG;
> + if (ret >= 0) {
> + pr_debug("Opening %s write=%d\n", buf, readwrite);
> + if (readwrite)
> + ret = open(buf, O_RDWR | O_APPEND, 0);
> + else
> + ret = open(buf, O_RDONLY, 0);
> +
> + if (ret < 0)
> + ret = -errno;
> + }
> + return ret;
> +}
> +
> +enum ftrace_readme {
> + FTRACE_README_PROBE_TYPE_X = 0,
> + FTRACE_README_KRETPROBE_OFFSET,
> + FTRACE_README_END,
> +};
> +
> +static struct {
> + const char *pattern;
> + bool avail;
> +} ftrace_readme_table[] = {
> +#define DEFINE_TYPE(idx, pat) \
> + [idx] = {.pattern = pat, .avail = false}
> + DEFINE_TYPE(FTRACE_README_PROBE_TYPE_X, "*type: * x8/16/32/64,*"),
> + DEFINE_TYPE(FTRACE_README_KRETPROBE_OFFSET, "*place (kretprobe): *"),
> +};
> +
> +static bool scan_ftrace_readme(enum ftrace_readme type)
> +{
> + int fd;
> + FILE *fp;
> + char *buf = NULL;
> + size_t len = 0;
> + bool ret = false;
> + static bool scanned = false;
> +
> + if (scanned)
> + goto result;
> +
> + fd = open_trace_file("README", false);
> + if (fd < 0)
> + return ret;
> +
> + fp = fdopen(fd, "r");
> + if (!fp) {
> + close(fd);
> + return ret;
> + }
> +
> + while (getline(&buf, &len, fp) > 0)
> + for (enum ftrace_readme i = 0; i < FTRACE_README_END; i++)
> + if (!ftrace_readme_table[i].avail)
> + ftrace_readme_table[i].avail =
> + strglobmatch(buf, ftrace_readme_table[i].pattern);
> + scanned = true;
> +
> + fclose(fp);
> + free(buf);
> +
> +result:
> + if (type >= FTRACE_README_END)
> + return false;
> +
> + return ftrace_readme_table[type].avail;
> +}
> +
> +bool kretprobe_offset_is_supported(void)
> +{
> + return scan_ftrace_readme(FTRACE_README_KRETPROBE_OFFSET);
> +}
> +
> +bool probe_type_x_is_supported(void)
> +{
> + return scan_ftrace_readme(FTRACE_README_PROBE_TYPE_X);
> +}
> diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
> index 1fbc044f9eb0..ecda2d822daf 100644
> --- a/tools/perf/util/trace-event.h
> +++ b/tools/perf/util/trace-event.h
> @@ -37,6 +37,10 @@ int parse_ftrace_file(struct pevent *pevent, char *buf, unsigned long size);
> int parse_event_file(struct pevent *pevent,
> char *buf, unsigned long size, char *sys);
>
> +int open_trace_file(const char *trace_file, bool readwrite);
> +bool kretprobe_offset_is_supported(void);
> +bool probe_type_x_is_supported(void);
> +
> unsigned long long
> raw_field_value(struct event_format *event, const char *name, void *data);
>
> --
> 2.11.1
>
--
Masami Hiramatsu <mhiramat@xxxxxxxxxx>