[PATCH] make gcc -O1 in fs/reiserfs optional

[PATCH] make gcc -O1 in fs/reiserfs optional

[Olaf Hering]

Jeff,

you added this EXTRA_CFLAGS= during 2.4 development, I think the broken
compiler was gcc 3.2 on SLES8. Can we turn this -O1 into a .config
option?


Signed-off-by: Olaf Hering <olh@suse.de>

Index: linux-2.6.12-rc3-olh/fs/reiserfs/Makefile
===================================================================
--- linux-2.6.12-rc3-olh.orig/fs/reiserfs/Makefile
+++ linux-2.6.12-rc3-olh/fs/reiserfs/Makefile
@@ -21,13 +21,7 @@ ifeq ($(CONFIG_REISERFS_FS_POSIX_ACL),y)
 reiserfs-objs += xattr_acl.o
 endif
 
-# gcc -O2 (the kernel default)  is overaggressive on ppc32 when many inline
-# functions are used.  This causes the compiler to advance the stack
-# pointer out of the available stack space, corrupting kernel space,
-# and causing a panic. Since this behavior only affects ppc32, this ifeq
-# will work around it. If any other architecture displays this behavior,
-# add it here.
-ifeq ($(CONFIG_PPC32),y)
+ifeq ($(CONFIG_REISERFS_CC_REDUCE_OPTIMZE),y)
 EXTRA_CFLAGS := -O1
 endif
 
Index: linux-2.6.12-rc3-olh/fs/Kconfig
===================================================================
--- linux-2.6.12-rc3-olh.orig/fs/Kconfig
+++ linux-2.6.12-rc3-olh/fs/Kconfig
@@ -186,6 +186,18 @@ config REISERFS_FS
 	  If you like it, you can pay us to add new features to it that you
 	  need, buy a support contract, or pay us to port it to another OS.
 
+config REISERFS_CC_REDUCE_OPTIMZE
+	bool "Reduce CC optimization level to workaround compiler bugs"
+	depends on PPC32
+	default n
+	help
+	  gcc -O2 (the kernel default) is overaggressive on ppc32 when many inline
+	  functions are used.  This causes the compiler to advance the stack
+	  pointer out of the available stack space, corrupting kernel space,
+	  and causing a panic. Since this behavior only affects ppc32, this ifeq
+	  will work around it. If any other architecture displays this behavior,
+	  add it here.
+
 config REISERFS_CHECK
 	bool "Enable reiserfs debug mode"
 	depends on REISERFS_FS

Re: [PATCH] make gcc -O1 in fs/reiserfs optional

[Jeff Mahoney]

Olaf Hering wrote:
> Jeff,
> 
> you added this EXTRA_CFLAGS= during 2.4 development, I think the broken
> compiler was gcc 3.2 on SLES8. Can we turn this -O1 into a .config
> option?

Hi Olaf -

Yeah, the initial problem was that the anti aliasing[*] code when -O2
was enabled was being way too cautious and ended up allocating something
like 6k on the stack in do_balance(). It was observable elsewhere, but
not so problematic.

My ppc box isn't booting right now. If you've verified that newer
versions of the compiler don't blow the stack in do_balance(), I'm not
opposed to it.

-Jeff

[*]: I think that's the right term - I'm not a compiler developer :)

-- 
Jeff Mahoney
SuSE Labs

Re: [PATCH] make gcc -O1 in fs/reiserfs optional

[Hans Reiser]

Olaf Hering wrote:

>Jeff,
>
>you added this EXTRA_CFLAGS= during 2.4 development, I think the broken
>compiler was gcc 3.2 on SLES8. Can we turn this -O1 into a .config
>option?
>
>
>Signed-off-by: Olaf Hering <olh@suse.de>
>
>Index: linux-2.6.12-rc3-olh/fs/reiserfs/Makefile
>===================================================================
>--- linux-2.6.12-rc3-olh.orig/fs/reiserfs/Makefile
>+++ linux-2.6.12-rc3-olh/fs/reiserfs/Makefile
>@@ -21,13 +21,7 @@ ifeq ($(CONFIG_REISERFS_FS_POSIX_ACL),y)
> reiserfs-objs += xattr_acl.o
> endif
> 
>-# gcc -O2 (the kernel default)  is overaggressive on ppc32 when many inline
>-# functions are used.  This causes the compiler to advance the stack
>-# pointer out of the available stack space, corrupting kernel space,
>-# and causing a panic. Since this behavior only affects ppc32, this ifeq
>-# will work around it. If any other architecture displays this behavior,
>-# add it here.
>-ifeq ($(CONFIG_PPC32),y)
>+ifeq ($(CONFIG_REISERFS_CC_REDUCE_OPTIMZE),y)
> EXTRA_CFLAGS := -O1
> endif
> 
>Index: linux-2.6.12-rc3-olh/fs/Kconfig
>===================================================================
>--- linux-2.6.12-rc3-olh.orig/fs/Kconfig
>+++ linux-2.6.12-rc3-olh/fs/Kconfig
>@@ -186,6 +186,18 @@ config REISERFS_FS
> 	  If you like it, you can pay us to add new features to it that you
> 	  need, buy a support contract, or pay us to port it to another OS.
> 
>+config REISERFS_CC_REDUCE_OPTIMZE
>+	bool "Reduce CC optimization level to workaround compiler bugs"
>+	depends on PPC32
>+	default n
>+	help
>+	  gcc -O2 (the kernel default) is overaggressive on ppc32 when many inline
>+	  functions are used.  This causes the compiler to advance the stack
>+	  pointer out of the available stack space, corrupting kernel space,
>+	  and causing a panic. Since this behavior only affects ppc32, this ifeq
>+	  will work around it. If any other architecture displays this behavior,
>+	  add it here.
>+
> config REISERFS_CHECK
> 	bool "Enable reiserfs debug mode"
> 	depends on REISERFS_FS
>
>
>  
>
Sounds reasonable.

[PATCH] make gcc -O1 in fs/reiserfs optional

[Olaf Hering]

 On Wed, Apr 27, Hans Reiser wrote:

> Olaf Hering wrote:
> 
> >Jeff,
> >
> >you added this EXTRA_CFLAGS= during 2.4 development, I think the broken
> >compiler was gcc 3.2 on SLES8. Can we turn this -O1 into a .config
> >option?

> Sounds reasonable.

 only compile with -O1 if the (very old) compiler is broken
 We use reiserfs alot in SLES9 on ppc64, and it was never seen
 with gcc33.

Signed-off-by: Olaf Hering <olh@suse.de>

Index: linux-2.6.12-rc3-olh/fs/reiserfs/Makefile
===================================================================
--- linux-2.6.12-rc3-olh.orig/fs/reiserfs/Makefile
+++ linux-2.6.12-rc3-olh/fs/reiserfs/Makefile
@@ -21,13 +21,7 @@ ifeq ($(CONFIG_REISERFS_FS_POSIX_ACL),y)
 reiserfs-objs += xattr_acl.o
 endif
 
-# gcc -O2 (the kernel default)  is overaggressive on ppc32 when many inline
-# functions are used.  This causes the compiler to advance the stack
-# pointer out of the available stack space, corrupting kernel space,
-# and causing a panic. Since this behavior only affects ppc32, this ifeq
-# will work around it. If any other architecture displays this behavior,
-# add it here.
-ifeq ($(CONFIG_PPC32),y)
+ifeq ($(CONFIG_REISERFS_CC_REDUCE_OPTIMZE),y)
 EXTRA_CFLAGS := -O1
 endif
 
Index: linux-2.6.12-rc3-olh/fs/Kconfig
===================================================================
--- linux-2.6.12-rc3-olh.orig/fs/Kconfig
+++ linux-2.6.12-rc3-olh/fs/Kconfig
@@ -186,6 +186,18 @@ config REISERFS_FS
 	  If you like it, you can pay us to add new features to it that you
 	  need, buy a support contract, or pay us to port it to another OS.
 
+config REISERFS_CC_REDUCE_OPTIMZE
+	bool "Reduce CC optimization level to workaround compiler bugs"
+	depends on PPC32
+	default n
+	help
+	  gcc -O2 (the kernel default) is overaggressive on ppc32 when many inline
+	  functions are used.  This causes the compiler to advance the stack
+	  pointer out of the available stack space, corrupting kernel space,
+	  and causing a panic. Since this behavior only affects ppc32, this ifeq
+	  will work around it. If any other architecture displays this behavior,
+	  add it here.
+
 config REISERFS_CHECK
 	bool "Enable reiserfs debug mode"
 	depends on REISERFS_FS
-- 
short story of a lazy sysadmin:
 alias appserv=wotan

Re: [PATCH] make gcc -O1 in fs/reiserfs optional

[Hans Reiser]

Sounds good to me.  Thanks Olaf!

Hans

Olaf Hering wrote:

> On Wed, Apr 27, Hans Reiser wrote:
>
>  
>
>>Olaf Hering wrote:
>>
>>    
>>
>>>Jeff,
>>>
>>>you added this EXTRA_CFLAGS= during 2.4 development, I think the broken
>>>compiler was gcc 3.2 on SLES8. Can we turn this -O1 into a .config
>>>option?
>>>      
>>>
>
>  
>
>>Sounds reasonable.
>>    
>>
>
> only compile with -O1 if the (very old) compiler is broken
> We use reiserfs alot in SLES9 on ppc64, and it was never seen
> with gcc33.
>
>Signed-off-by: Olaf Hering <olh@suse.de>
>
>Index: linux-2.6.12-rc3-olh/fs/reiserfs/Makefile
>===================================================================
>--- linux-2.6.12-rc3-olh.orig/fs/reiserfs/Makefile
>+++ linux-2.6.12-rc3-olh/fs/reiserfs/Makefile
>@@ -21,13 +21,7 @@ ifeq ($(CONFIG_REISERFS_FS_POSIX_ACL),y)
> reiserfs-objs += xattr_acl.o
> endif
> 
>-# gcc -O2 (the kernel default)  is overaggressive on ppc32 when many inline
>-# functions are used.  This causes the compiler to advance the stack
>-# pointer out of the available stack space, corrupting kernel space,
>-# and causing a panic. Since this behavior only affects ppc32, this ifeq
>-# will work around it. If any other architecture displays this behavior,
>-# add it here.
>-ifeq ($(CONFIG_PPC32),y)
>+ifeq ($(CONFIG_REISERFS_CC_REDUCE_OPTIMZE),y)
> EXTRA_CFLAGS := -O1
> endif
> 
>Index: linux-2.6.12-rc3-olh/fs/Kconfig
>===================================================================
>--- linux-2.6.12-rc3-olh.orig/fs/Kconfig
>+++ linux-2.6.12-rc3-olh/fs/Kconfig
>@@ -186,6 +186,18 @@ config REISERFS_FS
> 	  If you like it, you can pay us to add new features to it that you
> 	  need, buy a support contract, or pay us to port it to another OS.
> 
>+config REISERFS_CC_REDUCE_OPTIMZE
>+	bool "Reduce CC optimization level to workaround compiler bugs"
>+	depends on PPC32
>+	default n
>+	help
>+	  gcc -O2 (the kernel default) is overaggressive on ppc32 when many inline
>+	  functions are used.  This causes the compiler to advance the stack
>+	  pointer out of the available stack space, corrupting kernel space,
>+	  and causing a panic. Since this behavior only affects ppc32, this ifeq
>+	  will work around it. If any other architecture displays this behavior,
>+	  add it here.
>+
> config REISERFS_CHECK
> 	bool "Enable reiserfs debug mode"
> 	depends on REISERFS_FS
>  
>

Re: [PATCH] make gcc -O1 in fs/reiserfs optional

[Andrew Morton]

Olaf Hering <olh@suse.de> wrote:
>
>  On Wed, Apr 27, Hans Reiser wrote:
> 
> > Olaf Hering wrote:
> > 
> > >Jeff,
> > >
> > >you added this EXTRA_CFLAGS= during 2.4 development, I think the broken
> > >compiler was gcc 3.2 on SLES8. Can we turn this -O1 into a .config
> > >option?
> 
> > Sounds reasonable.
> 
>  only compile with -O1 if the (very old) compiler is broken
>  We use reiserfs alot in SLES9 on ppc64, and it was never seen
>  with gcc33.
> 

I dunno about this.

> 
> Index: linux-2.6.12-rc3-olh/fs/reiserfs/Makefile
> ===================================================================
> --- linux-2.6.12-rc3-olh.orig/fs/reiserfs/Makefile
> +++ linux-2.6.12-rc3-olh/fs/reiserfs/Makefile
> @@ -21,13 +21,7 @@ ifeq ($(CONFIG_REISERFS_FS_POSIX_ACL),y)
>  reiserfs-objs += xattr_acl.o
>  endif
>  
> -# gcc -O2 (the kernel default)  is overaggressive on ppc32 when many inline
> -# functions are used.  This causes the compiler to advance the stack
> -# pointer out of the available stack space, corrupting kernel space,
> -# and causing a panic. Since this behavior only affects ppc32, this ifeq
> -# will work around it. If any other architecture displays this behavior,
> -# add it here.
> -ifeq ($(CONFIG_PPC32),y)
> +ifeq ($(CONFIG_REISERFS_CC_REDUCE_OPTIMZE),y)
>  EXTRA_CFLAGS := -O1
>  endif
>  
> Index: linux-2.6.12-rc3-olh/fs/Kconfig
> ===================================================================
> --- linux-2.6.12-rc3-olh.orig/fs/Kconfig
> +++ linux-2.6.12-rc3-olh/fs/Kconfig
> @@ -186,6 +186,18 @@ config REISERFS_FS
>  	  If you like it, you can pay us to add new features to it that you
>  	  need, buy a support contract, or pay us to port it to another OS.
>  
> +config REISERFS_CC_REDUCE_OPTIMZE
> +	bool "Reduce CC optimization level to workaround compiler bugs"
> +	depends on PPC32
> +	default n
> +	help
> +	  gcc -O2 (the kernel default) is overaggressive on ppc32 when many inline
> +	  functions are used.  This causes the compiler to advance the stack
> +	  pointer out of the available stack space, corrupting kernel space,
> +	  and causing a panic. Since this behavior only affects ppc32, this ifeq
> +	  will work around it. If any other architecture displays this behavior,
> +	  add it here.
> +

Are you sure it's due to inline functions?  I thought the problem was that
certain versions of gcc did automatic inlining of non-inlined functions and
we get excessive stack windup due to that.  And iirc we put in global
compiler options to defeat that behaviour.  Andi would recall.

Furthermore, we do have infrastructure for detecting the gcc version at
build time.  It would be better to use that for disabling `-O2', rather
than a config option.

Re: [PATCH] make gcc -O1 in fs/reiserfs optional

[Jeff Mahoney]

Andrew Morton wrote:
> Are you sure it's due to inline functions?  I thought the problem was that
> certain versions of gcc did automatic inlining of non-inlined functions and
> we get excessive stack windup due to that.  And iirc we put in global
> compiler options to defeat that behaviour.  Andi would recall.
> 
> Furthermore, we do have infrastructure for detecting the gcc version at
> build time.  It would be better to use that for disabling `-O2', rather
> than a config option.

Andrew -

This "fix" has been in the kernel since I developed the endian safe
patches for ReiserFS in the 2.4 days; This patch just makes it optional
rather than required.

You could be correct regarding the behavior. I noticed it was related to
inline functions and stack usage. Since this was quite a few years ago
now, I don't recall the exact details, just that -O1 worked around it. I
was content blaming gcc and moving on with development. I seem to recall
thinking that was overaggressive aliasing avoidance between temporary
variables in static inline functions. The endian safe code frequently
uses static inlines for conversion of disk order bitfields (like key
type and offset). balance_leaf() is a beast of a function at around 1400
lines or so. When compiled with -O2, it wanted to allocate 6k of stack
space. With -O1, it ended up allocating about 230 bytes. It seemed to be
the only function really affected by the bug, but I wanted to be certain.

I do agree that using the kbuild infrastructure to determine the need
for this option would work quite a bit better.

-Jeff

-- 
Jeff Mahoney
SUSE Labs

[PATCH] use gcc -O1 in fs/reiserfs only for ancient gcc versions

[Olaf Hering]

only compile with -O1 if the (very old) compiler is broken.
We use reiserfs alot since SLES9 on ppc64, and it was never seen
with gcc33.
Assume the broken gcc is gcc-3.4 or older.

Signed-off-by: Olaf Hering <olh@suse.de>

---
 fs/reiserfs/Makefile |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.18-rc2/fs/reiserfs/Makefile
===================================================================
--- linux-2.6.18-rc2.orig/fs/reiserfs/Makefile
+++ linux-2.6.18-rc2/fs/reiserfs/Makefile
@@ -28,7 +28,7 @@ endif
 # will work around it. If any other architecture displays this behavior,
 # add it here.
 ifeq ($(CONFIG_PPC32),y)
-EXTRA_CFLAGS := -O1
+EXTRA_CFLAGS := $(call cc-ifversion, -lt, 0400, -O1)
 endif
 
 TAGS: