* Re: [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today
2020-10-01 1:12 ` [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today Andrew Lunn
@ 2020-10-01 23:09 ` Nick Desaulniers
2020-10-02 1:44 ` Andrew Lunn
2020-10-02 9:04 ` kernel test robot
2020-10-02 11:08 ` kernel test robot
2 siblings, 1 reply; 20+ messages in thread
From: Nick Desaulniers @ 2020-10-01 23:09 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, David Miller, Jakub Kicinski, Masahiro Yamada,
Michal Marek, Rohit Maheshwari, Linux Kbuild mailing list,
clang-built-linux, Arnd Bergmann
On Wed, Sep 30, 2020 at 6:12 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> There is a movement to try to make more and more of /drivers W=1
> clean. But it will only stay clean if new warnings are quickly
> detected and fixed, ideally by the developer adding the new code.
>
> To allow subdirectories to sign up to being W=1 clean for a given
> definition of W=1, export the current set of additional compile flags
> using the symbol KBUILD_CFLAGS_W1_20200930. Subdirectory Makefiles can
> then use:
>
> subdir-ccflags-y := $(KBUILD_CFLAGS_W1_20200930)
>
> To indicate they want to W=1 warnings as defined on 20200930.
>
> Additional warnings can be added to the W=1 definition. This will not
> affect KBUILD_CFLAGS_W1_20200930 and hence no additional warnings will
> start appearing unless W=1 is actually added to the command
> line. Developers can then take their time to fix any new W=1 warnings,
> and then update to the latest KBUILD_CFLAGS_W1_<DATESTAMP> symbol.
I'm not a fan of this approach. Are DATESTAMP configs just going to
keep being added? Is it going to complicate isolating the issue from a
randconfig build? If we want more things to build warning-free at
W=1, then why don't we start moving warnings from W=1 into the
default, until this is no W=1 left? That way we're cutting down on
the kernel's configuration combinatorial explosion, rather than adding
to it?
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
> scripts/Makefile.extrawarn | 34 ++++++++++++++++++----------------
> 1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> index 95e4cdb94fe9..957dca35ae3e 100644
> --- a/scripts/Makefile.extrawarn
> +++ b/scripts/Makefile.extrawarn
> @@ -20,24 +20,26 @@ export KBUILD_EXTRA_WARN
> #
> # W=1 - warnings which may be relevant and do not occur too often
> #
> -ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),)
> -
> -KBUILD_CFLAGS += -Wextra -Wunused -Wno-unused-parameter
> -KBUILD_CFLAGS += -Wmissing-declarations
> -KBUILD_CFLAGS += -Wmissing-format-attribute
> -KBUILD_CFLAGS += -Wmissing-prototypes
> -KBUILD_CFLAGS += -Wold-style-definition
> -KBUILD_CFLAGS += -Wmissing-include-dirs
> -KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable)
> -KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable)
> -KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned)
> -KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation)
> +KBUILD_CFLAGS_W1_20200930 += -Wextra -Wunused -Wno-unused-parameter
> +KBUILD_CFLAGS_W1_20200930 += -Wmissing-declarations
> +KBUILD_CFLAGS_W1_20200930 += -Wmissing-format-attribute
> +KBUILD_CFLAGS_W1_20200930 += -Wmissing-prototypes
> +KBUILD_CFLAGS_W1_20200930 += -Wold-style-definition
> +KBUILD_CFLAGS_W1_20200930 += -Wmissing-include-dirs
> +KBUILD_CFLAGS_W1_20200930 += $(call cc-option, -Wunused-but-set-variable)
> +KBUILD_CFLAGS_W1_20200930 += $(call cc-option, -Wunused-const-variable)
> +KBUILD_CFLAGS_W1_20200930 += $(call cc-option, -Wpacked-not-aligned)
> +KBUILD_CFLAGS_W1_20200930 += $(call cc-option, -Wstringop-truncation)
> # The following turn off the warnings enabled by -Wextra
> -KBUILD_CFLAGS += -Wno-missing-field-initializers
> -KBUILD_CFLAGS += -Wno-sign-compare
> -KBUILD_CFLAGS += -Wno-type-limits
> +KBUILD_CFLAGS_W1_20200930 += -Wno-missing-field-initializers
> +KBUILD_CFLAGS_W1_20200930 += -Wno-sign-compare
> +KBUILD_CFLAGS_W1_20200930 += -Wno-type-limits
> +
> +export KBUILD_CFLAGS_W1_20200930
> +
> +ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),)
>
> -KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN1
> +KBUILD_CPPFLAGS += $(KBUILD_CFLAGS_W1_20200930) -DKBUILD_EXTRA_WARN1
>
> else
>
> --
> 2.28.0
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20201001011232.4050282-2-andrew%40lunn.ch.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today
2020-10-01 23:09 ` Nick Desaulniers
@ 2020-10-02 1:44 ` Andrew Lunn
2020-10-02 12:20 ` Arnd Bergmann
2020-10-05 17:31 ` Nick Desaulniers
0 siblings, 2 replies; 20+ messages in thread
From: Andrew Lunn @ 2020-10-02 1:44 UTC (permalink / raw)
To: Nick Desaulniers
Cc: netdev, David Miller, Jakub Kicinski, Masahiro Yamada,
Michal Marek, Rohit Maheshwari, Linux Kbuild mailing list,
clang-built-linux, Arnd Bergmann
On Thu, Oct 01, 2020 at 04:09:43PM -0700, Nick Desaulniers wrote:
> On Wed, Sep 30, 2020 at 6:12 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > There is a movement to try to make more and more of /drivers W=1
> > clean. But it will only stay clean if new warnings are quickly
> > detected and fixed, ideally by the developer adding the new code.
> >
> > To allow subdirectories to sign up to being W=1 clean for a given
> > definition of W=1, export the current set of additional compile flags
> > using the symbol KBUILD_CFLAGS_W1_20200930. Subdirectory Makefiles can
> > then use:
> >
> > subdir-ccflags-y := $(KBUILD_CFLAGS_W1_20200930)
> >
> > To indicate they want to W=1 warnings as defined on 20200930.
> >
> > Additional warnings can be added to the W=1 definition. This will not
> > affect KBUILD_CFLAGS_W1_20200930 and hence no additional warnings will
> > start appearing unless W=1 is actually added to the command
> > line. Developers can then take their time to fix any new W=1 warnings,
> > and then update to the latest KBUILD_CFLAGS_W1_<DATESTAMP> symbol.
>
> I'm not a fan of this approach. Are DATESTAMP configs just going to
> keep being added? Is it going to complicate isolating the issue from a
> randconfig build? If we want more things to build warning-free at
> W=1, then why don't we start moving warnings from W=1 into the
> default, until this is no W=1 left? That way we're cutting down on
> the kernel's configuration combinatorial explosion, rather than adding
> to it?
Hi Nick
I don't see randconfig being an issue. driver/net/ethernet would
always be build W=1, by some stable definition of W=1. randconfig
would not enable or disable additional warnings. It to make it clear,
KBUILD_CFLAGS_W1_20200930 is not a Kconfig option you can select. It
is a Makefile constant, a list of warnings which define what W=1 means
on that specific day. See patch 1/2.
I see a few issues with moving individual warnings from W=1 to the
default:
One of the comments for v1 of this patchset is that we cannot
introduce new warnings in the build. The complete tree needs to clean
of a particularly warning, before it can be added to the default list.
But that is not how people are cleaning up code, nor how the
infrastructure is designed. Those doing the cleanup are not take the
first from the list, -Wextra and cleanup up the whole tree for that
one warnings. They are rather enabling W=1 on a subdirectory, and
cleanup up all warnings on that subdirectory. So using this approach,
in order to move a warning from W=1 to the default, we are going to
have to get the entire tree W=1 clean, and move them all the warnings
are once.
People generally don't care about the tree as a whole. They care about
their own corner. The idea of fixing one warning thought the whole
tree is 'slicing and dicing' the kernel the wrong way. As we have seen
with the recent work with W=1, the more natural way to slice/dice the
kernel is by subdirectories.
I do however understand the fear that we end up with lots of
KBUILD_CFLAGS_W1_<DATESTAMP> constants. So i looked at the git history
of script/Makefile.extrawarn. These are historically the symbols we
would have, if we started this idea 1/1/2018:
KBUILD_CFLAGS_W1_20200326 # CLANG only change
KBUILD_CFLAGS_W1_20190907
KBUILD_CFLAGS_W1_20190617 # CLANG only change
KBUILD_CFLAGS_W1_20190614 # CLANG only change
KBUILD_CFLAGS_W1_20190509
KBUILD_CFLAGS_W1_20180919
KBUILD_CFLAGS_W1_20180111
So not too many.
Andrew
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today
2020-10-02 1:44 ` Andrew Lunn
@ 2020-10-02 12:20 ` Arnd Bergmann
2020-10-02 12:51 ` Andrew Lunn
2020-10-12 1:00 ` Masahiro Yamada
2020-10-05 17:31 ` Nick Desaulniers
1 sibling, 2 replies; 20+ messages in thread
From: Arnd Bergmann @ 2020-10-02 12:20 UTC (permalink / raw)
To: Andrew Lunn
Cc: Nick Desaulniers, netdev, David Miller, Jakub Kicinski,
Masahiro Yamada, Michal Marek, Rohit Maheshwari,
Linux Kbuild mailing list, clang-built-linux
On Fri, Oct 2, 2020 at 3:44 AM Andrew Lunn <andrew@lunn.ch> wrote:
> On Thu, Oct 01, 2020 at 04:09:43PM -0700, Nick Desaulniers wrote:
> > On Wed, Sep 30, 2020 at 6:12 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > There is a movement to try to make more and more of /drivers W=1
> > > clean. But it will only stay clean if new warnings are quickly
> > > detected and fixed, ideally by the developer adding the new code.
Nice, I think everyone agrees that this is a good goal.
> > > To allow subdirectories to sign up to being W=1 clean for a given
> > > definition of W=1, export the current set of additional compile flags
> > > using the symbol KBUILD_CFLAGS_W1_20200930. Subdirectory Makefiles can
> > > then use:
> > >
> > > subdir-ccflags-y := $(KBUILD_CFLAGS_W1_20200930)
> > >
> > > To indicate they want to W=1 warnings as defined on 20200930.
> > >
> > > Additional warnings can be added to the W=1 definition. This will not
> > > affect KBUILD_CFLAGS_W1_20200930 and hence no additional warnings will
> > > start appearing unless W=1 is actually added to the command
> > > line. Developers can then take their time to fix any new W=1 warnings,
> > > and then update to the latest KBUILD_CFLAGS_W1_<DATESTAMP> symbol.
> >
> > I'm not a fan of this approach. Are DATESTAMP configs just going to
> > keep being added? Is it going to complicate isolating the issue from a
> > randconfig build? If we want more things to build warning-free at
> > W=1, then why don't we start moving warnings from W=1 into the
> > default, until this is no W=1 left? That way we're cutting down on
> > the kernel's configuration combinatorial explosion, rather than adding
> > to it?
I'm also a little sceptical about the datestamp.
> Hi Nick
>
> I don't see randconfig being an issue. driver/net/ethernet would
> always be build W=1, by some stable definition of W=1. randconfig
> would not enable or disable additional warnings. It to make it clear,
> KBUILD_CFLAGS_W1_20200930 is not a Kconfig option you can select. It
> is a Makefile constant, a list of warnings which define what W=1 means
> on that specific day. See patch 1/2.
It won't change with the configuration, but it will change with the
specific compiler version. When you enable a warning in a
particular driver or directory, this may have different effects
on one compiler compared to another: clang and gcc sometimes
differ in their interpretation of which specific forms of an expression
to warn about or not, and any multiplexing options like -Wextra
or -Wformat may turn on additional warnings in later releases.
> I see a few issues with moving individual warnings from W=1 to the
> default:
>
> One of the comments for v1 of this patchset is that we cannot
> introduce new warnings in the build. The complete tree needs to clean
> of a particularly warning, before it can be added to the default list.
> But that is not how people are cleaning up code, nor how the
> infrastructure is designed. Those doing the cleanup are not take the
> first from the list, -Wextra and cleanup up the whole tree for that
> one warnings. They are rather enabling W=1 on a subdirectory, and
> cleanup up all warnings on that subdirectory. So using this approach,
> in order to move a warning from W=1 to the default, we are going to
> have to get the entire tree W=1 clean, and move them all the warnings
> are once.
I think the two approaches are orthogonal, and I would like to
see both happening as much as possible:
- any warning flag in the W=1 set (including many things implied
by -Wextra that have or should have their own flags) that
only causes a few lines of output should just be enabled by
default after we address the warnings
- Code with maintainers that care should have a way to enable
the entire W=1 set per directory or per file after addressing all
the warnings they do see, with new flags only getting added to
W=1 when they don't cause regressions.
There are more things that we might want to do on top of this:
- identify additional warning flags that we be good to add to W=1
- identify warning flags that are better off being turned into errors,
like we do with -Werror=strict-prototypes
- Fix the warnings in W=2 that show up in common header files,
to actually make it realistic to build specific drivers with W=2
and not have interesting issues drowned out in the noise.
- ensure that any warning flag we turn *off* in W=1 or by default
is turned on again in one of the higher levels
> People generally don't care about the tree as a whole. They care about
> their own corner. The idea of fixing one warning thought the whole
> tree is 'slicing and dicing' the kernel the wrong way. As we have seen
> with the recent work with W=1, the more natural way to slice/dice the
> kernel is by subdirectories.
I do care about the tree as a whole, and I'm particularly interested in
having -Wmissing-declarations/-Wmissing-prototypes enabled globally
at some point in the future.
Arnd
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today
2020-10-02 12:20 ` Arnd Bergmann
@ 2020-10-02 12:51 ` Andrew Lunn
2020-10-02 13:15 ` Arnd Bergmann
2020-10-12 1:00 ` Masahiro Yamada
1 sibling, 1 reply; 20+ messages in thread
From: Andrew Lunn @ 2020-10-02 12:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Nick Desaulniers, netdev, David Miller, Jakub Kicinski,
Masahiro Yamada, Michal Marek, Rohit Maheshwari,
Linux Kbuild mailing list, clang-built-linux
On Fri, Oct 02, 2020 at 02:20:50PM +0200, Arnd Bergmann wrote:
> On Fri, Oct 2, 2020 at 3:44 AM Andrew Lunn <andrew@lunn.ch> wrote:
> > On Thu, Oct 01, 2020 at 04:09:43PM -0700, Nick Desaulniers wrote:
> > > On Wed, Sep 30, 2020 at 6:12 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > > >
> > > > There is a movement to try to make more and more of /drivers W=1
> > > > clean. But it will only stay clean if new warnings are quickly
> > > > detected and fixed, ideally by the developer adding the new code.
>
> Nice, I think everyone agrees that this is a good goal.
>
> > > > To allow subdirectories to sign up to being W=1 clean for a given
> > > > definition of W=1, export the current set of additional compile flags
> > > > using the symbol KBUILD_CFLAGS_W1_20200930. Subdirectory Makefiles can
> > > > then use:
> > > >
> > > > subdir-ccflags-y := $(KBUILD_CFLAGS_W1_20200930)
> > > >
> > > > To indicate they want to W=1 warnings as defined on 20200930.
> > > >
> > > > Additional warnings can be added to the W=1 definition. This will not
> > > > affect KBUILD_CFLAGS_W1_20200930 and hence no additional warnings will
> > > > start appearing unless W=1 is actually added to the command
> > > > line. Developers can then take their time to fix any new W=1 warnings,
> > > > and then update to the latest KBUILD_CFLAGS_W1_<DATESTAMP> symbol.
> > >
> > > I'm not a fan of this approach. Are DATESTAMP configs just going to
> > > keep being added? Is it going to complicate isolating the issue from a
> > > randconfig build? If we want more things to build warning-free at
> > > W=1, then why don't we start moving warnings from W=1 into the
> > > default, until this is no W=1 left? That way we're cutting down on
> > > the kernel's configuration combinatorial explosion, rather than adding
> > > to it?
>
> I'm also a little sceptical about the datestamp.
Hi Arnd
Any suggestions for an alternative?
> It won't change with the configuration, but it will change with the
> specific compiler version. When you enable a warning in a
> particular driver or directory, this may have different effects
> on one compiler compared to another: clang and gcc sometimes
> differ in their interpretation of which specific forms of an expression
> to warn about or not, and any multiplexing options like -Wextra
> or -Wformat may turn on additional warnings in later releases.
How do we deal with this at the moment? Or will -Wextra and -Wformat
never get moved into the default?
> I think the two approaches are orthogonal, and I would like to
> see both happening as much as possible:
>
> - any warning flag in the W=1 set (including many things implied
> by -Wextra that have or should have their own flags) that
> only causes a few lines of output should just be enabled by
> default after we address the warnings
Is there currently any simple way to get statistics about how many
actual warnings there are per warnings flag in W=1? W=1 on the tree as
a whole does not look good, but maybe there is some low hanging fruit
and we can quickly promote some from W=1 to default?
> - Code with maintainers that care should have a way to enable
> the entire W=1 set per directory or per file after addressing all
> the warnings they do see, with new flags only getting added to
> W=1 when they don't cause regressions.
Yes, this is what i'm trying to push forward here. I don't
particularly care how it happen, so if somebody comes up with a
generally acceptable idea, i will go away and implement it.
> > People generally don't care about the tree as a whole. They care about
> > their own corner. The idea of fixing one warning thought the whole
> > tree is 'slicing and dicing' the kernel the wrong way. As we have seen
> > with the recent work with W=1, the more natural way to slice/dice the
> > kernel is by subdirectories.
>
> I do care about the tree as a whole, and I'm particularly interested in
> having -Wmissing-declarations/-Wmissing-prototypes enabled globally
> at some point in the future.
I know you care. But you are vastly out numbered by developers who
care about their own little corner. Which is why i said 'generally'.
We definitely should come at the problem from both directions, but i
guess we can make more progress with tools for the large number of
developers each in their own corner, than tools for the few who work
tree wide.
Andrew
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today
2020-10-02 12:51 ` Andrew Lunn
@ 2020-10-02 13:15 ` Arnd Bergmann
0 siblings, 0 replies; 20+ messages in thread
From: Arnd Bergmann @ 2020-10-02 13:15 UTC (permalink / raw)
To: Andrew Lunn
Cc: Nick Desaulniers, netdev, David Miller, Jakub Kicinski,
Masahiro Yamada, Michal Marek, Rohit Maheshwari,
Linux Kbuild mailing list, clang-built-linux
On Fri, Oct 2, 2020 at 2:51 PM Andrew Lunn <andrew@lunn.ch> wrote:
> On Fri, Oct 02, 2020 at 02:20:50PM +0200, Arnd Bergmann wrote:
> > On Fri, Oct 2, 2020 at 3:44 AM Andrew Lunn <andrew@lunn.ch> wrote:
> > > On Thu, Oct 01, 2020 at 04:09:43PM -0700, Nick Desaulniers wrote:
> > > >
> > > > I'm not a fan of this approach. Are DATESTAMP configs just going to
> > > > keep being added? Is it going to complicate isolating the issue from a
> > > > randconfig build? If we want more things to build warning-free at
> > > > W=1, then why don't we start moving warnings from W=1 into the
> > > > default, until this is no W=1 left? That way we're cutting down on
> > > > the kernel's configuration combinatorial explosion, rather than adding
> > > > to it?
> >
> > I'm also a little sceptical about the datestamp.
>
> Hi Arnd
>
> Any suggestions for an alternative?
I think we should deal with it the same way we deal with new
compiler versions: before a new compiler starts getting widely
used, someone has to address the new warnings that show up,
or at the minimum they have to get turned off by default until
they are addressed.
Today, moving a warning flag from W=1 to default requires that
there won't be any regressions in the output. The same should
apply to adding W=1 warnings if there is a way for drivers to
default-enable them.
> > It won't change with the configuration, but it will change with the
> > specific compiler version. When you enable a warning in a
> > particular driver or directory, this may have different effects
> > on one compiler compared to another: clang and gcc sometimes
> > differ in their interpretation of which specific forms of an expression
> > to warn about or not, and any multiplexing options like -Wextra
> > or -Wformat may turn on additional warnings in later releases.
>
> How do we deal with this at the moment? Or will -Wextra and -Wformat
> never get moved into the default?
I think for Wextra, that would likely stay with W=1, though individual
warnings in that set should be enabled by default whenever they
make sense. For -Wformat, we probably want the opposite and
enable the global option by default but make individual sub-options
W=1 or W=2 if there is too much undesired output.
> > I think the two approaches are orthogonal, and I would like to
> > see both happening as much as possible:
> >
> > - any warning flag in the W=1 set (including many things implied
> > by -Wextra that have or should have their own flags) that
> > only causes a few lines of output should just be enabled by
> > default after we address the warnings
>
> Is there currently any simple way to get statistics about how many
> actual warnings there are per warnings flag in W=1? W=1 on the tree as
> a whole does not look good, but maybe there is some low hanging fruit
> and we can quickly promote some from W=1 to default?
I have done this a few times in the past, essentially building a
few hundred randconfig kernels across multiple architectures
and then processing the output in a script. I usually treat a
file:line:warning tuple as a single instance and then count
how many there are.
> > - Code with maintainers that care should have a way to enable
> > the entire W=1 set per directory or per file after addressing all
> > the warnings they do see, with new flags only getting added to
> > W=1 when they don't cause regressions.
>
> Yes, this is what i'm trying to push forward here. I don't
> particularly care how it happen, so if somebody comes up with a
> generally acceptable idea, i will go away and implement it.
I actually have a set of patches that I started a while ago to
move the logic from scripts/Makefile.extrawarn into
include/linux/warnings.h, using '_Pragma("GCC diagnostic ...")'
with some infrastructure around it, to also allow drivers to
set the level as well as individual warnings when needed.
I never managed to get that patch series into a state for submission
so far, with a few things that need to be addressed first:
- any Makefile that changes warning options needs to be
converted to use macro syntax
- I need to check that the patches don't accidentally disable
warnings that are currently enabled (this is harder than
checking the reverse)
- some specific warnings have problems with this new method
for options that control multiple other options.
Arnd
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today
2020-10-02 12:20 ` Arnd Bergmann
2020-10-02 12:51 ` Andrew Lunn
@ 2020-10-12 1:00 ` Masahiro Yamada
2020-10-12 8:05 ` Arnd Bergmann
1 sibling, 1 reply; 20+ messages in thread
From: Masahiro Yamada @ 2020-10-12 1:00 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Andrew Lunn, Nick Desaulniers, netdev, David Miller,
Jakub Kicinski, Michal Marek, Rohit Maheshwari,
Linux Kbuild mailing list, clang-built-linux
On Fri, Oct 2, 2020 at 9:21 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> I do care about the tree as a whole, and I'm particularly interested in
> having -Wmissing-declarations/-Wmissing-prototypes enabled globally
> at some point in the future.
>
> Arnd
BTW, if possible, please educate me about the difference
between -Wmissing-declarations and -Wmissing-prototypes.
The GCC manual says as follows:
-Wmissing-prototypes (C and Objective-C only)
Warn if a global function is defined without a previous prototype
declaration. This warning is issued even if the definition itself
provides a prototype. Use this option to detect global functions that
do not have a matching prototype declaration in a header file. This
option is not valid for C++ because all function declarations provide
prototypes and a non-matching declaration declares an overload rather
than conflict with an earlier declaration. Use -Wmissing-declarations
to detect missing declarations in C++.
-Wmissing-declarations
Warn if a global function is defined without a previous declaration.
Do so even if the definition itself provides a prototype. Use this
option to detect global functions that are not declared in header
files. In C, no warnings are issued for functions with previous
non-prototype declarations; use -Wmissing-prototypes to detect missing
prototypes. In C++, no warnings are issued for function templates, or
for inline functions, or for functions in anonymous namespaces.
The difference is still unclear to me...
For example, if I add -Wmissing-declarations, I get the following:
kernel/sched/core.c:2380:6: warning: no previous declaration for
‘sched_set_stop_task’ [-Wmissing-declarations]
2380 | void sched_set_stop_task(int cpu, struct task_struct *stop)
| ^~~~~~~~~~~~~~~~~~~
But, if I add both -Wmissing-declarations and -Wmissing-prototypes,
-Wmissing-declarations is superseded by -Wmissing-prototypes.
kernel/sched/core.c:2380:6: warning: no previous prototype for
‘sched_set_stop_task’ [-Wmissing-prototypes]
2380 | void sched_set_stop_task(int cpu, struct task_struct *stop)
| ^~~~~~~~~~~~~~~~~~~
Do we need to specify both in W=1 ?
If yes, what is the difference between them?
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today
2020-10-12 1:00 ` Masahiro Yamada
@ 2020-10-12 8:05 ` Arnd Bergmann
0 siblings, 0 replies; 20+ messages in thread
From: Arnd Bergmann @ 2020-10-12 8:05 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Andrew Lunn, Nick Desaulniers, netdev, David Miller,
Jakub Kicinski, Michal Marek, Rohit Maheshwari,
Linux Kbuild mailing list, clang-built-linux
On Mon, Oct 12, 2020 at 3:00 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> BTW, if possible, please educate me about the difference
> between -Wmissing-declarations and -Wmissing-prototypes.
>
...
> Do we need to specify both in W=1 ?
> If yes, what is the difference between them?
I think they do the same thing in the kernel, as we already set
'-Wstrict-prototypes', which requires that there are no declarations
without having a prototype first. Adding either one should be sufficient.
Arnd
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today
2020-10-02 1:44 ` Andrew Lunn
2020-10-02 12:20 ` Arnd Bergmann
@ 2020-10-05 17:31 ` Nick Desaulniers
2020-10-05 19:49 ` Andrew Lunn
1 sibling, 1 reply; 20+ messages in thread
From: Nick Desaulniers @ 2020-10-05 17:31 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, David Miller, Jakub Kicinski, Masahiro Yamada,
Michal Marek, Rohit Maheshwari, Linux Kbuild mailing list,
clang-built-linux, Arnd Bergmann
On Thu, Oct 1, 2020 at 6:44 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Thu, Oct 01, 2020 at 04:09:43PM -0700, Nick Desaulniers wrote:
> > On Wed, Sep 30, 2020 at 6:12 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > There is a movement to try to make more and more of /drivers W=1
> > > clean. But it will only stay clean if new warnings are quickly
> > > detected and fixed, ideally by the developer adding the new code.
> > >
> > > To allow subdirectories to sign up to being W=1 clean for a given
> > > definition of W=1, export the current set of additional compile flags
> > > using the symbol KBUILD_CFLAGS_W1_20200930. Subdirectory Makefiles can
> > > then use:
> > >
> > > subdir-ccflags-y := $(KBUILD_CFLAGS_W1_20200930)
> > >
> > > To indicate they want to W=1 warnings as defined on 20200930.
> > >
> > > Additional warnings can be added to the W=1 definition. This will not
> > > affect KBUILD_CFLAGS_W1_20200930 and hence no additional warnings will
> > > start appearing unless W=1 is actually added to the command
> > > line. Developers can then take their time to fix any new W=1 warnings,
> > > and then update to the latest KBUILD_CFLAGS_W1_<DATESTAMP> symbol.
> >
> > I'm not a fan of this approach. Are DATESTAMP configs just going to
> > keep being added? Is it going to complicate isolating the issue from a
> > randconfig build? If we want more things to build warning-free at
> > W=1, then why don't we start moving warnings from W=1 into the
> > default, until this is no W=1 left? That way we're cutting down on
> > the kernel's configuration combinatorial explosion, rather than adding
> > to it?
>
> Hi Nick
>
> I don't see randconfig being an issue. driver/net/ethernet would
> always be build W=1, by some stable definition of W=1. randconfig
> would not enable or disable additional warnings. It to make it clear,
> KBUILD_CFLAGS_W1_20200930 is not a Kconfig option you can select. It
> is a Makefile constant, a list of warnings which define what W=1 means
> on that specific day. See patch 1/2.
>
> I see a few issues with moving individual warnings from W=1 to the
> default:
>
> One of the comments for v1 of this patchset is that we cannot
> introduce new warnings in the build. The complete tree needs to clean
> of a particularly warning, before it can be added to the default list.
> But that is not how people are cleaning up code, nor how the
> infrastructure is designed. Those doing the cleanup are not take the
> first from the list, -Wextra and cleanup up the whole tree for that
> one warnings. They are rather enabling W=1 on a subdirectory, and
> cleanup up all warnings on that subdirectory. So using this approach,
> in order to move a warning from W=1 to the default, we are going to
> have to get the entire tree W=1 clean, and move them all the warnings
> are once.
Sorry, to be more specific about my concern; I like the idea of
exporting the W=* flags, then selectively applying them via
subdir-ccflags-y. I don't like the idea of supporting W=1 as defined
at a precise point in time via multiple date specific symbols. If
someone adds something to W=1, then they should need to ensure subdirs
build warning-free, so I don't think you need to "snapshot" W=1 based
on what it looked like on 20200930.
>
> People generally don't care about the tree as a whole. They care about
> their own corner. The idea of fixing one warning thought the whole
> tree is 'slicing and dicing' the kernel the wrong way. As we have seen
> with the recent work with W=1, the more natural way to slice/dice the
> kernel is by subdirectories.
I'm not sure I agree with this paragraph. ^ If a warning is not
enabled by default implicitly, then someone would need to clean the
tree to turn it on. It's very messy to apply it on a child directory,
then try to work up. We've done multiple tree wide warning cleanups
and it's not too bad.
>
> I do however understand the fear that we end up with lots of
> KBUILD_CFLAGS_W1_<DATESTAMP> constants. So i looked at the git history
> of script/Makefile.extrawarn. These are historically the symbols we
> would have, if we started this idea 1/1/2018:
>
> KBUILD_CFLAGS_W1_20200326 # CLANG only change
> KBUILD_CFLAGS_W1_20190907
> KBUILD_CFLAGS_W1_20190617 # CLANG only change
> KBUILD_CFLAGS_W1_20190614 # CLANG only change
> KBUILD_CFLAGS_W1_20190509
> KBUILD_CFLAGS_W1_20180919
> KBUILD_CFLAGS_W1_20180111
>
> So not too many.
It's a useful visualization. I still would prefer W=1 to get enabled
by default if all of the CI systems have flushed out the existing
warnings. That way we have one less combination of things to test;
not more.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today
2020-10-05 17:31 ` Nick Desaulniers
@ 2020-10-05 19:49 ` Andrew Lunn
2020-10-05 20:03 ` Arnd Bergmann
0 siblings, 1 reply; 20+ messages in thread
From: Andrew Lunn @ 2020-10-05 19:49 UTC (permalink / raw)
To: Nick Desaulniers
Cc: netdev, David Miller, Jakub Kicinski, Masahiro Yamada,
Michal Marek, Rohit Maheshwari, Linux Kbuild mailing list,
clang-built-linux, Arnd Bergmann
> Sorry, to be more specific about my concern; I like the idea of
> exporting the W=* flags, then selectively applying them via
> subdir-ccflags-y. I don't like the idea of supporting W=1 as defined
> at a precise point in time via multiple date specific symbols. If
> someone adds something to W=1, then they should need to ensure subdirs
> build warning-free, so I don't think you need to "snapshot" W=1 based
> on what it looked like on 20200930.
Hi Nick
That then contradicts what Masahiro Yamada said to the first version i
posted:
https://www.spinics.net/lists/netdev/msg685284.html
> With this patch series applied, where should we add -Wfoo-bar?
> Adding it to W=1 would emit warnings under drivers/net/ since W=1 is
> now the default for the net subsystem.
The idea with the date stamps was to allow new warnings to be added to
W=1 without them immediately causing warnings on normal builds. You
are saying that whoever adds a new warning to W=1 needs to cleanup the
tree which is already W=1 clean? That might have the side effect that
no more warnings are added to W=1 :-(
Andrew
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today
2020-10-05 19:49 ` Andrew Lunn
@ 2020-10-05 20:03 ` Arnd Bergmann
2020-10-05 21:08 ` Andrew Lunn
0 siblings, 1 reply; 20+ messages in thread
From: Arnd Bergmann @ 2020-10-05 20:03 UTC (permalink / raw)
To: Andrew Lunn
Cc: Nick Desaulniers, netdev, David Miller, Jakub Kicinski,
Masahiro Yamada, Michal Marek, Rohit Maheshwari,
Linux Kbuild mailing list, clang-built-linux
On Mon, Oct 5, 2020 at 9:49 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > Sorry, to be more specific about my concern; I like the idea of
> > exporting the W=* flags, then selectively applying them via
> > subdir-ccflags-y. I don't like the idea of supporting W=1 as defined
> > at a precise point in time via multiple date specific symbols. If
> > someone adds something to W=1, then they should need to ensure subdirs
> > build warning-free, so I don't think you need to "snapshot" W=1 based
> > on what it looked like on 20200930.
>
> That then contradicts what Masahiro Yamada said to the first version i
> posted:
>
> https://www.spinics.net/lists/netdev/msg685284.html
> > With this patch series applied, where should we add -Wfoo-bar?
> > Adding it to W=1 would emit warnings under drivers/net/ since W=1 is
> > now the default for the net subsystem.
>
> The idea with the date stamps was to allow new warnings to be added to
> W=1 without them immediately causing warnings on normal builds. You
> are saying that whoever adds a new warning to W=1 needs to cleanup the
> tree which is already W=1 clean? That might have the side effect that
> no more warnings are added to W=1 :-(
It depends a lot on what portion of the kernel gets enabled for W=1.
As long as it's only drivers that are actively maintained, and they
make up a fairly small portion of all code, it should not be a problem
to find someone to fix useful warnings.
The only reason to add a flag to W=1 would be that the bugs it reports
are important enough to look at the false positives and address
those as well. Whoever decided to enable W=1 by default for their
subsystem should then also be interested in adding the new warnings.
If I wanted to add a new flag to W=1 and this introduces output
for allmodconfig, I would start by mailing that output to the
respective maintainers.
Arnd
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today
2020-10-05 20:03 ` Arnd Bergmann
@ 2020-10-05 21:08 ` Andrew Lunn
2020-10-11 13:03 ` Masahiro Yamada
0 siblings, 1 reply; 20+ messages in thread
From: Andrew Lunn @ 2020-10-05 21:08 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Nick Desaulniers, netdev, David Miller, Jakub Kicinski,
Masahiro Yamada, Michal Marek, Rohit Maheshwari,
Linux Kbuild mailing list, clang-built-linux
> It depends a lot on what portion of the kernel gets enabled for W=1.
>
> As long as it's only drivers that are actively maintained, and they
> make up a fairly small portion of all code, it should not be a problem
> to find someone to fix useful warnings.
Well, drivers/net/ethernet is around 1.5M LOC. The tree as a whole is
just short of 23M LOC. So i guess that is a small portion of all the
code.
Andrew
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today
2020-10-05 21:08 ` Andrew Lunn
@ 2020-10-11 13:03 ` Masahiro Yamada
2020-10-12 8:11 ` Arnd Bergmann
2020-10-16 14:12 ` Andrew Lunn
0 siblings, 2 replies; 20+ messages in thread
From: Masahiro Yamada @ 2020-10-11 13:03 UTC (permalink / raw)
To: Andrew Lunn
Cc: Arnd Bergmann, Nick Desaulniers, netdev, David Miller,
Jakub Kicinski, Michal Marek, Rohit Maheshwari,
Linux Kbuild mailing list, clang-built-linux
On Tue, Oct 6, 2020 at 6:08 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > It depends a lot on what portion of the kernel gets enabled for W=1.
> >
> > As long as it's only drivers that are actively maintained, and they
> > make up a fairly small portion of all code, it should not be a problem
> > to find someone to fix useful warnings.
>
> Well, drivers/net/ethernet is around 1.5M LOC. The tree as a whole is
> just short of 23M LOC. So i guess that is a small portion of all the
> code.
>
> Andrew
I am not a big fan of KBUILD_CFLAGS_W1_<timestamp>
since it is ugly.
I'd like to start with adding individual flags
like drivers/gpu/drm/i915/Makefile, and see
how difficult it would be to maintain it.
One drawback of your approach is that
you cannot set KBUILD_CFLAGS_W1_20200930
until you eliminate all the warnings in the
sub-directory in interest.
(i.e. all or nothing approach)
At best, you can only work out from 'old -> new' order
because KBUILD_CFLAGS_W1_20200326 is a suer-set of
KBUILD_CFLAGS_W1_20190907, which is a suer-set of
KBUILD_CFLAGS_W1_20190617 ...
If you add flags individually, you can start with
low-hanging fruits, or ones with higher priority
as Arnd mentions about -Wmissing-{declaration,prototypes}.
For example, you might be able to set
'subdir-ccflags-y += -Wmissing-declarations'
to drivers/net/Makefile, while
'subdir-ccflags-y += -Wunused-but-set-variable'
stays in drivers/net/ethernet/Makefile.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today
2020-10-11 13:03 ` Masahiro Yamada
@ 2020-10-12 8:11 ` Arnd Bergmann
2020-10-16 14:12 ` Andrew Lunn
1 sibling, 0 replies; 20+ messages in thread
From: Arnd Bergmann @ 2020-10-12 8:11 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Andrew Lunn, Nick Desaulniers, netdev, David Miller,
Jakub Kicinski, Michal Marek, Rohit Maheshwari,
Linux Kbuild mailing list, clang-built-linux
On Sun, Oct 11, 2020 at 3:04 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> On Tue, Oct 6, 2020 at 6:08 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
>
> I am not a big fan of KBUILD_CFLAGS_W1_<timestamp>
> since it is ugly.
>
> I'd like to start with adding individual flags
> like drivers/gpu/drm/i915/Makefile, and see
> how difficult it would be to maintain it.
I don't really like that either, to be honest, mostly because that is
somewhat incompatible with my plan to move all the warning flags
out the command line and into _Pragma() lines in header files.
> One drawback of your approach is that
> you cannot set KBUILD_CFLAGS_W1_20200930
> until you eliminate all the warnings in the
> sub-directory in interest.
> (i.e. all or nothing approach)
>
> At best, you can only work out from 'old -> new' order
> because KBUILD_CFLAGS_W1_20200326 is a suer-set of
> KBUILD_CFLAGS_W1_20190907, which is a suer-set of
> KBUILD_CFLAGS_W1_20190617 ...
>
>
>
> If you add flags individually, you can start with
> low-hanging fruits, or ones with higher priority
> as Arnd mentions about -Wmissing-{declaration,prototypes}.
>
>
> For example, you might be able to set
> 'subdir-ccflags-y += -Wmissing-declarations'
> to drivers/net/Makefile, while
> 'subdir-ccflags-y += -Wunused-but-set-variable'
> stays in drivers/net/ethernet/Makefile.
I agree with the goal though. In particular it may be helpful to pick
a set of warning flags to always be enabled without also setting
-Wextra, which has different meanings depending on compiler
version, or between gcc and clang.
I wonder how different they really are, and we can probably find
out from https://github.com/Barro/compiler-warnings, but I have
not checked myself.
Arnd
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today
2020-10-11 13:03 ` Masahiro Yamada
2020-10-12 8:11 ` Arnd Bergmann
@ 2020-10-16 14:12 ` Andrew Lunn
[not found] ` <CAK8P3a1nBhmf1PQwHHbEjiVgRTXi4UuJAbwuK92CKEbR=yKGWw@mail.gmail.com>
1 sibling, 1 reply; 20+ messages in thread
From: Andrew Lunn @ 2020-10-16 14:12 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Arnd Bergmann, Nick Desaulniers, netdev, David Miller,
Jakub Kicinski, Michal Marek, Rohit Maheshwari,
Linux Kbuild mailing list, clang-built-linux
> One drawback of your approach is that
> you cannot set KBUILD_CFLAGS_W1_20200930
> until you eliminate all the warnings in the
> sub-directory in interest.
> (i.e. all or nothing approach)
Hi Mashiro
That actual works well for my use case. drivers/net/ethernet is W=1
clean. So is drivers/net/phy, drivers/net/mdio. Developers generally
clean up a subsystem by adding W=1 to the command line because that is
the simple tool they have.
And my aim here is to keep those subsystem W=1 clean. I don't care
about individual warnings within W=1, because those subsystems are
passed that stage already.
Andrew
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today
2020-10-01 1:12 ` [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today Andrew Lunn
2020-10-01 23:09 ` Nick Desaulniers
@ 2020-10-02 9:04 ` kernel test robot
2020-10-02 11:08 ` kernel test robot
2 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2020-10-02 9:04 UTC (permalink / raw)
To: Andrew Lunn, netdev
Cc: kbuild-all, Jakub Kicinski, Masahiro Yamada, Michal Marek,
Rohit Maheshwari, linux-kbuild, clang-built-linux, Andrew Lunn
[-- Attachment #1: Type: text/plain, Size: 10452 bytes --]
Hi Andrew,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Andrew-Lunn/driver-net-ethernet-W-1-by-default/20201001-091431
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git e13dbc4f41db7f7b86f17a2efd7fbe19fc5b6d7a
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/b50d78df08d105cf0f0f2a1f4d2225656fd531cc
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andrew-Lunn/driver-net-ethernet-W-1-by-default/20201001-091431
git checkout b50d78df08d105cf0f0f2a1f4d2225656fd531cc
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/firmware/efi/libstub/x86-stub.c:669:15: warning: no previous prototype for 'efi_main' [-Wmissing-prototypes]
669 | unsigned long efi_main(efi_handle_t handle,
| ^~~~~~~~
vim +/efi_main +669 drivers/firmware/efi/libstub/x86-stub.c
291f36325f9f252 arch/x86/boot/compressed/eboot.c Matt Fleming 2011-12-12 663
9ca8f72a9297f20 arch/x86/boot/compressed/eboot.c Matt Fleming 2012-07-19 664 /*
8acf63efa1712fa drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 665 * On success, we return the address of startup_32, which has potentially been
8acf63efa1712fa drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 666 * relocated by efi_relocate_kernel.
8acf63efa1712fa drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 667 * On failure, we exit to the firmware via efi_exit instead of returning.
9ca8f72a9297f20 arch/x86/boot/compressed/eboot.c Matt Fleming 2012-07-19 668 */
8acf63efa1712fa drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 @669 unsigned long efi_main(efi_handle_t handle,
c3710de5065d63f arch/x86/boot/compressed/eboot.c Ard Biesheuvel 2019-12-24 670 efi_system_table_t *sys_table_arg,
796eb8d26a57f91 arch/x86/boot/compressed/eboot.c Ard Biesheuvel 2020-01-13 671 struct boot_params *boot_params)
9ca8f72a9297f20 arch/x86/boot/compressed/eboot.c Matt Fleming 2012-07-19 672 {
04a7d0e15606769 arch/x86/boot/compressed/eboot.c Ard Biesheuvel 2020-02-10 673 unsigned long bzimage_addr = (unsigned long)startup_32;
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 674 unsigned long buffer_start, buffer_end;
9ca8f72a9297f20 arch/x86/boot/compressed/eboot.c Matt Fleming 2012-07-19 675 struct setup_header *hdr = &boot_params->hdr;
9ca8f72a9297f20 arch/x86/boot/compressed/eboot.c Matt Fleming 2012-07-19 676 efi_status_t status;
54b52d872680348 arch/x86/boot/compressed/eboot.c Matt Fleming 2014-01-10 677
ccc27ae77494252 drivers/firmware/efi/libstub/x86-stub.c Ard Biesheuvel 2020-04-16 678 efi_system_table = sys_table_arg;
9ca8f72a9297f20 arch/x86/boot/compressed/eboot.c Matt Fleming 2012-07-19 679
9ca8f72a9297f20 arch/x86/boot/compressed/eboot.c Matt Fleming 2012-07-19 680 /* Check if we were booted by the EFI firmware */
ccc27ae77494252 drivers/firmware/efi/libstub/x86-stub.c Ard Biesheuvel 2020-04-16 681 if (efi_system_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE)
3b8f44fc0810d51 drivers/firmware/efi/libstub/x86-stub.c Ard Biesheuvel 2020-02-16 682 efi_exit(handle, EFI_INVALID_PARAMETER);
9ca8f72a9297f20 arch/x86/boot/compressed/eboot.c Matt Fleming 2012-07-19 683
04a7d0e15606769 arch/x86/boot/compressed/eboot.c Ard Biesheuvel 2020-02-10 684 /*
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 685 * If the kernel isn't already loaded at a suitable address,
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 686 * relocate it.
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 687 *
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 688 * It must be loaded above LOAD_PHYSICAL_ADDR.
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 689 *
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 690 * The maximum address for 64-bit is 1 << 46 for 4-level paging. This
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 691 * is defined as the macro MAXMEM, but unfortunately that is not a
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 692 * compile-time constant if 5-level paging is configured, so we instead
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 693 * define our own macro for use here.
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 694 *
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 695 * For 32-bit, the maximum address is complicated to figure out, for
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 696 * now use KERNEL_IMAGE_SIZE, which will be 512MiB, the same as what
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 697 * KASLR uses.
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 698 *
21cb9b414301c76 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-04-09 699 * Also relocate it if image_offset is zero, i.e. the kernel wasn't
21cb9b414301c76 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-04-09 700 * loaded by LoadImage, but rather by a bootloader that called the
21cb9b414301c76 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-04-09 701 * handover entry. The reason we must always relocate in this case is
21cb9b414301c76 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-04-09 702 * to handle the case of systemd-boot booting a unified kernel image,
21cb9b414301c76 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-04-09 703 * which is a PE executable that contains the bzImage and an initrd as
21cb9b414301c76 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-04-09 704 * COFF sections. The initrd section is placed after the bzImage
21cb9b414301c76 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-04-09 705 * without ensuring that there are at least init_size bytes available
21cb9b414301c76 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-04-09 706 * for the bzImage, and thus the compressed kernel's startup code may
21cb9b414301c76 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-04-09 707 * overwrite the initrd unless it is moved out of the way.
04a7d0e15606769 arch/x86/boot/compressed/eboot.c Ard Biesheuvel 2020-02-10 708 */
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 709
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 710 buffer_start = ALIGN(bzimage_addr - image_offset,
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 711 hdr->kernel_alignment);
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 712 buffer_end = buffer_start + hdr->init_size;
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 713
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 714 if ((buffer_start < LOAD_PHYSICAL_ADDR) ||
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 715 (IS_ENABLED(CONFIG_X86_32) && buffer_end > KERNEL_IMAGE_SIZE) ||
d5cdf4cfeac9146 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 716 (IS_ENABLED(CONFIG_X86_64) && buffer_end > MAXMEM_X86_64_4LEVEL) ||
21cb9b414301c76 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-04-09 717 (image_offset == 0)) {
04a7d0e15606769 arch/x86/boot/compressed/eboot.c Ard Biesheuvel 2020-02-10 718 status = efi_relocate_kernel(&bzimage_addr,
04a7d0e15606769 arch/x86/boot/compressed/eboot.c Ard Biesheuvel 2020-02-10 719 hdr->init_size, hdr->init_size,
04a7d0e15606769 arch/x86/boot/compressed/eboot.c Ard Biesheuvel 2020-02-10 720 hdr->pref_address,
04a7d0e15606769 arch/x86/boot/compressed/eboot.c Ard Biesheuvel 2020-02-10 721 hdr->kernel_alignment,
04a7d0e15606769 arch/x86/boot/compressed/eboot.c Ard Biesheuvel 2020-02-10 722 LOAD_PHYSICAL_ADDR);
04a7d0e15606769 arch/x86/boot/compressed/eboot.c Ard Biesheuvel 2020-02-10 723 if (status != EFI_SUCCESS) {
36bdd0a78d56831 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-04-30 724 efi_err("efi_relocate_kernel() failed!\n");
04a7d0e15606769 arch/x86/boot/compressed/eboot.c Ard Biesheuvel 2020-02-10 725 goto fail;
04a7d0e15606769 arch/x86/boot/compressed/eboot.c Ard Biesheuvel 2020-02-10 726 }
1887c9b653f9957 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 727 /*
1887c9b653f9957 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 728 * Now that we've copied the kernel elsewhere, we no longer
1887c9b653f9957 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 729 * have a set up block before startup_32(), so reset image_offset
1887c9b653f9957 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 730 * to zero in case it was set earlier.
1887c9b653f9957 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 731 */
1887c9b653f9957 drivers/firmware/efi/libstub/x86-stub.c Arvind Sankar 2020-03-08 732 image_offset = 0;
04a7d0e15606769 arch/x86/boot/compressed/eboot.c Ard Biesheuvel 2020-02-10 733 }
04a7d0e15606769 arch/x86/boot/compressed/eboot.c Ard Biesheuvel 2020-02-10 734
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 74808 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today
2020-10-01 1:12 ` [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today Andrew Lunn
2020-10-01 23:09 ` Nick Desaulniers
2020-10-02 9:04 ` kernel test robot
@ 2020-10-02 11:08 ` kernel test robot
2 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2020-10-02 11:08 UTC (permalink / raw)
To: Andrew Lunn, netdev
Cc: kbuild-all, Jakub Kicinski, Masahiro Yamada, Michal Marek,
Rohit Maheshwari, linux-kbuild, clang-built-linux, Andrew Lunn
[-- Attachment #1: Type: text/plain, Size: 2343 bytes --]
Hi Andrew,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Andrew-Lunn/driver-net-ethernet-W-1-by-default/20201001-091431
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git e13dbc4f41db7f7b86f17a2efd7fbe19fc5b6d7a
config: parisc-randconfig-r016-20200930 (attached as .config)
compiler: hppa64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/b50d78df08d105cf0f0f2a1f4d2225656fd531cc
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andrew-Lunn/driver-net-ethernet-W-1-by-default/20201001-091431
git checkout b50d78df08d105cf0f0f2a1f4d2225656fd531cc
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
arch/parisc/boot/compressed/firmware.c: In function 'set_firmware_width_unlocked':
>> arch/parisc/boot/compressed/firmware.c:159:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
159 | int ret;
| ^~~
arch/parisc/boot/compressed/firmware.c: In function 'set_firmware_width':
arch/parisc/boot/compressed/firmware.c:176:16: warning: variable 'flags' set but not used [-Wunused-but-set-variable]
176 | unsigned long flags;
| ^~~~~
arch/parisc/boot/compressed/firmware.c: In function 'pdc_iodc_print':
arch/parisc/boot/compressed/firmware.c:1234:16: warning: variable 'flags' set but not used [-Wunused-but-set-variable]
1234 | unsigned long flags;
| ^~~~~
At top level:
arch/parisc/boot/compressed/firmware.c:121:22: warning: 'f_extend' defined but not used [-Wunused-function]
121 | static unsigned long f_extend(unsigned long address)
| ^~~~~~~~
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27348 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread