[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230817091254.GA176609@leoy-huanghe.lan>
Date: Thu, 17 Aug 2023 17:12:54 +0800
From: Leo Yan <leo.yan@...aro.org>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Ian Rogers <irogers@...gle.com>,
John Garry <john.g.garry@...cle.com>,
Will Deacon <will@...nel.org>,
James Clark <james.clark@....com>,
Mike Leach <mike.leach@...aro.org>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Guo Ren <guoren@...nel.org>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>,
Huacai Chen <chenhuacai@...nel.org>,
Ming Wang <wangming01@...ngson.cn>,
Eric Lin <eric.lin@...ive.com>,
Kan Liang <kan.liang@...ux.intel.com>,
Sandipan Das <sandipan.das@....com>,
Ivan Babrou <ivan@...udflare.com>,
Fangrui Song <maskray@...gle.com>,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-perf-users@...r.kernel.org, linux-csky@...r.kernel.org,
linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v2 0/6] perf parse-regs: Refactor architecture functions
On Wed, Aug 16, 2023 at 08:46:23AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Aug 16, 2023 at 10:07:15AM +0800, Leo Yan escreveu:
> > On Tue, Aug 15, 2023 at 03:57:17PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Tue, Aug 15, 2023 at 03:52:38PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > Em Tue, Aug 15, 2023 at 03:45:27PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > > > Agreed, applied to perf-tools-next, sorry for the delay.
> > > > >
> > > > > Had to add this to make 'perf test python' to work. Please run 'perf
> > > > > test' before sending patches.
> > > >
> > > > One more, please also do a 'make -C tools/perf build-test', with it I
> > > > caught this:
> > > >
> > > > make_no_libunwind_O: cd . && make NO_LIBUNWIND=1 FEATURES_DUMP=/var/home/acme/git/perf-tools-next/tools/perf/BUILD_TEST_FEATURE_DUMP -j32 O=/tmp/tmp.yeEGyQq2HR DESTDIR=/tmp/tmp.ITgoO16jjH
> > > > cd . && make NO_LIBUNWIND=1 FEATURES_DUMP=/var/home/acme/git/perf-tools-next/tools/perf/BUILD_TEST_FEATURE_DUMP -j32 O=/tmp/tmp.yeEGyQq2HR DESTDIR=/tmp/tmp.ITgoO16jjH
> > >
> > > +#include "util/env.h"
> > >
> > > As now we need it for perf_env__arch(ui->machine->env)
> >
> > Sorry for inconvenience.
> >
> > I saw this patch series has been picked into the branch:
> > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/log/?h=tmp.perf-tools-next
> >
> > If want me to follow up, let me know. Thank you!
>
> Right, I'll fix this ones:
>
> [perfbuilder@...e ~]$ grep "unused variable" dm.log/*:*
> dm.log/ubuntu:18.04-x-m68k:util/evsel.c:848:14: error: unused variable 'arch' [-Werror=unused-variable]
> dm.log/ubuntu:18.04-x-riscv64:util/evsel.c:848:14: error: unused variable 'arch' [-Werror=unused-variable]
> dm.log/ubuntu:18.04-x-sh4:util/evsel.c:848:14: error: unused variable 'arch' [-Werror=unused-variable]
> dm.log/ubuntu:18.04-x-sparc64:util/evsel.c:848:14: error: unused variable 'arch' [-Werror=unused-variable]
> [perfbuilder@...e ~]$
>
> And move that to perf-tools-next, we can go on from there.
>
> The above is because we don't define CONFIG_PERF_REGS for these
> architectures and thus that variable ends up not being used, so I'm
> fixing up like below, in the cset where you made DWARF_MINIMAL_REGS
> receive the arch parameter.
I reviewed your below amended change, it looks good to me.
And I tested the latest perf-tools-next branch for both native and cross
register parsing with perf Arm64 binaries. Looks good to me.
Thanks a lot for fixing up!
> Also I haven't checked how gracefully we react when processing a
> perf.data collected in one of those unsupported arches, can you please
> check?
At the first glance, we can add checking arch support of register
parsing in the function callchain_param_setup(), which is invoked by
`perf report` and `perf script`, but we need to handle `perf top`
specifically. Or we can consider to add checking in the function
__parse_callchain_report_opt().
Anyway, I will look into deails and work out patches.
Thanks,
Leo
> - Arnaldo
>
> diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
> index 790c1a26bbfe9b4b..de1673057e502de9 100644
> --- a/tools/perf/util/perf_regs.h
> +++ b/tools/perf/util/perf_regs.h
> @@ -32,9 +32,6 @@ extern const struct sample_reg sample_reg_masks[];
>
> #include <perf_regs.h>
>
> -#define DWARF_MINIMAL_REGS(arch) \
> - ((1ULL << perf_arch_reg_ip(arch)) | (1ULL << perf_arch_reg_sp(arch)))
> -
> const char *perf_reg_name(int id, const char *arch);
> int perf_reg_value(u64 *valp, struct regs_dump *regs, int id);
> uint64_t perf_arch_reg_ip(const char *arch);
> @@ -67,11 +64,19 @@ const char *__perf_reg_name_x86(int id);
> uint64_t __perf_reg_ip_x86(void);
> uint64_t __perf_reg_sp_x86(void);
>
> +static inline uint64_t DWARF_MINIMAL_REGS(const char *arch)
> +{
> + return (1ULL << perf_arch_reg_ip(arch)) | (1ULL << perf_arch_reg_sp(arch));
> +}
> +
> #else
> #define PERF_REGS_MASK 0
> #define PERF_REGS_MAX 0
>
> -#define DWARF_MINIMAL_REGS(arch) PERF_REGS_MASK
> +static inline uint64_t DWARF_MINIMAL_REGS(const char *arch __maybe_unused)
> +{
> + return PERF_REGS_MASK;
> +}
>
> static inline const char *perf_reg_name(int id __maybe_unused, const char *arch __maybe_unused)
> {
>
> > > > CC /tmp/tmp.yeEGyQq2HR/util/expr-flex.o
> > > > util/unwind-libdw.c: In function ‘memory_read’:
> > > > util/unwind-libdw.c:173:28: error: implicit declaration of function ‘perf_env__arch’ [-Werror=implicit-function-declaration]
> > > > 173 | const char *arch = perf_env__arch(ui->machine->env);
> > > > | ^~~~~~~~~~~~~~
> > > > util/unwind-libdw.c:173:28: error: initialization of ‘const char *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
> > > > util/unwind-libdw.c: In function ‘unwind__get_entries’:
> > > > util/unwind-libdw.c:258:28: error: initialization of ‘const char *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
> > > > 258 | const char *arch = perf_env__arch(ui_buf.machine->env);
> > > > | ^~~~~~~~~~~~~~
> > > > cc1: all warnings being treated as errors
> > > > make[6]: *** [/var/home/acme/git/perf-tools-next/tools/build/Makefile.build:98: /tmp/tmp.yeEGyQq2HR/util/unwind-libdw.o] Error 1
> > > > make[6]: *** Waiting for unfinished jobs....
> > > > make[5]: *** [/var/home/acme/git/perf-tools-next/tools/build/Makefile.build:150: util] Error 2
> > > > make[4]: *** [Makefile.perf:662: /tmp/tmp.yeEGyQq2HR/perf-in.o] Error 2
> > > > make[4]: *** Waiting for unfinished jobs....
> > > > CC /tmp/tmp.yeEGyQq2HR/pmu-events/pmu-events.o
> > > > LD /tmp/tmp.yeEGyQq2HR/pmu-events/pmu-events-in.o
> > > > make[3]: *** [Makefile.perf:238: sub-make] Error 2
> > > > make[2]: *** [Makefile:70: all] Error 2
> > > > make[1]: *** [tests/make:337: make_no_libunwind_O] Error 1
> > > > make: *** [Makefile:103: build-test] Error 2
> > > > make: Leaving directory '/var/home/acme/git/perf-tools-next/tools/perf'
> > > >
> > > > real 1m29.784s
> > > > user 10m41.597s
> > > > sys 2m55.948s
> > > > ⬢[acme@...lbox perf-tools-next]$
> > > >
> > > > I'm trying to fix
> > >
> > > --
> > >
> > > - Arnaldo
>
> --
>
> - Arnaldo
Powered by blists - more mailing lists