From: Jiri Olsa <olsajiri@gmail.com>
To: "Thomas Weißschuh" <linux@weissschuh.net>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
Masahiro Yamada <masahiroy@kernel.org>,
Nathan Chancellor <nathan@kernel.org>,
Nicolas Schier <nicolas@fjasle.eu>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>,
Martin KaFai Lau <martin.lau@linux.dev>,
Eduard Zingerman <eddyz87@gmail.com>, Song Liu <song@kernel.org>,
Yonghong Song <yonghong.song@linux.dev>,
John Fastabend <john.fastabend@gmail.com>,
KP Singh <kpsingh@kernel.org>,
Stanislav Fomichev <sdf@fomichev.me>, Hao Luo <haoluo@google.com>,
Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH 3/3] kbuild: propagate CONFIG_WERROR to resolve_btfids
Date: Mon, 25 Nov 2024 10:33:25 +0100 [thread overview]
Message-ID: <Z0REZczFIfGHtjsQ@krava> (raw)
In-Reply-To: <f7764e9b-6254-42af-94b8-41562a18b58b@t-8ch.de>
On Mon, Nov 25, 2024 at 09:20:37AM +0100, Thomas Weißschuh wrote:
> On 2024-11-24 15:38:40-0800, Alexei Starovoitov wrote:
> > On Sat, Nov 23, 2024 at 5:33 AM Thomas Weißschuh <linux@weissschuh.net> wrote:
> > >
> > > Use CONFIG_WERROR to also fail on warnings emitted by resolve_btfids.
> > > Allow the CI bots to prevent the introduction of new warnings.
> > >
> > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > > ---
> > > scripts/link-vmlinux.sh | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > > index a9b3f34a78d2cd4514e73a728f1a784eee891768..61f1f670291351a276221153146d66001eca556c 100755
> > > --- a/scripts/link-vmlinux.sh
> > > +++ b/scripts/link-vmlinux.sh
> > > @@ -274,7 +274,11 @@ vmlinux_link vmlinux
> > > # fill in BTF IDs
> > > if is_enabled CONFIG_DEBUG_INFO_BTF; then
> > > info BTFIDS vmlinux
> > > - ${RESOLVE_BTFIDS} vmlinux
> > > + RESOLVE_BTFIDS_ARGS=""
> > > + if is_enabled CONFIG_WERROR; then
> > > + RESOLVE_BTFIDS_ARGS=" --fatal-warnings "
> > > + fi
> > > + ${RESOLVE_BTFIDS} ${RESOLVE_BTFIDS_ARGS} vmlinux
> >
> > I'm not convinced we need to fail the build when functions are renamed.
> > These warns are eventually found and fixed.
>
> The same could be said for most other build warnings.
> CONFIG_WERROR is a well-known opt-in switch for exactly this behavior.
>
> Fixing these warnings before they hit mainline has various
> advantages. The author introducing the warning knows about the full
> impact of their change, discussions can be had when everybody still
> has the topic fresh on their mind and other unrelated people don't get
> confused, like me or [0].
>
> The "eventually fixed" part seems to have been me the last two times :-)
>
> Given the fairly simple implementation, in my opinion this is worth doing.
>
> Please note that I have two fairly trivial changes for a v2 and would
> also like to get some feedback from Masahiro, especially for patch 1.
ok, I think it's fine to fail for CONFIG_WERROR option, for patchset:
Acked-by: Jiri Olsa <jolsa@kernel.org>
thanks,
jirka
>
>
> Thomas
>
> [0] https://lore.kernel.org/lkml/20241113093703.9936-1-laura.nao@collabora.com/
prev parent reply other threads:[~2024-11-25 9:33 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-23 13:33 [PATCH 0/3] kbuild: propagate CONFIG_WERROR to resolve_btfids Thomas Weißschuh
2024-11-23 13:33 ` [PATCH 1/3] kbuild: add dependency from vmlinux " Thomas Weißschuh
2024-11-24 20:33 ` Jiri Olsa
2024-11-24 20:57 ` Thomas Weißschuh
2024-11-25 8:35 ` Masahiro Yamada
2024-11-26 16:52 ` Masahiro Yamada
2024-11-23 13:33 ` [PATCH 2/3] tools/resolve_btfids: Add --fatal-warnings option Thomas Weißschuh
2024-11-23 13:33 ` [PATCH 3/3] kbuild: propagate CONFIG_WERROR to resolve_btfids Thomas Weißschuh
2024-11-24 23:38 ` Alexei Starovoitov
2024-11-25 8:20 ` Thomas Weißschuh
2024-11-25 9:33 ` Jiri Olsa [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z0REZczFIfGHtjsQ@krava \
--to=olsajiri@gmail.com \
--cc=alexei.starovoitov@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=kpsingh@kernel.org \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@weissschuh.net \
--cc=martin.lau@linux.dev \
--cc=masahiroy@kernel.org \
--cc=nathan@kernel.org \
--cc=nicolas@fjasle.eu \
--cc=sdf@fomichev.me \
--cc=song@kernel.org \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.