[PATCH] powerpc: Add configurable -Werror for arch/powerpc

[PATCH] powerpc: Add configurable -Werror for arch/powerpc

[Michael Ellerman]

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.

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
-- 
1.6.2.1

Re: [PATCH] powerpc: Add configurable -Werror for arch/powerpc

[Olof Johansson]

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

Re: [PATCH] powerpc: Add configurable -Werror for arch/powerpc

[David Miller]

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.

Re: [PATCH] powerpc: Add configurable -Werror for arch/powerpc

[Olof Johansson]

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

Re: [PATCH] powerpc: Add configurable -Werror for arch/powerpc

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1631 bytes --]

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

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] powerpc: Add configurable -Werror for arch/powerpc

[Olof Johansson]

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

Re: [PATCH] powerpc: Add configurable -Werror for arch/powerpc

[Geert Uytterhoeven]

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

Re: [PATCH] powerpc: Add configurable -Werror for arch/powerpc

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 689 bytes --]

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).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] powerpc: Add configurable -Werror for arch/powerpc

[Kumar Gala]

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] powerpc: Add configurable -Werror for arch/powerpc

[Michael Ellerman]

Add the option to build the code under arch/powerpc with -Werror.

The intention is to make it harder for people to inadvertantly introduce
warnings 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.

The option is a negative, ie. don't enable -Werror, so that it will be
turned on for allyes and allmodconfig builds.

The default is n, in the hope that developers will build with -Werror,
that will probably lead to some build breaks, I am prepared to be flamed.

It's not enabled for math-emu, which is a steaming pile of warnings.

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

diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index d79a902..3b10051 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -2,6 +2,23 @@ menu "Kernel hacking"
 
 source "lib/Kconfig.debug"
 
+config PPC_DISABLE_WERROR
+	bool "Don't build arch/powerpc code with -Werror"
+	default n
+	help
+	  This option tells the compiler NOT to build the code under
+	  arch/powerpc with the -Werror flag (which means warnings
+	  are treated as errors).
+
+	  Only enable this if you are hitting a build failure in the
+	  arch/powerpc code caused by a warning, and you don't feel
+	  inclined to fix it.
+
+config PPC_WERROR
+	bool
+	depends on !PPC_DISABLE_WERROR
+	default y
+
 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 65cf365..110ec6d 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -4,6 +4,8 @@
 
 CFLAGS_ptrace.o		+= -DUTS_MACHINE='"$(UTS_MACHINE)"'
 
+subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
+
 ifeq ($(CONFIG_PPC64),y)
 CFLAGS_prom_init.o	+= -mno-minimal-toc
 endif
diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
index 4b2df66..459c7ee 100644
--- a/arch/powerpc/kvm/Makefile
+++ b/arch/powerpc/kvm/Makefile
@@ -2,6 +2,8 @@
 # Makefile for Kernel-based Virtual Machine module
 #
 
+subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
+
 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 29b742b..3040dac 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -2,6 +2,8 @@
 # Makefile for ppc-specific library files..
 #
 
+subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
+
 ifeq ($(CONFIG_PPC64),y)
 EXTRA_CFLAGS		+= -mno-minimal-toc
 endif
diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
index c4bcf07..2d2192e 100644
--- a/arch/powerpc/mm/Makefile
+++ b/arch/powerpc/mm/Makefile
@@ -2,6 +2,8 @@
 # Makefile for the linux ppc-specific parts of the memory manager.
 #
 
+subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
+
 ifeq ($(CONFIG_PPC64),y)
 EXTRA_CFLAGS	+= -mno-minimal-toc
 endif
diff --git a/arch/powerpc/oprofile/Makefile b/arch/powerpc/oprofile/Makefile
index 2ef6b0d..73e1c2c 100644
--- a/arch/powerpc/oprofile/Makefile
+++ b/arch/powerpc/oprofile/Makefile
@@ -1,3 +1,5 @@
+subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
+
 ifeq ($(CONFIG_PPC64),y)
 EXTRA_CFLAGS	+= -mno-minimal-toc
 endif
diff --git a/arch/powerpc/platforms/Makefile b/arch/powerpc/platforms/Makefile
index f741919..a6812ee 100644
--- a/arch/powerpc/platforms/Makefile
+++ b/arch/powerpc/platforms/Makefile
@@ -1,4 +1,6 @@
 
+subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
+
 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 2d1c87d..d073bfd 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -1,3 +1,5 @@
+subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
+
 ifeq ($(CONFIG_PPC64),y)
 EXTRA_CFLAGS			+= -mno-minimal-toc
 endif
diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
index 9cb03b7..85ab97a 100644
--- a/arch/powerpc/xmon/Makefile
+++ b/arch/powerpc/xmon/Makefile
@@ -1,5 +1,7 @@
 # Makefile for xmon
 
+subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
+
 ifdef CONFIG_PPC64
 EXTRA_CFLAGS += -mno-minimal-toc
 endif
-- 
1.6.2.1

Re: [PATCH] powerpc: Add configurable -Werror for arch/powerpc

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1071 bytes --]

On Wed, 2009-06-10 at 16:48 +1000, Michael Ellerman wrote:
> Add the option to build the code under arch/powerpc with -Werror.
> 
> The intention is to make it harder for people to inadvertantly introduce
> warnings 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.
> 
> The option is a negative, ie. don't enable -Werror, so that it will be
> turned on for allyes and allmodconfig builds.
> 
> The default is n, in the hope that developers will build with -Werror,
> that will probably lead to some build breaks, I am prepared to be flamed.

Currently this appears to break only one of the defconfigs, chrp32.

http://kisskb.ellerman.id.au/kisskb/head/1907/

And that's a legitimate error AFAICT:

arch/powerpc/platforms/chrp/setup.c:378: error: the frame size of 1040 bytes is larger than 1024 bytes

From:

367 void
368 chrp_event_scan(unsigned long unused)
369 {
370         unsigned char log[1024];
371         int ret = 0;


cheers

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] powerpc: Add configurable -Werror for arch/powerpc

[Benjamin Herrenschmidt]

> Currently this appears to break only one of the defconfigs, chrp32.
> 
> http://kisskb.ellerman.id.au/kisskb/head/1907/
> 
> And that's a legitimate error AFAICT:
> 
> arch/powerpc/platforms/chrp/setup.c:378: error: the frame size of 1040 bytes is larger than 1024 bytes
> 
> From:
> 
> 367 void
> 368 chrp_event_scan(unsigned long unused)
> 369 {
> 370         unsigned char log[1024];
> 371         int ret = 0;

I wonder to what extent we could merge that with the pSeries
event-scan...

We can have a closer look tomorrow. In any case, stack alloc for that is
indeed fishy.

Cheers,
Ben.

Re: [PATCH] powerpc: Add configurable -Werror for arch/powerpc

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]

On Mon, 2009-06-15 at 19:40 +1000, Benjamin Herrenschmidt wrote:
> > Currently this appears to break only one of the defconfigs, chrp32.
> > 
> > http://kisskb.ellerman.id.au/kisskb/head/1907/
> > 
> > And that's a legitimate error AFAICT:
> > 
> > arch/powerpc/platforms/chrp/setup.c:378: error: the frame size of 1040 bytes is larger than 1024 bytes
> > 
> > From:
> > 
> > 367 void
> > 368 chrp_event_scan(unsigned long unused)
> > 369 {
> > 370         unsigned char log[1024];
> > 371         int ret = 0;
> 
> I wonder to what extent we could merge that with the pSeries
> event-scan...

Yeah that occured to me, someone with a chrp machine would need to test
it, do we have one?

> We can have a closer look tomorrow. In any case, stack alloc for that is
> indeed fishy.

The obvious patch to make it static doesn't fly because it's called on
every cpu via a timer, so it needs to be a per-cpu at least I think. Or
borrow the pseries trick of calling it on each cpu in succession, or
just borrow the pseries code.

cheers

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] powerpc: Add configurable -Werror for arch/powerpc

[Benjamin Herrenschmidt]

> Yeah that occured to me, someone with a chrp machine would need to test
> it, do we have one?

Yup, I think we do. I'll check that when I'm in the office.

> > We can have a closer look tomorrow. In any case, stack alloc for that is
> > indeed fishy.
> 
> The obvious patch to make it static doesn't fly because it's called on
> every cpu via a timer, so it needs to be a per-cpu at least I think. Or
> borrow the pseries trick of calling it on each cpu in succession, or
> just borrow the pseries code.

Right.

Cheers,
Ben.

Re: [PATCH] powerpc: Add configurable -Werror for arch/powerpc

[Timur Tabi]

On Mon, Jun 15, 2009 at 2:50 AM, Michael Ellerman<michael@ellerman.id.au> wrote:
> arch/powerpc/platforms/chrp/setup.c:378: error: the frame size of 1040 bytes is larger than 1024 bytes

What's so bad about a frame size larger than 1024?


-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH] powerpc: Add configurable -Werror for arch/powerpc

[Arnd Bergmann]

On Monday 15 June 2009, Timur Tabi wrote:
> On Mon, Jun 15, 2009 at 2:50 AM, Michael Ellerman<michael@ellerman.id.au> wrote:
> > arch/powerpc/platforms/chrp/setup.c:378: error: the frame size of 1040 bytes is larger than 1024 bytes
> 
> What's so bad about a frame size larger than 1024?
> 

It's not necessarily a bug, but all frame sizes in the call chain
combined must never exceed the kernel stack size. 1024 is an
arbitrary limit that we warn about for a single function, because
it's likely that things will break if you have more than one of these.

	Arnd <><

[PATCH] powerpc: Add configurable -Werror for arch/powerpc

[Michael Ellerman]

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/math-emu/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 ++++
 10 files changed, 44 insertions(+), 0 deletions(-)

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/math-emu/Makefile b/arch/powerpc/math-emu/Makefile
index 0c16ab9..dcb2148 100644
--- a/arch/powerpc/math-emu/Makefile
+++ b/arch/powerpc/math-emu/Makefile
@@ -1,4 +1,8 @@
 
+ifeq ($(CONFIG_PPC_WERROR),y)
+EXTRA_CFLAGS += -Werror
+endif
+
 obj-$(CONFIG_MATH_EMULATION)	+= fabs.o fadd.o fadds.o fcmpo.o fcmpu.o \
 					fctiw.o fctiwz.o fdiv.o fdivs.o \
 					fmadd.o fmadds.o fmsub.o fmsubs.o \
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
-- 
1.6.1.2

Re: [PATCH] powerpc: Add configurable -Werror for arch/powerpc

[Kumar Gala]

On Apr 5, 2009, at 11:18 PM, Michael Ellerman wrote:

> diff --git a/arch/powerpc/math-emu/Makefile b/arch/powerpc/math-emu/ 
> Makefile
> index 0c16ab9..dcb2148 100644
> --- a/arch/powerpc/math-emu/Makefile
> +++ b/arch/powerpc/math-emu/Makefile
> @@ -1,4 +1,8 @@
>
> +ifeq ($(CONFIG_PPC_WERROR),y)
> +EXTRA_CFLAGS += -Werror
> +endif
> +
> obj-$(CONFIG_MATH_EMULATION)	+= fabs.o fadd.o fadds.o fcmpo.o  
> fcmpu.o \
> 					fctiw.o fctiwz.o fdiv.o fdivs.o \
> 					fmadd.o fmadds.o fmsub.o fmsubs.o \

We explicitly build math-emu with -w to not see the warnings from it.

- k