Re: [PATCH v7 7/9] perf: arm64: Add test for userspace counter access on heterogeneous systems

From: Masayoshi Mizuma
Date: Fri Apr 30 2021 - 16:16:35 EST


On Fri, Apr 30, 2021 at 01:20:58PM -0500, Rob Herring wrote:
> On Fri, Apr 30, 2021 at 1:17 PM Rob Herring <robh@xxxxxxxxxx> wrote:
> >
> > On Fri, Apr 30, 2021 at 11:46 AM Masayoshi Mizuma <msys.mizuma@xxxxxxxxx> wrote:
> > >
> > > On Mon, Apr 19, 2021 at 10:15:09PM -0500, Rob Herring wrote:
> > > > Userspace counter access only works on heterogeneous systems with some
> > > > restrictions. The userspace process must be pinned to a homogeneous
> > > > subset of CPUs and must open the corresponding PMU for those CPUs. This
> > > > commit adds a test implementing these requirements.
> > > >
> > > > Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
> > > > ---
> > > > v6:
> > > > - Add a check on cap_user_rdpmc
> > > > v5:
> > > > - Adapt to libperf mmap API changes
> > > > v4:
> > > > - Update perf_evsel__mmap params
> > > > v2:
> > > > - Drop all but heterogeneous test as others covered by libperf tests
> > > > - Rework to use libperf
> > > > ---
> > > > tools/perf/arch/arm64/include/arch-tests.h | 7 +
> > > > tools/perf/arch/arm64/tests/Build | 1 +
> > > > tools/perf/arch/arm64/tests/arch-tests.c | 4 +
> > > > tools/perf/arch/arm64/tests/user-events.c | 177 +++++++++++++++++++++
> > > > 4 files changed, 189 insertions(+)
> > > > create mode 100644 tools/perf/arch/arm64/tests/user-events.c
> > > >
> > > > diff --git a/tools/perf/arch/arm64/include/arch-tests.h b/tools/perf/arch/arm64/include/arch-tests.h
> > > > index 90ec4c8cb880..380ad34a3f09 100644
> > > > --- a/tools/perf/arch/arm64/include/arch-tests.h
> > > > +++ b/tools/perf/arch/arm64/include/arch-tests.h
> > > > @@ -2,11 +2,18 @@
> > > > #ifndef ARCH_TESTS_H
> > > > #define ARCH_TESTS_H
> > > >
> > > > +#include <linux/compiler.h>
> > > > +
> > > > #ifdef HAVE_DWARF_UNWIND_SUPPORT
> > > > struct thread;
> > > > struct perf_sample;
> > > > +int test__arch_unwind_sample(struct perf_sample *sample,
> > > > + struct thread *thread);
> > > > #endif
> > >
> > > Hello,
> > >
> > > I got the following compile error with aarch64 on Fedora33.
> > >
> > > # make tools/perf
> > > ...
> > > In file included from arch/arm64/tests/arch-tests.c:4:
> > > /root//libperf_v7/tools/perf/arch/arm64/include/arch-tests.h:10:5: error: redundant redeclaration of ‘test__arch_unwind_sample’ [-Werror=redundant-decls]
> > > 10 | int test__arch_unwind_sample(struct perf_sample *sample,
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~
> > > In file included from arch/arm64/tests/arch-tests.c:3:
> > > /root//libperf_v7/tools/perf/tests/tests.h:140:5: note: previous declaration of ‘test__arch_unwind_sample’ was here
> > > 140 | int test__arch_unwind_sample(struct perf_sample *sample,
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~
> > > cc1: all warnings being treated as errors
> > > make[8]: *** [/root//libperf_v7/tools/build/Makefile.build:97: /root/libperf_v7/tools/perf/arch/arm64/tests/arch-tests.o] Error 1
> > > make[8]: *** Waiting for unfinished jobs....
> > > In file included from arch/arm64/tests/user-events.c:13:
> > > /root//libperf_v7/tools/perf/arch/arm64/include/arch-tests.h:10:5: error: redundant redeclaration of ‘test__arch_unwind_sample’ [-Werror=redundant-decls]
> > > 10 | int test__arch_unwind_sample(struct perf_sample *sample,
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~
> > > In file included from arch/arm64/tests/user-events.c:12:
> > > /root//libperf_v7/tools/perf/tests/tests.h:140:5: note: previous declaration of ‘test__arch_unwind_sample’ was here
> > > 140 | int test__arch_unwind_sample(struct perf_sample *sample,
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~
> > > cc1: all warnings being treated as errors
> > > ...
> > >
> > > The error is gone after the following patch is applied.
> >
> > Thanks. Honestly, I'm not sure why it was there in the first place.
> > Looking at the git history and this series history doesn't give any
> > clues.
>
> Well, except that both x86 and powerpc have the same hunk in their
> arch-tests.h. Do you see errors on those arches?

I didn't see the errors on x86_64.
It seems that the errors happen on aarch64 because
test__arch_unwind_sample() is defined only if the arch
is arm or arm64 in tools/perf/tests/tests.h:

#if defined(__arm__) || defined(__aarch64__)
#ifdef HAVE_DWARF_UNWIND_SUPPORT
struct thread;
struct perf_sample;
int test__arch_unwind_sample(struct perf_sample *sample,
struct thread *thread);
#endif
#endif

The following patch may be another solution which is same way as
commit d8b167f9d8af ("perf tests: Move x86 tests into arch directory").

---
tools/perf/arch/arm64/tests/dwarf-unwind.c | 1 +
tools/perf/tests/dwarf-unwind.c | 2 +-
tools/perf/tests/tests.h | 2 +-
3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/arch/arm64/tests/dwarf-unwind.c b/tools/perf/arch/arm64/tests/dwarf-unwind.c
index 46147a483..02ba87f2b 100644
--- a/tools/perf/arch/arm64/tests/dwarf-unwind.c
+++ b/tools/perf/arch/arm64/tests/dwarf-unwind.c
@@ -7,6 +7,7 @@
#include "event.h"
#include "debug.h"
#include "tests/tests.h"
+#include "arch-tests.h"

#define STACK_SIZE 8192

diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
index 83638097c..daffe2d66 100644
--- a/tools/perf/tests/dwarf-unwind.c
+++ b/tools/perf/tests/dwarf-unwind.c
@@ -17,7 +17,7 @@
#include "callchain.h"
#include "util/synthetic-events.h"

-#if defined (__x86_64__) || defined (__i386__) || defined (__powerpc__)
+#if defined (__x86_64__) || defined (__i386__) || defined (__powerpc__) || defined(__aarch64__)
#include "arch-tests.h"
#endif

diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index b85f00530..40cbdfa46 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -133,7 +133,7 @@ bool test__bp_account_is_supported(void);
bool test__wp_is_supported(void);
bool test__tsc_is_supported(void);

-#if defined(__arm__) || defined(__aarch64__)
+#if defined(__arm__)
#ifdef HAVE_DWARF_UNWIND_SUPPORT
struct thread;
struct perf_sample;
--

Thanks,
Masa