Message ID | 20230606014559.21783-1-leo.yan@linaro.org (mailing list archive) |
---|---|
Headers | show |
Series | perf parse-regs: Refactor architecture functions | expand |
On Mon, Jun 5, 2023 at 6:46 PM Leo Yan <leo.yan@linaro.org> wrote: > > This patch series is to refactor arch related functions for register > parsing, which follows up the discussion for v1: > https://lore.kernel.org/lkml/20230520025537.1811986-1-leo.yan@linaro.org/ > > Compared to patch series v1, this patch series introduces new functions > perf_arch_reg_{ip|sp}(), so this can allow the tool to support cross > analysis. > > To verify the cross analysis, I used below steps: > > - Firstly, I captured perf data on Arm64 machine: > > $ perf record --call-graph fp -- ./test_program > > Or ... > > $ perf record --call-graph dwarf -- ./test_program > > Then, I also archived associated debug data: > > $ perf archive > > - Secondly, I copied the perf data file and debug tar file on my x86 > machine: > > $ scp perf.data perf.data.tar.bz2 leoy@IP_ADDRESS:/target/path/ > > - On x86 machine, I need to build perf for support multi-arch unwinding: > > $ git clone http://git.savannah.gnu.org/r/libunwind.git > $ cd libunwind > $ autoreconf -i > > # Build and install libunwind aarch64: > $ ./configure prefix=/home/leoy/Work/tools/libunwind/install/ \ > --target=aarch64-linux-gnu CC=x86_64-linux-gnu-gcc > $ make && make install > > # Build and install libunwind x86: > $ ./configure prefix=/home/leoy/Work/tools/libunwind/install/ \ > --target=x86_64-linux-gnu CC=x86_64-linux-gnu-gcc > $ make && make install > > - Build perf tool for support multi-archs: > > $ cd $LINUX/tools/perf > $ make VF=1 DEBUG=1 LIBUNWIND_DIR=/home/leoy/Work/tools/libunwind/install > > At the end, I verified the x86 perf tool can do cross analysis for aarch64's > perf data file. > > Note, I still see x86 perf tool cannot display the complete callgraph > for aarch64, but it should not the issue caused by this series, which > will be addressed by separate patches. > > I also built this patch series on my Arm64 and x86 machines, both can > compile perf tool successfully; but I have no chance to build other > archs natively. > > Changes from v1: > - For support cross analysis for IP/SP registers, introduced patch 0002 > (James Clark, Ian Rogers). > > > Leo Yan (6): > perf parse-regs: Refactor arch register parsing functions > perf parse-regs: Introduce functions perf_arch_reg_{ip|sp}() > perf unwind: Use perf_arch_reg_{ip|sp}() to substitute macros > perf parse-regs: Remove unused macros PERF_REG_{IP|SP} > perf parse-regs: Remove PERF_REGS_{MAX|MASK} from common code > perf parse-regs: Move out arch specific header from util/perf_regs.h Sorry for the slow review. For the series: Acked-by: Ian Rogers <irogers@google.com> Some thoughts: uint64_t __perf_reg_ip_arm(void) uint64_t seems like we're giving a lot of space for future register encodings. I think some of the other functions use this size of value due to returning a bitmap/mask, but here it isn't clear and just feels excessive. Do we need the "__" prefix on all the functions? In Makefile.config there are NO_PERF_REGS and CONFIG_PERF_REGS then the define HAVE_PERF_REGS_SUPPORT. Is this still relevant? If we had an architecture with no support, couldn't it still read a perf.data file from a supported architecture? It would be nice to remove at least NO_PERF_REGS and HAVE_PERF_REGS_SUPPORT. This change is very worthwhile fix and cleanup, it didn't introduce what is pondered above, hence the acked-by. Thanks! Ian > tools/perf/arch/arm/include/perf_regs.h | 3 - > tools/perf/arch/arm/util/perf_regs.c | 11 + > tools/perf/arch/arm/util/unwind-libdw.c | 1 + > tools/perf/arch/arm64/include/perf_regs.h | 3 - > tools/perf/arch/arm64/util/machine.c | 1 + > tools/perf/arch/arm64/util/perf_regs.c | 6 + > tools/perf/arch/arm64/util/unwind-libdw.c | 1 + > tools/perf/arch/csky/include/perf_regs.h | 3 - > tools/perf/arch/csky/util/perf_regs.c | 11 + > tools/perf/arch/csky/util/unwind-libdw.c | 1 + > tools/perf/arch/loongarch/include/perf_regs.h | 2 - > tools/perf/arch/loongarch/util/perf_regs.c | 11 + > tools/perf/arch/loongarch/util/unwind-libdw.c | 1 + > tools/perf/arch/mips/include/perf_regs.h | 2 - > tools/perf/arch/mips/util/perf_regs.c | 11 + > tools/perf/arch/powerpc/include/perf_regs.h | 3 - > tools/perf/arch/powerpc/util/perf_regs.c | 6 + > tools/perf/arch/powerpc/util/unwind-libdw.c | 1 + > tools/perf/arch/riscv/include/perf_regs.h | 3 - > tools/perf/arch/riscv/util/perf_regs.c | 11 + > tools/perf/arch/riscv/util/unwind-libdw.c | 1 + > tools/perf/arch/s390/include/perf_regs.h | 3 - > tools/perf/arch/s390/util/perf_regs.c | 11 + > tools/perf/arch/s390/util/unwind-libdw.c | 1 + > tools/perf/arch/x86/include/perf_regs.h | 2 - > tools/perf/arch/x86/util/perf_regs.c | 6 + > tools/perf/arch/x86/util/unwind-libdw.c | 1 + > tools/perf/util/Build | 1 + > tools/perf/util/evsel.c | 6 +- > tools/perf/util/libunwind/arm64.c | 2 - > tools/perf/util/libunwind/x86_32.c | 2 - > tools/perf/util/perf-regs-arch/Build | 9 + > .../util/perf-regs-arch/perf_regs_aarch64.c | 96 +++ > .../perf/util/perf-regs-arch/perf_regs_arm.c | 60 ++ > .../perf/util/perf-regs-arch/perf_regs_csky.c | 100 +++ > .../util/perf-regs-arch/perf_regs_loongarch.c | 91 +++ > .../perf/util/perf-regs-arch/perf_regs_mips.c | 87 ++ > .../util/perf-regs-arch/perf_regs_powerpc.c | 145 ++++ > .../util/perf-regs-arch/perf_regs_riscv.c | 92 +++ > .../perf/util/perf-regs-arch/perf_regs_s390.c | 96 +++ > .../perf/util/perf-regs-arch/perf_regs_x86.c | 98 +++ > tools/perf/util/perf_regs.c | 772 ++---------------- > tools/perf/util/perf_regs.h | 49 +- > tools/perf/util/unwind-libdw.c | 7 +- > tools/perf/util/unwind-libunwind-local.c | 6 +- > tools/perf/util/unwind.h | 8 - > 46 files changed, 1078 insertions(+), 766 deletions(-) > create mode 100644 tools/perf/util/perf-regs-arch/Build > create mode 100644 tools/perf/util/perf-regs-arch/perf_regs_aarch64.c > create mode 100644 tools/perf/util/perf-regs-arch/perf_regs_arm.c > create mode 100644 tools/perf/util/perf-regs-arch/perf_regs_csky.c > create mode 100644 tools/perf/util/perf-regs-arch/perf_regs_loongarch.c > create mode 100644 tools/perf/util/perf-regs-arch/perf_regs_mips.c > create mode 100644 tools/perf/util/perf-regs-arch/perf_regs_powerpc.c > create mode 100644 tools/perf/util/perf-regs-arch/perf_regs_riscv.c > create mode 100644 tools/perf/util/perf-regs-arch/perf_regs_s390.c > create mode 100644 tools/perf/util/perf-regs-arch/perf_regs_x86.c > > -- > 2.34.1 >
Em Wed, Jul 12, 2023 at 03:37:36PM -0700, Ian Rogers escreveu: > On Mon, Jun 5, 2023 at 6:46 PM Leo Yan <leo.yan@linaro.org> wrote: > > > > This patch series is to refactor arch related functions for register > > parsing, which follows up the discussion for v1: > > https://lore.kernel.org/lkml/20230520025537.1811986-1-leo.yan@linaro.org/ > > > > Compared to patch series v1, this patch series introduces new functions > > perf_arch_reg_{ip|sp}(), so this can allow the tool to support cross > > analysis. > > > > To verify the cross analysis, I used below steps: > > > > - Firstly, I captured perf data on Arm64 machine: > > > > $ perf record --call-graph fp -- ./test_program > > > > Or ... > > > > $ perf record --call-graph dwarf -- ./test_program > > > > Then, I also archived associated debug data: > > > > $ perf archive > > > > - Secondly, I copied the perf data file and debug tar file on my x86 > > machine: > > > > $ scp perf.data perf.data.tar.bz2 leoy@IP_ADDRESS:/target/path/ > > > > - On x86 machine, I need to build perf for support multi-arch unwinding: > > > > $ git clone http://git.savannah.gnu.org/r/libunwind.git > > $ cd libunwind > > $ autoreconf -i > > > > # Build and install libunwind aarch64: > > $ ./configure prefix=/home/leoy/Work/tools/libunwind/install/ \ > > --target=aarch64-linux-gnu CC=x86_64-linux-gnu-gcc > > $ make && make install > > > > # Build and install libunwind x86: > > $ ./configure prefix=/home/leoy/Work/tools/libunwind/install/ \ > > --target=x86_64-linux-gnu CC=x86_64-linux-gnu-gcc > > $ make && make install > > > > - Build perf tool for support multi-archs: > > > > $ cd $LINUX/tools/perf > > $ make VF=1 DEBUG=1 LIBUNWIND_DIR=/home/leoy/Work/tools/libunwind/install > > > > At the end, I verified the x86 perf tool can do cross analysis for aarch64's > > perf data file. > > > > Note, I still see x86 perf tool cannot display the complete callgraph > > for aarch64, but it should not the issue caused by this series, which > > will be addressed by separate patches. > > > > I also built this patch series on my Arm64 and x86 machines, both can > > compile perf tool successfully; but I have no chance to build other > > archs natively. > > > > Changes from v1: > > - For support cross analysis for IP/SP registers, introduced patch 0002 > > (James Clark, Ian Rogers). > > > > > > Leo Yan (6): > > perf parse-regs: Refactor arch register parsing functions > > perf parse-regs: Introduce functions perf_arch_reg_{ip|sp}() > > perf unwind: Use perf_arch_reg_{ip|sp}() to substitute macros > > perf parse-regs: Remove unused macros PERF_REG_{IP|SP} > > perf parse-regs: Remove PERF_REGS_{MAX|MASK} from common code > > perf parse-regs: Move out arch specific header from util/perf_regs.h > > Sorry for the slow review. For the series: > Acked-by: Ian Rogers <irogers@google.com> > > Some thoughts: > uint64_t __perf_reg_ip_arm(void) > uint64_t seems like we're giving a lot of space for future register > encodings. I think some of the other functions use this size of value > due to returning a bitmap/mask, but here it isn't clear and just feels > excessive. > > Do we need the "__" prefix on all the functions? > > In Makefile.config there are NO_PERF_REGS and CONFIG_PERF_REGS then > the define HAVE_PERF_REGS_SUPPORT. Is this still relevant? If we had > an architecture with no support, couldn't it still read a perf.data > file from a supported architecture? It would be nice to remove at > least NO_PERF_REGS and HAVE_PERF_REGS_SUPPORT. > > This change is very worthwhile fix and cleanup, it didn't introduce > what is pondered above, hence the acked-by. Agreed, applied to perf-tools-next, sorry for the delay. - Arnaldo
Em Tue, Aug 15, 2023 at 03:24:04PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Wed, Jul 12, 2023 at 03:37:36PM -0700, Ian Rogers escreveu: > > On Mon, Jun 5, 2023 at 6:46 PM Leo Yan <leo.yan@linaro.org> wrote: > > > > > > This patch series is to refactor arch related functions for register > > > parsing, which follows up the discussion for v1: > > > https://lore.kernel.org/lkml/20230520025537.1811986-1-leo.yan@linaro.org/ > > > > > > Compared to patch series v1, this patch series introduces new functions > > > perf_arch_reg_{ip|sp}(), so this can allow the tool to support cross > > > analysis. > > > > > > To verify the cross analysis, I used below steps: > > > > > > - Firstly, I captured perf data on Arm64 machine: > > > > > > $ perf record --call-graph fp -- ./test_program > > > > > > Or ... > > > > > > $ perf record --call-graph dwarf -- ./test_program > > > > > > Then, I also archived associated debug data: > > > > > > $ perf archive > > > > > > - Secondly, I copied the perf data file and debug tar file on my x86 > > > machine: > > > > > > $ scp perf.data perf.data.tar.bz2 leoy@IP_ADDRESS:/target/path/ > > > > > > - On x86 machine, I need to build perf for support multi-arch unwinding: > > > > > > $ git clone http://git.savannah.gnu.org/r/libunwind.git > > > $ cd libunwind > > > $ autoreconf -i > > > > > > # Build and install libunwind aarch64: > > > $ ./configure prefix=/home/leoy/Work/tools/libunwind/install/ \ > > > --target=aarch64-linux-gnu CC=x86_64-linux-gnu-gcc > > > $ make && make install > > > > > > # Build and install libunwind x86: > > > $ ./configure prefix=/home/leoy/Work/tools/libunwind/install/ \ > > > --target=x86_64-linux-gnu CC=x86_64-linux-gnu-gcc > > > $ make && make install > > > > > > - Build perf tool for support multi-archs: > > > > > > $ cd $LINUX/tools/perf > > > $ make VF=1 DEBUG=1 LIBUNWIND_DIR=/home/leoy/Work/tools/libunwind/install > > > > > > At the end, I verified the x86 perf tool can do cross analysis for aarch64's > > > perf data file. > > > > > > Note, I still see x86 perf tool cannot display the complete callgraph > > > for aarch64, but it should not the issue caused by this series, which > > > will be addressed by separate patches. > > > > > > I also built this patch series on my Arm64 and x86 machines, both can > > > compile perf tool successfully; but I have no chance to build other > > > archs natively. > > > > > > Changes from v1: > > > - For support cross analysis for IP/SP registers, introduced patch 0002 > > > (James Clark, Ian Rogers). > > > > > > > > > Leo Yan (6): > > > perf parse-regs: Refactor arch register parsing functions > > > perf parse-regs: Introduce functions perf_arch_reg_{ip|sp}() > > > perf unwind: Use perf_arch_reg_{ip|sp}() to substitute macros > > > perf parse-regs: Remove unused macros PERF_REG_{IP|SP} > > > perf parse-regs: Remove PERF_REGS_{MAX|MASK} from common code > > > perf parse-regs: Move out arch specific header from util/perf_regs.h > > > > Sorry for the slow review. For the series: > > Acked-by: Ian Rogers <irogers@google.com> > > > > Some thoughts: > > uint64_t __perf_reg_ip_arm(void) > > uint64_t seems like we're giving a lot of space for future register > > encodings. I think some of the other functions use this size of value > > due to returning a bitmap/mask, but here it isn't clear and just feels > > excessive. > > > > Do we need the "__" prefix on all the functions? > > > > In Makefile.config there are NO_PERF_REGS and CONFIG_PERF_REGS then > > the define HAVE_PERF_REGS_SUPPORT. Is this still relevant? If we had > > an architecture with no support, couldn't it still read a perf.data > > file from a supported architecture? It would be nice to remove at > > least NO_PERF_REGS and HAVE_PERF_REGS_SUPPORT. > > > > This change is very worthwhile fix and cleanup, it didn't introduce > > what is pondered above, hence the acked-by. > > 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. - Arnaldo diff --git a/tools/perf/util/python-ext-sources b/tools/perf/util/python-ext-sources index d4c9b4cd35efa556..26e1c8d973ea0b95 100644 --- a/tools/perf/util/python-ext-sources +++ b/tools/perf/util/python-ext-sources @@ -40,3 +40,12 @@ util/rwsem.c util/hashmap.c util/perf_regs.c util/fncache.c +util/perf-regs-arch/perf_regs_aarch64.c +util/perf-regs-arch/perf_regs_arm.c +util/perf-regs-arch/perf_regs_csky.c +util/perf-regs-arch/perf_regs_loongarch.c +util/perf-regs-arch/perf_regs_mips.c +util/perf-regs-arch/perf_regs_powerpc.c +util/perf-regs-arch/perf_regs_riscv.c +util/perf-regs-arch/perf_regs_s390.c +util/perf-regs-arch/perf_regs_x86.c
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 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@toolbox perf-tools-next]$ I'm trying to fix
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) > 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@toolbox perf-tools-next]$ > > I'm trying to fix
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! > > 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@toolbox perf-tools-next]$ > > > > I'm trying to fix > > -- > > - Arnaldo
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@five ~]$ 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@five ~]$ 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. 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? - 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@toolbox perf-tools-next]$ > > > > > > I'm trying to fix > > > > -- > > > > - Arnaldo
Em Wed, Aug 16, 2023 at 08:46:23AM -0300, Arnaldo Carvalho de Melo escreveu: > 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@five ~]$ 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@five ~]$ > > 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 added this to the cset commit message: Committer notes: Make DWARF_MINIMAL_REGS() an inline function, so that we can use the __maybe_unused attribute for the 'arch' parameter, as this will avoid a build failure when that variable is unused in the callers. That happens when building on unsupported architectures, the ones without HAVE_PERF_REGS_SUPPORT defined. > 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? > > - 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@toolbox perf-tools-next]$ > > > > > > > > I'm trying to fix > > > > > > -- > > > > > > - Arnaldo > > -- > > - Arnaldo
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@five ~]$ 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@five ~]$ > > 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@toolbox perf-tools-next]$ > > > > > > > > I'm trying to fix > > > > > > -- > > > > > > - Arnaldo > > -- > > - Arnaldo
On Wed, Aug 16, 2023 at 08:48:30AM -0300, Arnaldo Carvalho de Melo wrote: [...] > > 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 added this to the cset commit message: > > Committer notes: > > Make DWARF_MINIMAL_REGS() an inline function, so that we can use the > __maybe_unused attribute for the 'arch' parameter, as this will avoid a > build failure when that variable is unused in the callers. That happens > when building on unsupported architectures, the ones without > HAVE_PERF_REGS_SUPPORT defined. Looks good to me, thanks for fixing. Leo