On Fri, 2023-10-20 at 08:52 -0400, Bruce Ashfield wrote:
> On Fri, Oct 20, 2023 at 7:19 AM Rasmus Villemoes
> <rasmus.villem...@prevas.dk> wrote:
> >
> > On 20/10/2023 12.13, Richard Purdie wrote:
> > > On Fri, 2023-10-20 at 12:03 +0200, Rasmus Villemoes wrote:
> > > > On 20/10/2023 11.38, Richard Purdie wrote:
> > > > > On Fri, 2023-10-20 at 10:10 +0200, Rasmus Villemoes wrote:
> > > > > > On 19/10/2023 14.48, Richard Purdie wrote:
> > > >
> > > > > > > The fact this works suggests perf is ignoring TARGET_CFLAGS. Is
> > > > > > > there
> > > > > > > anything in the perf build system where we should be passing in
> > > > > > > cflags
> > > > > > > we really want used?
> > > > > >
> > > > > > Well, IIUC, the normal way TARGET_CFLAGS would take effect is via
> > > > > > the
> > > > > > 'export CFLAGS = "${TARGET_CFLAGS}'. But in the perf recipe, both
> > > > > > do_compile and do_install start with
> > > > > >
> > > > > > # Linux kernel build system is expected to do the right
> > > > > > thing
> > > > > > unset CFLAGS
> > > > > >
> > > > > > And perf's build system does indeed add lots of flags of its own, at
> > > > > > least for most TUs, but nowhere is any -f(macro/debug/file)-prefix
> > > > > > being
> > > > > > set.
> > > > > >
> > > > > > So I do think that TARGET_CC_ARCH is the best place for flags that
> > > > > > we
> > > > > > really want used. At least as an initial step, because this is
> > > > > > known to
> > > > > > work when using the security_flags.inc file. Maybe there's some
> > > > > > cleaner
> > > > > > way where we don't unset CFLAGS, but that could be a giant can of
> > > > > > worms.
> > > > > >
> > > > > > And yes, a comment should be added. Is something like
> > > > > >
> > > > > > # Perf's build system adds its own optimization flags for most TUs,
> > > > > > # overriding the flags included here. But for some, perf does not
> > > > > > add
> > > > > > # any -O option, so ensure the distro's chosen optimization gets
> > > > > > used
> > > > > > # for those. Since ${SELECTED_OPTIMIZATION} always includes
> > > > > > # ${DEBUG_FLAGS} which in turn includes ${DEBUG_PREFIX_MAP}, this
> > > > > > also
> > > > > > # ensures perf is built with appropriate -f*-prefix-map options,
> > > > > > # avoiding the 'buildpaths' QA warning.
> > > > > >
> > > > > > too verbose?
> > > > >
> > > > > If it were me I'd probably write:
> > > > >
> > > > > # The perf makefile has "unset CFLAGS" as "Linux kernel build system
> > > > > is
> > > > > # expected to do the right thing".
> > > >
> > > > It's not the perf makefile, it's right there in our recipe itself. I
> > > > don't think upstream perf is to blame.
> > >
> > > Sorry, I misunderstood. I thought this was the perf environment that
> > > was doing this so in that case your comment does make much more sense
> > > than mine.
> >
> > OK. Bruce, should I resend?
>
> Yes, let's go for a resend with the comment added. If we can avoid even some
> of
> the pain wondering about this again .. our future selves will thank us :)
>
> >
> > > Out of interest did you see what happens if you allow our CFLAGS in and
> > > remove that unset?
> >
> > I don't think I've tried that, but I've tried a few other things that I
> > thought would be cleaner but which then exploded, so in the end I
> > decided that just lifting exactly what the poky distro does should be
> > safest and have the highest likelyhood of being accepted.
>
> This is not directed at Rasmus, but a general comment ..
>
> For pretty much anything kernel, and perf has always strongly been in
> that category, we've always unset everything and allowed the Makefiles
> (and the nested mess of tests, includes and trickery) to set their own
> flags. It has saved subtle runtime issues and this is one area that the
> developers tend to know what they are doing.
For the kernel itself, yes, they do. For userspace binaries, we do in
general have a good idea of how binaries should be being built and
we're a bit ahead of the curve on the flags/features that should be
configured.
> When there's a chance to append, they've give us a variable to do so,
> that's why we've always used EXTRA_CFLAGS, and that likely is the
> right place to use here as well. But even then, we shouldn't pass ALL of
> our flags in there, just what we think needs to be in addition to the
> perf generated set of CFLAGS.
I'm not entirely convinced of that. Which CFLAGS do we have which would
be harmful to userspace binaries?
I'm basically saying we should at least investigate this as I'm not
sure which things we have in CFLAGS would be problematic to userspace
binaries in general. It may not work and that is fine but I don't think
perf userspace should be that sensitive to the things we usually have
set.
Cheers,
Richard
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#189539):
https://lists.openembedded.org/g/openembedded-core/message/189539
Mute This Topic: https://lists.openembedded.org/mt/102058904/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
