On Sun, Dec 06, 2020 at 11:45:27PM +0900, Chanho Park wrote:
> __always_inline can cause build error on musl libc. The fix patch has
> submitted but not merged yet[1]. To build perf tool with musl libc,
> <linux/stddef.h> inclusion is necessary and it should be included before
> perf_event.h.
>
> from /usr/include/linux/byteorder/little_endian.h:13,
> from /usr/include/asm/byteorder.h:23,
> from tools/include/uapi/linux/perf_event.h:20,
> from arch/arm64/util/../../arm/util/pmu.c:9:
>
> /usr/include/linux/swab.h:171:8: error: unknown type name '__always_inline'
> 171 | static __always_inline __u16 __swab16p(const __u16 *p)
> | ^~~~~~~~~~~~~~~
>
> [1]: https://lkml.org/lkml/2018/9/13/78
> Cc: Mathieu Poirier <[email protected]>
> Cc: Suzuki K Poulose <[email protected]>
> Cc: Mike Leach <[email protected]>
> Cc: Leo Yan <[email protected]>
> Cc: John Garry <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Khem Raj <[email protected]>
> Signed-off-by: Chanho Park <[email protected]>
> ---
> tools/perf/arch/arm/util/pmu.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/tools/perf/arch/arm/util/pmu.c b/tools/perf/arch/arm/util/pmu.c
> index bbc297a7e2e3..4c0357e8c0ab 100644
> --- a/tools/perf/arch/arm/util/pmu.c
> +++ b/tools/perf/arch/arm/util/pmu.c
> @@ -6,6 +6,9 @@
>
> #include <string.h>
> #include <linux/coresight-pmu.h>
> +#if !defined(__GLIBC__)
> +#include <linux/stddef.h>
> +#endif
Looks like other files just include this unconditionally, but have a comment
explaining why. See util/branch.h and util/event.h. Maybe we should do the
same for util/pmu.h, which is already included here?
Will
Hi Will,
> Looks like other files just include this unconditionally, but have a
> comment explaining why. See util/branch.h and util/event.h. Maybe we
> should do the same for util/pmu.h, which is already included here?
I found below files which perf includes <linux/perf_event.h>. Instead of
doing same for all, we'd better put this only for
tools/include/uapi/linux/perf_event.h.
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -17,6 +17,9 @@
#include <linux/types.h>
#include <linux/ioctl.h>
+#if !defined(__GLIBC__)
+#include <linux/stddef.h>
+#endif
#include <asm/byteorder.h>
/*
tools/perf/arch/arm/util/pmu.c:#include <linux/perf_event.h>
tools/perf/arch/x86/util/pmu.c:#include <linux/perf_event.h>
tools/perf/arch/x86/util/tsc.c:#include <linux/perf_event.h>
tools/perf/lib/include/internal/evsel.h:#include <linux/perf_event.h>
tools/perf/lib/include/perf/event.h:#include <linux/perf_event.h>
tools/perf/lib/tests/test-evlist.c:#include <linux/perf_event.h>
tools/perf/lib/tests/test-evsel.c:#include <linux/perf_event.h>
tools/perf/tests/hists_common.c:#include <linux/perf_event.h>
tools/perf/util/auxtrace.c:#include <linux/perf_event.h>
tools/perf/util/auxtrace.h:#include <linux/perf_event.h>
tools/perf/util/branch.h:#include <linux/perf_event.h>
tools/perf/util/event.c:#include <linux/perf_event.h>
tools/perf/util/evsel.c:#include <linux/perf_event.h>
tools/perf/util/evsel.h:#include <linux/perf_event.h>
tools/perf/util/header.h:#include <linux/perf_event.h>
tools/perf/util/mem-events.h:#include <linux/perf_event.h>
tools/perf/util/namespaces.h:#include <linux/perf_event.h>
tools/perf/util/record.h:#include <linux/perf_event.h>
tools/perf/util/session.h:#include <linux/perf_event.h>
tools/perf/util/pmu.h:#include <linux/perf_event.h>
tools/perf/util/synthetic-events.c:#include <linux/perf_event.h>
tools/perf/util/parse-events.h:#include <linux/perf_event.h>
tools/perf/util/perf_event_attr_fprintf.c:#include <linux/perf_event.h>
Best Regards,
Chanho Park
Hi Chanho,
On Mon, Dec 7, 2020 at 8:58 PM Chanho Park <[email protected]> wrote:
>
> Hi Will,
>
> > Looks like other files just include this unconditionally, but have a
> > comment explaining why. See util/branch.h and util/event.h. Maybe we
> > should do the same for util/pmu.h, which is already included here?
>
> I found below files which perf includes <linux/perf_event.h>. Instead of
> doing same for all, we'd better put this only for
> tools/include/uapi/linux/perf_event.h.
It's a copy of the kernel header, I'm not sure we want to add something there.
Thanks,
Namhyung
On Mon, Dec 07, 2020 at 09:31:06PM +0900, Namhyung Kim wrote:
> Hi Chanho,
>
> On Mon, Dec 7, 2020 at 8:58 PM Chanho Park <[email protected]> wrote:
> >
> > Hi Will,
> >
> > > Looks like other files just include this unconditionally, but have a
> > > comment explaining why. See util/branch.h and util/event.h. Maybe we
> > > should do the same for util/pmu.h, which is already included here?
> >
> > I found below files which perf includes <linux/perf_event.h>. Instead of
> > doing same for all, we'd better put this only for
> > tools/include/uapi/linux/perf_event.h.
>
> It's a copy of the kernel header, I'm not sure we want to add something there.
right, we want to copy that directly from kernel uapi
so let's not do any changes in here
jirka
Hi Namhyung and Jiri,
> -----Original Message-----
> From: Jiri Olsa <[email protected]>
> Sent: Monday, December 7, 2020 10:29 PM
> To: Namhyung Kim <[email protected]>
> Cc: Chanho Park <[email protected]>; Will Deacon
<[email protected]>;
> Chanho Park <[email protected]>; [email protected];
> [email protected]; [email protected]; linux-arm-
> [email protected]; linux-kernel <[email protected]>;
> Mike Leach <[email protected]>; Leo Yan <[email protected]>; John
> Garry <[email protected]>; Peter Zijlstra <[email protected]>; Ingo
> Molnar <[email protected]>; Arnaldo Carvalho de Melo <[email protected]>;
> Mark Rutland <[email protected]>; Khem Raj <[email protected]>
> Subject: Re: [PATCH] perf arm pmu: fix build error on MUSL libc
>
> On Mon, Dec 07, 2020 at 09:31:06PM +0900, Namhyung Kim wrote:
> > Hi Chanho,
> >
> > On Mon, Dec 7, 2020 at 8:58 PM Chanho Park <[email protected]>
> wrote:
> > >
> > > Hi Will,
> > >
> > > > Looks like other files just include this unconditionally, but have
> > > > a comment explaining why. See util/branch.h and util/event.h.
> > > > Maybe we should do the same for util/pmu.h, which is already
> included here?
> > >
> > > I found below files which perf includes <linux/perf_event.h>.
> > > Instead of doing same for all, we'd better put this only for
> > > tools/include/uapi/linux/perf_event.h.
> >
> > It's a copy of the kernel header, I'm not sure we want to add something
> there.
>
> right, we want to copy that directly from kernel uapi so let's not do any
> changes in here
>
I tried to modify it from include/uapi/linux/perf_event.h but it didn't
work. And then, I found perf tool tried to refer the header from
tools/include/ directory. I should go include/uapi first and sync the change
to tools/ directory. I'll re-spin this patch.
Best Regard,
Chanho Park