diff mbox series

[04/15] perf: fix buildpaths QA warning in 6.4+

Message ID 1c4a47b33bed622c5b7c137a830e359302293887.1688958789.git.bruce.ashfield@gmail.com
State Accepted, archived
Commit b9f1d1ec162531e0ce59ea829ae570ca907b3448
Headers show
Series [01/15] linux-yocto/6.1: update to v6.1.36 | expand

Commit Message

Bruce Ashfield July 10, 2023, 3:20 a.m. UTC
From: Bruce Ashfield <bruce.ashfield@gmail.com>

kernel version 6.4 introduces a new file that need to have
absolute paths removed, so we can avoid the buildpaths QA
warning and have relocatable packages.

We add pmu-flex.h to the processing, and the issue is resolved.

Signed-off-by: Bruce Ashfield <bruce.ashfield@gmail.com>
---
 meta/recipes-kernel/perf/perf.bb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Rasmus Villemoes Oct. 10, 2023, 11:44 a.m. UTC | #1
On 10/07/2023 05.20, Bruce Ashfield via lists.openembedded.org wrote:
> From: Bruce Ashfield <bruce.ashfield@gmail.com>
> 
> kernel version 6.4 introduces a new file that need to have
> absolute paths removed, so we can avoid the buildpaths QA
> warning and have relocatable packages.
> 
> We add pmu-flex.h to the processing, and the issue is resolved.

Hi Bruce

We've seen those buildpaths qa warning when building perf for a long
time, and they happen whether we build against a 5.15, 6.2 or 6.5
kernel. More specifically, what we see is

WARNING: perf-1.0-r9 do_package_qa: QA Issue: File /usr/bin/.debug/perf
in package perf-dbg contains reference to TMPDIR [buildpaths]
WARNING: perf-1.0-r9 do_package_qa: QA Issue: File /usr/bin/trace in
package perf contains reference to TMPDIR
File /usr/bin/perf in package perf contains reference to TMPDIR [buildpaths]

Looking at the .o.cmd files, there is indeed nowhere a
-fmacro-prefix-map or -fdebug-prefix-map, and apart from the bison/yacc
stuff, I also don't see where the perf recipe attempts to pass such flags.

It seems we can fix it with

$ git diff
diff --git a/meta/recipes-kernel/perf/perf.bb
b/meta/recipes-kernel/perf/perf.bb
index 420286e..59c0c10 100644
--- a/meta/recipes-kernel/perf/perf.bb
+++ b/meta/recipes-kernel/perf/perf.bb
@@ -81,7 +81,7 @@ EXTRA_OEMAKE = '\
     LDSHARED="${CC} -shared" \
     AR="${AR}" \
     LD="${LD}" \
-    EXTRA_CFLAGS="-ldw -I${S}" \
+    EXTRA_CFLAGS="-ldw -I${S} ${DEBUG_PREFIX_MAP}" \
     YFLAGS='-y
--file-prefix-map=${WORKDIR}=/usr/src/debug/${PN}/${EXTENDPE}${PV}-${PR}' \
     EXTRA_LDFLAGS="${PERF_EXTRA_LDFLAGS}" \
     perfexecdir=${libexecdir} \

but I'm quite curious what the difference between our setups is, since
apparently the only problem oe-core has/had was that new file introduced
in v6.4.

FWIW, what we build are vanilla -stable kernels, i.e. "6.2" means
v6.2.16 as tagged by Greg. The only modification of the perf recipe we
do is removing "scripting" from PACKAGECONFIG.

Rasmus
Bruce Ashfield Oct. 10, 2023, 12:31 p.m. UTC | #2
On Tue, Oct 10, 2023 at 7:44 AM Rasmus Villemoes <rasmus.villemoes@prevas.dk>
wrote:

> On 10/07/2023 05.20, Bruce Ashfield via lists.openembedded.org wrote:
> > From: Bruce Ashfield <bruce.ashfield@gmail.com>
> >
> > kernel version 6.4 introduces a new file that need to have
> > absolute paths removed, so we can avoid the buildpaths QA
> > warning and have relocatable packages.
> >
> > We add pmu-flex.h to the processing, and the issue is resolved.
>
> Hi Bruce
>
> We've seen those buildpaths qa warning when building perf for a long
> time, and they happen whether we build against a 5.15, 6.2 or 6.5
> kernel. More specifically, what we see is
>
> WARNING: perf-1.0-r9 do_package_qa: QA Issue: File /usr/bin/.debug/perf
> in package perf-dbg contains reference to TMPDIR [buildpaths]
> WARNING: perf-1.0-r9 do_package_qa: QA Issue: File /usr/bin/trace in
> package perf contains reference to TMPDIR
> File /usr/bin/perf in package perf contains reference to TMPDIR
> [buildpaths]
>
> Looking at the .o.cmd files, there is indeed nowhere a
> -fmacro-prefix-map or -fdebug-prefix-map, and apart from the bison/yacc
> stuff, I also don't see where the perf recipe attempts to pass such flags.
>
> It seems we can fix it with
>
> $ git diff
> diff --git a/meta/recipes-kernel/perf/perf.bb
> b/meta/recipes-kernel/perf/perf.bb
> index 420286e..59c0c10 100644
> --- a/meta/recipes-kernel/perf/perf.bb
> +++ b/meta/recipes-kernel/perf/perf.bb
> @@ -81,7 +81,7 @@ EXTRA_OEMAKE = '\
>      LDSHARED="${CC} -shared" \
>      AR="${AR}" \
>      LD="${LD}" \
> -    EXTRA_CFLAGS="-ldw -I${S}" \
> +    EXTRA_CFLAGS="-ldw -I${S} ${DEBUG_PREFIX_MAP}" \
>      YFLAGS='-y
> --file-prefix-map=${WORKDIR}=/usr/src/debug/${PN}/${EXTENDPE}${PV}-${PR}' \
>      EXTRA_LDFLAGS="${PERF_EXTRA_LDFLAGS}" \
>      perfexecdir=${libexecdir} \
>
> but I'm quite curious what the difference between our setups is, since
> apparently the only problem oe-core has/had was that new file introduced
> in v6.4.
>

You can see any patches that we have against perf in the kernel-cache
repository (https://git.yoctoproject.org/yocto-kernel-cache/). That being
said, we unfortunately cannot really patch perf, since the recipe is
supported against more than our reference kernels.

The debug prefix didn't initially work on some kernels (and hence perf),
so we went with the more manual approach.

It is very possible that with a newer window of supported kernels that
we could change how we deal with TMPDIR.

Other than that, there's no special setup (in particular no setup that
I have) as it is the autobuilder infrastructure that typically finds and
reports these issues.



>
> FWIW, what we build are vanilla -stable kernels, i.e. "6.2" means
> v6.2.16 as tagged by Greg. The only modification of the perf recipe we
> do is removing "scripting" from PACKAGECONFIG.
>
>
FWIW, you could save some effort and simply build v6.<x>/base on
linux-yocto and you'd have exactly the same source.

Bruce



> Rasmus
>
>
Rasmus Villemoes Oct. 10, 2023, 2:37 p.m. UTC | #3
On 10/10/2023 14.31, Bruce Ashfield wrote:
> 
> 
> On Tue, Oct 10, 2023 at 7:44 AM Rasmus Villemoes
>     It seems we can fix it with
> 
>     $ git diff
>     diff --git a/meta/recipes-kernel/perf/perf.bb <http://perf.bb>
>     b/meta/recipes-kernel/perf/perf.bb <http://perf.bb>
>     index 420286e..59c0c10 100644
>     --- a/meta/recipes-kernel/perf/perf.bb <http://perf.bb>
>     +++ b/meta/recipes-kernel/perf/perf.bb <http://perf.bb>
>     @@ -81,7 +81,7 @@ EXTRA_OEMAKE = '\
>          LDSHARED="${CC} -shared" \
>          AR="${AR}" \
>          LD="${LD}" \
>     -    EXTRA_CFLAGS="-ldw -I${S}" \
>     +    EXTRA_CFLAGS="-ldw -I${S} ${DEBUG_PREFIX_MAP}" \
>          YFLAGS='-y
>     --file-prefix-map=${WORKDIR}=/usr/src/debug/${PN}/${EXTENDPE}${PV}-${PR}' \
>          EXTRA_LDFLAGS="${PERF_EXTRA_LDFLAGS}" \
>          perfexecdir=${libexecdir} \
> 
>     but I'm quite curious what the difference between our setups is, since
>     apparently the only problem oe-core has/had was that new file introduced
>     in v6.4.
> 
> 
> You can see any patches that we have against perf in the kernel-cache
> repository (https://git.yoctoproject.org/yocto-kernel-cache/
> <https://git.yoctoproject.org/yocto-kernel-cache/>). That being
> said, we unfortunately cannot really patch perf, since the recipe is
> supported against more than our reference kernels.

I didn't suggest to patch perf, I suggested to patch the perf recipe.
> 
> Other than that, there's no special setup (in particular no setup that
> I have) as it is the autobuilder infrastructure that typically finds and
> reports these issues.

So I found the difference, and those autobuilder builds do in fact make
the compiler use all the right -f*-prefix-map options. It's because
meta/conf/distro/include/security_flags.inc contains

TARGET_CC_ARCH:append:pn-perf = " ${SELECTED_OPTIMIZATION}"

and that includes the DEBUG_PREFIX_MAP setting, and in turn gets
included in the CC setting via HOST_CC_ARCH. And of course the poky
distro includes that security_flags.inc file, while our custom distro
does not (up until five minutes ago, I never knew that file existed).

Of course one wonders why those :append:pn-* settings that don't
actually append any security related flags, but do affect stuff like the
recipes passing QA, are hidden away in a file called security_flags.inc
instead of those settings just being directly in the recipes themselves.
There's no explanation in oe-core e93765ffb57 ; it removed a whole bunch
of append_pn-* stuff which had apparently become redundant (the commit
log hints as much), but doesn't offer any explanation as to why pn-perf
should grow that setting. And AFAICT that long predates the invention of
the -f*-prefix-map flags, so it seems to be mostly by accident that
perf, with the poky distro, actually does build with those flags.

Rasmus
Bruce Ashfield Oct. 10, 2023, 2:58 p.m. UTC | #4
On Tue, Oct 10, 2023 at 10:37 AM Rasmus Villemoes <
rasmus.villemoes@prevas.dk> wrote:

> On 10/10/2023 14.31, Bruce Ashfield wrote:
> >
> >
> > On Tue, Oct 10, 2023 at 7:44 AM Rasmus Villemoes
> >     It seems we can fix it with
> >
> >     $ git diff
> >     diff --git a/meta/recipes-kernel/perf/perf.bb <http://perf.bb>
> >     b/meta/recipes-kernel/perf/perf.bb <http://perf.bb>
> >     index 420286e..59c0c10 100644
> >     --- a/meta/recipes-kernel/perf/perf.bb <http://perf.bb>
> >     +++ b/meta/recipes-kernel/perf/perf.bb <http://perf.bb>
> >     @@ -81,7 +81,7 @@ EXTRA_OEMAKE = '\
> >          LDSHARED="${CC} -shared" \
> >          AR="${AR}" \
> >          LD="${LD}" \
> >     -    EXTRA_CFLAGS="-ldw -I${S}" \
> >     +    EXTRA_CFLAGS="-ldw -I${S} ${DEBUG_PREFIX_MAP}" \
> >          YFLAGS='-y
> >
>  --file-prefix-map=${WORKDIR}=/usr/src/debug/${PN}/${EXTENDPE}${PV}-${PR}' \
> >          EXTRA_LDFLAGS="${PERF_EXTRA_LDFLAGS}" \
> >          perfexecdir=${libexecdir} \
> >
> >     but I'm quite curious what the difference between our setups is,
> since
> >     apparently the only problem oe-core has/had was that new file
> introduced
> >     in v6.4.
> >
> >
> > You can see any patches that we have against perf in the kernel-cache
> > repository (https://git.yoctoproject.org/yocto-kernel-cache/
> > <https://git.yoctoproject.org/yocto-kernel-cache/>). That being
> > said, we unfortunately cannot really patch perf, since the recipe is
> > supported against more than our reference kernels.
>
> I didn't suggest to patch perf, I suggested to patch the perf recipe.
>

I also didn't mean that. You asked what the differences might
be in our setup, which largely means linux-yocto. Hence why
the patches would be relevant to that question.


> >
> > Other than that, there's no special setup (in particular no setup that
> > I have) as it is the autobuilder infrastructure that typically finds and
> > reports these issues.
>
> So I found the difference, and those autobuilder builds do in fact make
> the compiler use all the right -f*-prefix-map options. It's because
> meta/conf/distro/include/security_flags.inc contains
>
> TARGET_CC_ARCH:append:pn-perf = " ${SELECTED_OPTIMIZATION}"
>
> and that includes the DEBUG_PREFIX_MAP setting, and in turn gets
> included in the CC setting via HOST_CC_ARCH. And of course the poky
> distro includes that security_flags.inc file, while our custom distro
> does not (up until five minutes ago, I never knew that file existed).
>
> Of course one wonders why those :append:pn-* settings that don't
> actually append any security related flags, but do affect stuff like the
> recipes passing QA, are hidden away in a file called security_flags.inc
> instead of those settings just being directly in the recipes themselves.
> There's no explanation in oe-core e93765ffb57 ; it removed a whole bunch
> of append_pn-* stuff which had apparently become redundant (the commit
> log hints as much), but doesn't offer any explanation as to why pn-perf
> should grow that setting. And AFAICT that long predates the invention of
> the -f*-prefix-map flags, so it seems to be mostly by accident that
> perf, with the poky distro, actually does build with those flags.
>

The settings and debug maps are typically distro policies, so that's
why they were historically controlled there. If we pull it out to the
recipe, we'd want to check a similar distro flag to enable/disable
(mainly for debug purposes).

I wouldn't say by accident at all, but there's been a lot of changes
over time, so there's bits and pieces that are stitched together
differently, with variable names that have had their meaning change
over time.

Bruce



>
> Rasmus
>
>
diff mbox series

Patch

diff --git a/meta/recipes-kernel/perf/perf.bb b/meta/recipes-kernel/perf/perf.bb
index 2d803381bb..7d90ac3612 100644
--- a/meta/recipes-kernel/perf/perf.bb
+++ b/meta/recipes-kernel/perf/perf.bb
@@ -383,7 +383,7 @@  PACKAGESPLITFUNCS =+ "perf_fix_sources"
 
 perf_fix_sources () {
 	for f in util/parse-events-flex.h util/parse-events-flex.c util/pmu-flex.c \
-			util/expr-flex.h util/expr-flex.c; do
+			util/pmu-flex.h util/expr-flex.h util/expr-flex.c; do
 		f=${PKGD}/usr/src/debug/${PN}/${EXTENDPE}${PV}-${PR}/$f
 		if [ -e $f ]; then
 			sed -i -e 's#${S}/##g' $f