[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ZIiFHJ01upokkYX3@kernel.org>
Date: Tue, 13 Jun 2023 12:02:52 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Adrian Hunter <adrian.hunter@...el.com>,
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>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
Andi Kleen <ak@...ux.intel.com>
Subject: Re: [PATCH v1 1/2] perf tests x86: Generate entire instruction
struct in C files
Em Tue, Jun 13, 2023 at 07:25:20AM -0700, Ian Rogers escreveu:
> On Tue, Jun 13, 2023 at 6:22 AM Adrian Hunter <adrian.hunter@...el.com> wrote:
> >
> > On 12/06/23 22:08, Arnaldo Carvalho de Melo wrote:
> > > Em Wed, May 31, 2023 at 08:43:32AM -0700, Ian Rogers escreveu:
> > >> Generate the entire struct in the C files. Later changes will break
> > >> apart the struct and so two phases of output are necessary, this isn't
> > >> possible if part of the struct is declared in insn-x86.c.
> > >
> > > Adrian,
> > >
> > > Could you please take a look at these two patches?
> >
> > One of the considerations when adding the generated code
> > was that it wouldn't have to be changed because the instructions
> > do not change.
> >
> > I would much prefer to move the test out of the default perf build.
> >
> > Here is a patch to do that:
>
> I'm happy to add Acked-by to the patch. Some other thoughts.
Thanks, applied and added Ian's Acked-by.
- Arnaldo
> We build libperf tests as standalone executables.:
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/lib/perf/Makefile?h=perf-tools-next#n144
> this causes issues as we don't run the tests.
>
> We run jevents tests as part of the build, there is an output test log
> target that is a dependency on building pmu-events.c:
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/pmu-events/Build?h=perf-tools-next#n30
> This copies a pattern in other kernel build files. With tests written
> in C we'd need to make sure the test was host compiled to run it.
>
> Thanks,
> Ian
>
> > From: Adrian Hunter <adrian.hunter@...el.com>
> > Date: Tue, 13 Jun 2023 15:15:58 +0300
> > Subject: [PATCH] perf tests: Make x86 new instructions test optional at build
> > time
> >
> > The "x86 instruction decoder - new instructions" test takes up space but
> > is only really useful to developers. Make it optional at build time.
> >
> > Add variable EXTRA_TESTS which must be defined in order to build perf
> > with the test.
> >
> > Example:
> >
> > Before:
> >
> > $ make -C tools/perf clean >/dev/null
> > $ make -C tools/perf >/dev/null
> > Makefile.config:650: No libunwind found. Please install libunwind-dev[el] >= 1.1 and/or set LIBUNWIND_DIR
> > Makefile.config:1149: libpfm4 not found, disables libpfm4 support. Please install libpfm4-dev
> > PERF_VERSION = 6.4.rc3.gd15b8c76c964
> > $ readelf -SW tools/perf/perf | grep '\.rela.dyn\|.rodata\|\.data.rel.ro'
> > [10] .rela.dyn RELA 000000000002fcb0 02fcb0 0748b0 18 A 6 0 8
> > [18] .rodata PROGBITS 00000000002eb000 2eb000 6bac00 00 A 0 0 32
> > [25] .data.rel.ro PROGBITS 00000000009ea180 9e9180 04b540 00 WA 0 0 32
> >
> > After:
> >
> > $ make -C tools/perf clean >/dev/null
> > $ make -C tools/perf >/dev/null
> > Makefile.config:650: No libunwind found. Please install libunwind-dev[el] >= 1.1 and/or set LIBUNWIND_DIR
> > Makefile.config:1154: libpfm4 not found, disables libpfm4 support. Please install libpfm4-dev
> > PERF_VERSION = 6.4.rc3.g4ea9c1569ea4
> > $ readelf -SW tools/perf/perf | grep '\.rela.dyn\|.rodata\|\.data.rel.ro'
> > [10] .rela.dyn RELA 000000000002f3c8 02f3c8 036d68 18 A 6 0 8
> > [18] .rodata PROGBITS 00000000002ac000 2ac000 68da80 00 A 0 0 32
> > [25] .data.rel.ro PROGBITS 000000000097d440 97c440 022280 00 WA 0 0 32
> >
> > Signed-off-by: Adrian Hunter <adrian.hunter@...el.com>
> > ---
> > tools/perf/Makefile.config | 5 +++++
> > tools/perf/Makefile.perf | 4 ++++
> > tools/perf/arch/x86/include/arch-tests.h | 2 ++
> > tools/perf/arch/x86/tests/Build | 5 ++++-
> > tools/perf/arch/x86/tests/arch-tests.c | 4 ++++
> > tools/perf/tests/make | 1 +
> > 6 files changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index a794d9eca93d..9c5aa14a44cf 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -1075,6 +1075,11 @@ ifndef NO_AUXTRACE
> > endif
> > endif
> >
> > +ifdef EXTRA_TESTS
> > + $(call detected,CONFIG_EXTRA_TESTS)
> > + CFLAGS += -DHAVE_EXTRA_TESTS
> > +endif
> > +
> > ifndef NO_JVMTI
> > ifneq (,$(wildcard /usr/sbin/update-java-alternatives))
> > JDIR=$(shell /usr/sbin/update-java-alternatives -l | head -1 | awk '{print $$3}')
> > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > index f48794816d82..b1e62a621f92 100644
> > --- a/tools/perf/Makefile.perf
> > +++ b/tools/perf/Makefile.perf
> > @@ -128,6 +128,10 @@ include ../scripts/utilities.mak
> > #
> > # Define BUILD_NONDISTRO to enable building an linking against libbfd and
> > # libiberty distribution license incompatible libraries.
> > +#
> > +# Define EXTRA_TESTS to enable building extra tests useful mainly to perf
> > +# developers, such as:
> > +# x86 instruction decoder - new instructions test
> >
> > # As per kernel Makefile, avoid funny character set dependencies
> > unexport LC_ALL
> > diff --git a/tools/perf/arch/x86/include/arch-tests.h b/tools/perf/arch/x86/include/arch-tests.h
> > index 33d39c1d3e64..df133020d582 100644
> > --- a/tools/perf/arch/x86/include/arch-tests.h
> > +++ b/tools/perf/arch/x86/include/arch-tests.h
> > @@ -6,7 +6,9 @@ struct test_suite;
> >
> > /* Tests */
> > int test__rdpmc(struct test_suite *test, int subtest);
> > +#ifdef HAVE_EXTRA_TESTS
> > int test__insn_x86(struct test_suite *test, int subtest);
> > +#endif
> > int test__intel_pt_pkt_decoder(struct test_suite *test, int subtest);
> > int test__intel_pt_hybrid_compat(struct test_suite *test, int subtest);
> > int test__bp_modify(struct test_suite *test, int subtest);
> > diff --git a/tools/perf/arch/x86/tests/Build b/tools/perf/arch/x86/tests/Build
> > index 08cc8b9c931e..394771c00dca 100644
> > --- a/tools/perf/arch/x86/tests/Build
> > +++ b/tools/perf/arch/x86/tests/Build
> > @@ -4,5 +4,8 @@ perf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
> > perf-y += arch-tests.o
> > perf-y += sample-parsing.o
> > perf-y += hybrid.o
> > -perf-$(CONFIG_AUXTRACE) += insn-x86.o intel-pt-test.o
> > +perf-$(CONFIG_AUXTRACE) += intel-pt-test.o
> > +ifeq ($(CONFIG_EXTRA_TESTS),y)
> > +perf-$(CONFIG_AUXTRACE) += insn-x86.o
> > +endif
> > perf-$(CONFIG_X86_64) += bp-modify.o
> > diff --git a/tools/perf/arch/x86/tests/arch-tests.c b/tools/perf/arch/x86/tests/arch-tests.c
> > index 147ad0638bbb..3f2b90c59f92 100644
> > --- a/tools/perf/arch/x86/tests/arch-tests.c
> > +++ b/tools/perf/arch/x86/tests/arch-tests.c
> > @@ -4,7 +4,9 @@
> > #include "arch-tests.h"
> >
> > #ifdef HAVE_AUXTRACE_SUPPORT
> > +#ifdef HAVE_EXTRA_TESTS
> > DEFINE_SUITE("x86 instruction decoder - new instructions", insn_x86);
> > +#endif
> >
> > static struct test_case intel_pt_tests[] = {
> > TEST_CASE("Intel PT packet decoder", intel_pt_pkt_decoder),
> > @@ -37,7 +39,9 @@ struct test_suite *arch_tests[] = {
> > &suite__dwarf_unwind,
> > #endif
> > #ifdef HAVE_AUXTRACE_SUPPORT
> > +#ifdef HAVE_EXTRA_TESTS
> > &suite__insn_x86,
> > +#endif
> > &suite__intel_pt,
> > #endif
> > #if defined(__x86_64__)
> > diff --git a/tools/perf/tests/make b/tools/perf/tests/make
> > index 8dd3f8090352..885cd321d67b 100644
> > --- a/tools/perf/tests/make
> > +++ b/tools/perf/tests/make
> > @@ -69,6 +69,7 @@ make_clean_all := clean all
> > make_python_perf_so := $(python_perf_so)
> > make_debug := DEBUG=1
> > make_nondistro := BUILD_NONDISTRO=1
> > +make_extra_tests := EXTRA_TESTS=1
> > make_no_libperl := NO_LIBPERL=1
> > make_no_libpython := NO_LIBPYTHON=1
> > make_no_scripts := NO_LIBPYTHON=1 NO_LIBPERL=1
> > --
> > 2.34.1
> >
> >
--
- Arnaldo
Powered by blists - more mailing lists