Patchwork powerpc: Add configurable -Werror for arch/powerpc

login
register
mail settings
Submitter Michael Ellerman
Date June 10, 2009, 6:48 a.m.
Message ID <adf502e7c85161d77fdcaca24f55ba6b418c7f04.1244616523.git.michael@ellerman.id.au>
Download mbox | patch
Permalink /patch/28385/
State Accepted, archived
Commit ba55bd74360ea4b8b95e73ed79474d37ff482b36
Delegated to: Benjamin Herrenschmidt
Headers show

Comments

Michael Ellerman - June 10, 2009, 6:48 a.m.
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(-)
Michael Ellerman - June 15, 2009, 7:50 a.m.
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
Benjamin Herrenschmidt - June 15, 2009, 9:40 a.m.
> 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.
Michael Ellerman - June 15, 2009, 12:36 p.m.
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
Timur Tabi - June 15, 2009, 2:54 p.m.
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?
Benjamin Herrenschmidt - June 15, 2009, 9:23 p.m.
> 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.
Arnd Bergmann - June 18, 2009, 2:22 p.m.
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

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