Patchwork powerpc: Add configurable -Werror for arch/powerpc

login
register
mail settings
Submitter Michael Ellerman
Date April 7, 2009, 2 a.m.
Message ID <b0b55030b7ba669cbdec74a8b89875096ea07622.1239069602.git.michael@ellerman.id.au>
Download mbox | patch
Permalink /patch/25657/
State Changes Requested
Headers show

Comments

Michael Ellerman - April 7, 2009, 2 a.m.
Add an option, on by default, to build all code under arch/powerpc with
-Werror, which causes gcc to treat warnings as errors.

The intention is to make it harder for people to inadvertantly introduce
errors in the arch/powerpc code. It needs to be configurable so that
if a warning is introduced, people can easily work around it while it's
being fixed.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/Kconfig.debug      |    8 ++++++++
 arch/powerpc/kernel/Makefile    |    4 ++++
 arch/powerpc/kvm/Makefile       |    4 ++++
 arch/powerpc/lib/Makefile       |    4 ++++
 arch/powerpc/mm/Makefile        |    4 ++++
 arch/powerpc/oprofile/Makefile  |    4 ++++
 arch/powerpc/platforms/Makefile |    4 ++++
 arch/powerpc/sysdev/Makefile    |    4 ++++
 arch/powerpc/xmon/Makefile      |    4 ++++
 9 files changed, 40 insertions(+), 0 deletions(-)

v2: Leave math-emu alone, it's a steaming pile of warnings.
Olof Johansson - April 7, 2009, 2:27 a.m.
On Tue, Apr 07, 2009 at 12:00:41PM +1000, Michael Ellerman wrote:
> Add an option, on by default, to build all code under arch/powerpc with
> -Werror, which causes gcc to treat warnings as errors.
> 
> The intention is to make it harder for people to inadvertantly introduce
> errors in the arch/powerpc code. It needs to be configurable so that
> if a warning is introduced, people can easily work around it while it's
> being fixed.

This looks useful at least for the automated builds to catch new warnings,
but do similar options exist on other architectures, x86 in particular? I
think a Cc to LKML of this could be useful.

This is really only beneficial if various people build for powerpc often
enough. If major subsystem maintainers aren't going to hit the errors
it's more of a hinderance for PPC than a global benefit, right?


-Olof
David Miller - April 7, 2009, 2:28 a.m.
From: Olof Johansson <olof@lixom.net>
Date: Mon, 6 Apr 2009 21:27:43 -0500

> On Tue, Apr 07, 2009 at 12:00:41PM +1000, Michael Ellerman wrote:
>> Add an option, on by default, to build all code under arch/powerpc with
>> -Werror, which causes gcc to treat warnings as errors.
>> 
>> The intention is to make it harder for people to inadvertantly introduce
>> errors in the arch/powerpc code. It needs to be configurable so that
>> if a warning is introduced, people can easily work around it while it's
>> being fixed.
> 
> This looks useful at least for the automated builds to catch new warnings,
> but do similar options exist on other architectures, x86 in particular? I
> think a Cc to LKML of this could be useful.

On sparc64 we do this unconditionally.
Olof Johansson - April 7, 2009, 2:37 a.m.
On Mon, Apr 06, 2009 at 07:28:30PM -0700, David Miller wrote:
> From: Olof Johansson <olof@lixom.net>
> Date: Mon, 6 Apr 2009 21:27:43 -0500
> 
> > On Tue, Apr 07, 2009 at 12:00:41PM +1000, Michael Ellerman wrote:
> >> Add an option, on by default, to build all code under arch/powerpc with
> >> -Werror, which causes gcc to treat warnings as errors.
> >> 
> >> The intention is to make it harder for people to inadvertantly introduce
> >> errors in the arch/powerpc code. It needs to be configurable so that
> >> if a warning is introduced, people can easily work around it while it's
> >> being fixed.
> > 
> > This looks useful at least for the automated builds to catch new warnings,
> > but do similar options exist on other architectures, x86 in particular? I
> > think a Cc to LKML of this could be useful.
> 
> On sparc64 we do this unconditionally.

Great! I'm all for merging it in, having recently found new warnings on
RFC patches that went uncaught by the poster. :)


-Olof
Michael Ellerman - April 7, 2009, 3:01 a.m.
On Mon, 2009-04-06 at 21:27 -0500, Olof Johansson wrote:
> On Tue, Apr 07, 2009 at 12:00:41PM +1000, Michael Ellerman wrote:
> > Add an option, on by default, to build all code under arch/powerpc with
> > -Werror, which causes gcc to treat warnings as errors.
> > 
> > The intention is to make it harder for people to inadvertantly introduce
> > errors in the arch/powerpc code. It needs to be configurable so that
> > if a warning is introduced, people can easily work around it while it's
> > being fixed.
> 
> This looks useful at least for the automated builds to catch new warnings,
> but do similar options exist on other architectures, x86 in particular? I
> think a Cc to LKML of this could be useful.
> 
> This is really only beneficial if various people build for powerpc often
> enough. If major subsystem maintainers aren't going to hit the errors
> it's more of a hinderance for PPC than a global benefit, right?

I don't think so. We're only enabling it for code under arch/powerpc -
and most modifications to that code should come through linuxppc.

It's still possible that random stuff will get merged, or that someone
will change code in a header that only causes warnings on powerpc, but
it's less likely.

And that's why it's an option, if someone breaks the build we can work
around it until they're appropriately LARTed.

cheers
Geert Uytterhoeven - April 7, 2009, 7:41 a.m.
On Mon, 6 Apr 2009, Olof Johansson wrote:
> On Tue, Apr 07, 2009 at 12:00:41PM +1000, Michael Ellerman wrote:
> > Add an option, on by default, to build all code under arch/powerpc with
> > -Werror, which causes gcc to treat warnings as errors.
> > 
> > The intention is to make it harder for people to inadvertantly introduce
> > errors in the arch/powerpc code. It needs to be configurable so that
> > if a warning is introduced, people can easily work around it while it's
> > being fixed.
> 
> This looks useful at least for the automated builds to catch new warnings,
> but do similar options exist on other architectures, x86 in particular? I
> think a Cc to LKML of this could be useful.
> 
> This is really only beneficial if various people build for powerpc often
> enough. If major subsystem maintainers aren't going to hit the errors
> it's more of a hinderance for PPC than a global benefit, right?

Will be built nightly: http://kisskb.ellerman.id.au/kisskb/matrix/

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010
Stephen Rothwell - April 7, 2009, 7:50 a.m.
On Tue, 7 Apr 2009 09:41:38 +0200 (CEST) Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
>
> On Mon, 6 Apr 2009, Olof Johansson wrote:
> > 
> > This is really only beneficial if various people build for powerpc often
> > enough. If major subsystem maintainers aren't going to hit the errors
> > it's more of a hinderance for PPC than a global benefit, right?
> 
> Will be built nightly: http://kisskb.ellerman.id.au/kisskb/matrix/

And I build it several (i.e. between 20 and 80) times a day :-( (after
merging it with all sorts of stuff this is upcoming).
Olof Johansson - April 7, 2009, 2:11 p.m.
On Tue, Apr 07, 2009 at 03:01:33AM +0000, Michael Ellerman wrote:
> On Mon, 2009-04-06 at 21:27 -0500, Olof Johansson wrote:
> > On Tue, Apr 07, 2009 at 12:00:41PM +1000, Michael Ellerman wrote:
> > > Add an option, on by default, to build all code under arch/powerpc with
> > > -Werror, which causes gcc to treat warnings as errors.
> > > 
> > > The intention is to make it harder for people to inadvertantly introduce
> > > errors in the arch/powerpc code. It needs to be configurable so that
> > > if a warning is introduced, people can easily work around it while it's
> > > being fixed.
> > 
> > This looks useful at least for the automated builds to catch new warnings,
> > but do similar options exist on other architectures, x86 in particular? I
> > think a Cc to LKML of this could be useful.
> > 
> > This is really only beneficial if various people build for powerpc often
> > enough. If major subsystem maintainers aren't going to hit the errors
> > it's more of a hinderance for PPC than a global benefit, right?
> 
> I don't think so. We're only enabling it for code under arch/powerpc -
> and most modifications to that code should come through linuxppc.

Good point.

> It's still possible that random stuff will get merged, or that someone
> will change code in a header that only causes warnings on powerpc, but
> it's less likely.
> 
> And that's why it's an option, if someone breaks the build we can work
> around it until they're appropriately LARTed.

Yeah, sounds good to me.


-Olof
Kumar Gala - April 7, 2009, 3:05 p.m.
On Apr 6, 2009, at 9:00 PM, Michael Ellerman wrote:

> Add an option, on by default, to build all code under arch/powerpc  
> with
> -Werror, which causes gcc to treat warnings as errors.
>
> The intention is to make it harder for people to inadvertantly  
> introduce
> errors in the arch/powerpc code. It needs to be configurable so that
> if a warning is introduced, people can easily work around it while  
> it's
> being fixed.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
> arch/powerpc/Kconfig.debug      |    8 ++++++++
> arch/powerpc/kernel/Makefile    |    4 ++++
> arch/powerpc/kvm/Makefile       |    4 ++++
> arch/powerpc/lib/Makefile       |    4 ++++
> arch/powerpc/mm/Makefile        |    4 ++++
> arch/powerpc/oprofile/Makefile  |    4 ++++
> arch/powerpc/platforms/Makefile |    4 ++++
> arch/powerpc/sysdev/Makefile    |    4 ++++
> arch/powerpc/xmon/Makefile      |    4 ++++
> 9 files changed, 40 insertions(+), 0 deletions(-)
>
> v2: Leave math-emu alone, it's a steaming pile of warnings.

Acked-by: Kumar Gala <galak@kernel.crashing.org>

- k

Patch

diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index a1098e2..58d5c22 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -2,6 +2,14 @@  menu "Kernel hacking"
 
 source "lib/Kconfig.debug"
 
+config PPC_WERROR
+	bool "Build arch/powerpc code with -Werror"
+	default y
+	help
+	  This option tells the compiler to build all of the code under
+	  arch/powerpc with the -Werror flag, which means all warnings
+	  are treated as errors.
+
 config PRINT_STACK_DEPTH
 	int "Stack depth to print" if DEBUG_KERNEL
 	default 64
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 71901fb..b11ef4e 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -12,6 +12,10 @@  CFLAGS_prom_init.o      += -fPIC
 CFLAGS_btext.o		+= -fPIC
 endif
 
+ifeq ($(CONFIG_PPC_WERROR),y)
+EXTRA_CFLAGS += -Werror
+endif
+
 ifdef CONFIG_FUNCTION_TRACER
 # Do not trace early boot code
 CFLAGS_REMOVE_cputable.o = -pg -mno-sched-epilog
diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
index 4b2df66..51d2337 100644
--- a/arch/powerpc/kvm/Makefile
+++ b/arch/powerpc/kvm/Makefile
@@ -2,6 +2,10 @@ 
 # Makefile for Kernel-based Virtual Machine module
 #
 
+ifeq ($(CONFIG_PPC_WERROR),y)
+EXTRA_CFLAGS += -Werror
+endif
+
 EXTRA_CFLAGS += -Ivirt/kvm -Iarch/powerpc/kvm
 
 common-objs-y = $(addprefix ../../../virt/kvm/, kvm_main.o coalesced_mmio.o)
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 8db3527..536db60 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -2,6 +2,10 @@ 
 # Makefile for ppc-specific library files..
 #
 
+ifeq ($(CONFIG_PPC_WERROR),y)
+EXTRA_CFLAGS += -Werror
+endif
+
 ifeq ($(CONFIG_PPC64),y)
 EXTRA_CFLAGS		+= -mno-minimal-toc
 endif
diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
index 17290bc..4880a16 100644
--- a/arch/powerpc/mm/Makefile
+++ b/arch/powerpc/mm/Makefile
@@ -2,6 +2,10 @@ 
 # Makefile for the linux ppc-specific parts of the memory manager.
 #
 
+ifeq ($(CONFIG_PPC_WERROR),y)
+EXTRA_CFLAGS += -Werror
+endif
+
 ifeq ($(CONFIG_PPC64),y)
 EXTRA_CFLAGS	+= -mno-minimal-toc
 endif
diff --git a/arch/powerpc/oprofile/Makefile b/arch/powerpc/oprofile/Makefile
index 2ef6b0d..e607c74 100644
--- a/arch/powerpc/oprofile/Makefile
+++ b/arch/powerpc/oprofile/Makefile
@@ -1,3 +1,7 @@ 
+ifeq ($(CONFIG_PPC_WERROR),y)
+EXTRA_CFLAGS += -Werror
+endif
+
 ifeq ($(CONFIG_PPC64),y)
 EXTRA_CFLAGS	+= -mno-minimal-toc
 endif
diff --git a/arch/powerpc/platforms/Makefile b/arch/powerpc/platforms/Makefile
index f741919..b18b921 100644
--- a/arch/powerpc/platforms/Makefile
+++ b/arch/powerpc/platforms/Makefile
@@ -1,4 +1,8 @@ 
 
+ifeq ($(CONFIG_PPC_WERROR),y)
+EXTRA_CFLAGS += -Werror
+endif
+
 obj-$(CONFIG_FSL_ULI1575)	+= fsl_uli1575.o
 
 obj-$(CONFIG_PPC_PMAC)		+= powermac/
diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index b33b28a..508c305 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -1,3 +1,7 @@ 
+ifeq ($(CONFIG_PPC_WERROR),y)
+EXTRA_CFLAGS += -Werror
+endif
+
 ifeq ($(CONFIG_PPC64),y)
 EXTRA_CFLAGS			+= -mno-minimal-toc
 endif
diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
index 9cb03b7..3348822 100644
--- a/arch/powerpc/xmon/Makefile
+++ b/arch/powerpc/xmon/Makefile
@@ -1,5 +1,9 @@ 
 # Makefile for xmon
 
+ifeq ($(CONFIG_PPC_WERROR),y)
+EXTRA_CFLAGS += -Werror
+endif
+
 ifdef CONFIG_PPC64
 EXTRA_CFLAGS += -mno-minimal-toc
 endif