[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YK5WO3is6EW9TNrX@kernel.org>
Date: Wed, 26 May 2021 11:07:55 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Denys Zagorui <dzagorui@...co.com>
Cc: jolsa@...hat.com, linux-kernel@...r.kernel.org,
peterz@...radead.org, mingo@...hat.com, mark.rutland@....com,
alexander.shishkin@...ux.intel.com, namhyung@...nel.org
Subject: Re: [PATCH v7 1/3] perf report: compile tips.txt in perf binary
Em Fri, May 21, 2021 at 11:20:14PM -0700, Denys Zagorui escreveu:
> It seems there is some need to have an ability to invoke perf from
> build directory without installation
> (84cfac7f05e1: perf tools: Set and pass DOCDIR to builtin-report.c)
> DOCDIR definition contains an absolute path to kernel source directory.
> It is build machine related info and it makes perf binary unreproducible.
>
> This can be avoided by compiling tips.txt in perf directly.
>
> Signed-off-by: Denys Zagorui <dzagorui@...co.com>
> ---
> tools/perf/Build | 2 +-
> tools/perf/Documentation/Build | 9 +++++++++
> tools/perf/builtin-report.c | 34 +++++++++++++++++++++++++---------
> tools/perf/util/util.c | 28 ----------------------------
> tools/perf/util/util.h | 2 --
> 5 files changed, 35 insertions(+), 40 deletions(-)
> create mode 100644 tools/perf/Documentation/Build
>
> diff --git a/tools/perf/Build b/tools/perf/Build
> index db61dbe2b543..3a2e768d7576 100644
> --- a/tools/perf/Build
> +++ b/tools/perf/Build
> @@ -45,12 +45,12 @@ CFLAGS_perf.o += -DPERF_HTML_PATH="BUILD_STR($(htmldir_SQ))" \
> -DPREFIX="BUILD_STR($(prefix_SQ))"
> CFLAGS_builtin-trace.o += -DSTRACE_GROUPS_DIR="BUILD_STR($(STRACE_GROUPS_DIR_SQ))"
> CFLAGS_builtin-report.o += -DTIPDIR="BUILD_STR($(tipdir_SQ))"
> -CFLAGS_builtin-report.o += -DDOCDIR="BUILD_STR($(srcdir_SQ)/Documentation)"
>
> perf-y += util/
> perf-y += arch/
> perf-y += ui/
> perf-y += scripts/
> perf-$(CONFIG_TRACE) += trace/beauty/
> +perf-y += Documentation/
>
> gtk-y += ui/gtk/
> diff --git a/tools/perf/Documentation/Build b/tools/perf/Documentation/Build
> new file mode 100644
> index 000000000000..83e16764caa4
> --- /dev/null
> +++ b/tools/perf/Documentation/Build
> @@ -0,0 +1,9 @@
> +perf-y += tips.o
> +
> +quiet_cmd_ld_tips = LD $@
> + cmd_ld_tips = $(LD) -r -b binary -o $@ $<
> +
> +$(OUTPUT)Documentation/tips.o: Documentation/tips.txt FORCE
> + $(call rule_mkdir)
> + $(call if_changed,ld_tips)
> +
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 36f9ccfeb38a..4f2c7ee8fea1 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -47,7 +47,6 @@
> #include "util/time-utils.h"
> #include "util/auxtrace.h"
> #include "util/units.h"
> -#include "util/util.h" // perf_tip()
> #include "ui/ui.h"
> #include "ui/progress.h"
> #include "util/block-info.h"
> @@ -109,6 +108,9 @@ struct report {
> int nr_block_reports;
> };
>
> +extern char _binary_Documentation_tips_txt_start[];
> +extern char _binary_Documentation_tips_txt_end[];
> +
> static int report__config(const char *var, const char *value, void *cb)
> {
> struct report *rep = cb;
> @@ -614,19 +616,33 @@ static int report__gtk_browse_hists(struct report *rep, const char *help)
> return hist_browser(rep->session->evlist, help, NULL, rep->min_percent);
> }
>
> +static const char *perf_tip(void)
> +{
> + char *start = _binary_Documentation_tips_txt_start;
> + char *tok, *tmp, *prev;
> + int pick, size;
> +
> + size = _binary_Documentation_tips_txt_end - start;
> + pick = random() % size;
> +
> + _binary_Documentation_tips_txt_start[size - 1] = 0;
> +
> + for (tok = strtok_r(start, "\n", &tmp); tok;
> + tok = strtok_r(NULL, "\n", &tmp)) {
> + if (pick < (tok - start))
> + return prev;
> + prev = tok;
> + }
> +
> + return prev;
> +}
[perfbuilder@...e ~]$ export PERF_TARBALL=http://192.168.100.2/perf/perf-5.13.0-rc3.tar.xz
[perfbuilder@...e ~]$ time dm
Wed May 26 11:04:00 AM -03 2021
# export PERF_TARBALL=http://192.168.100.2/perf/perf-5.13.0-rc3.tar.xz
# dm
1 9.39 alpine:3.4 : FAIL gcc version 5.3.0 (Alpine 5.3.0)
builtin-report.c: In function 'cmd_report':
builtin-report.c:560:3: error: 'prev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
fprintf(stdout, "#\n# (%s)\n#\n", help);
^
builtin-report.c:622:20: note: 'prev' was declared here
char *tok, *tmp, *prev;
<SNIP>
10 13.35 alpine:3.13 : FAIL gcc version 10.2.1 20201203 (Alpine 10.2.1_pre1)
builtin-report.c: In function 'cmd_report':
builtin-report.c:560:3: error: 'prev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
560 | fprintf(stdout, "#\n# (%s)\n#\n", help);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
builtin-report.c:622:20: note: 'prev' was declared here
622 | char *tok, *tmp, *prev;
| ^~~~
cc1: all warnings being treated as errors
11 13.77 alpine:edge : FAIL gcc version 10.3.1 20210424 (Alpine 10.3.1_git20210424)
builtin-report.c: In function 'cmd_report':
builtin-report.c:560:3: error: 'prev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
560 | fprintf(stdout, "#\n# (%s)\n#\n", help);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
builtin-report.c:622:20: note: 'prev' was declared here
622 | char *tok, *tmp, *prev;
| ^~~~
cc1: all warnings being treated as errors
I'll check later...
> static int report__browse_hists(struct report *rep)
> {
> int ret;
> struct perf_session *session = rep->session;
> struct evlist *evlist = session->evlist;
> - const char *help = perf_tip(system_path(TIPDIR));
> -
> - if (help == NULL) {
> - /* fallback for people who don't install perf ;-) */
> - help = perf_tip(DOCDIR);
> - if (help == NULL)
> - help = "Cannot load tips.txt file, please install perf!";
> - }
> + const char *help = perf_tip();
>
> switch (use_browser) {
> case 1:
> diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
> index 37a9492edb3e..3bba74e431ed 100644
> --- a/tools/perf/util/util.c
> +++ b/tools/perf/util/util.c
> @@ -379,34 +379,6 @@ fetch_kernel_version(unsigned int *puint, char *str,
> return 0;
> }
>
> -const char *perf_tip(const char *dirpath)
> -{
> - struct strlist *tips;
> - struct str_node *node;
> - char *tip = NULL;
> - struct strlist_config conf = {
> - .dirname = dirpath,
> - .file_only = true,
> - };
> -
> - tips = strlist__new("tips.txt", &conf);
> - if (tips == NULL)
> - return errno == ENOENT ? NULL :
> - "Tip: check path of tips.txt or get more memory! ;-p";
> -
> - if (strlist__nr_entries(tips) == 0)
> - goto out;
> -
> - node = strlist__entry(tips, random() % strlist__nr_entries(tips));
> - if (asprintf(&tip, "Tip: %s", node->s) < 0)
> - tip = (char *)"Tip: get more memory! ;-)";
> -
> -out:
> - strlist__delete(tips);
> -
> - return tip;
> -}
> -
> char *perf_exe(char *buf, int len)
> {
> int n = readlink("/proc/self/exe", buf, len);
> diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
> index ad737052e597..80b194ee6c7d 100644
> --- a/tools/perf/util/util.h
> +++ b/tools/perf/util/util.h
> @@ -39,8 +39,6 @@ int fetch_kernel_version(unsigned int *puint,
> #define KVER_FMT "%d.%d.%d"
> #define KVER_PARAM(x) KVER_VERSION(x), KVER_PATCHLEVEL(x), KVER_SUBLEVEL(x)
>
> -const char *perf_tip(const char *dirpath);
> -
> #ifndef HAVE_SCHED_GETCPU_SUPPORT
> int sched_getcpu(void);
> #endif
> --
> 2.26.2.Cisco
>
--
- Arnaldo
Powered by blists - more mailing lists