make gcc -O1 in fs/reiserfs optional
PATCH 1/1 make gcc -O1 in fs/reiserfs optional
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?
fs/reiserfs/Makefile +1 -7
@@ -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
fs/Kconfig +12 -0
@@ -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
Quoted 5 lines · 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?
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 :)
Signature · 2 lines
Jeff Mahoney SuSE Labs
Quoted 48 lines · 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 <[email protected]>Index: linux-2.6.12-rc3-olh/fs/reiserfs/Makefile
===================================================================fs/reiserfs/Makefile +0
-0@@ -21,13 +21,7 @@ ifeq ($(CONFIG_REISERFS_FS_POSIX_ACL),y)reiserfs-objs += xattr_acl.o
endiffs/reiserfs/Makefile +1
-7-# 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
endifIndex: linux-2.6.12-rc3-olh/fs/Kconfig
===================================================================fs/Kconfig +12
-0@@ -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. +
Quoted 7 lines
config
REISERFS_CHECK
bool "Enable reiserfs debug mode"
depends onREISERFS_FS
Sounds reasonable.
PATCH 1/1 make gcc -O1 in fs/reiserfs optional
Quoted 9 lines · Hans Reiser
Quoted 5 lines · 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?
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.
fs/reiserfs/Makefile +1 -7
@@ -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
fs/Kconfig +12 -0
@@ -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
Signature · 2 lines
short story of a lazy sysadmin: alias appserv=wotan
Sounds good to me. Thanks Olaf!
Hans
Quoted 72 lines · Olaf Hering
Quoted 14 lines · Hans Reiser
Quoted 7 lines · 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?
Quoted 3 lines
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 <[email protected]>Index: linux-2.6.12-rc3-olh/fs/reiserfs/Makefile
===================================================================fs/reiserfs/Makefile +0
-0@@ -21,13 +21,7 @@ ifeq ($(CONFIG_REISERFS_FS_POSIX_ACL),y)reiserfs-objs += xattr_acl.o
endiffs/reiserfs/Makefile +1
-7-# 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
endifIndex: linux-2.6.12-rc3-olh/fs/Kconfig
===================================================================fs/Kconfig +12
-0@@ -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 onREISERFS_FS
Quoted 11 lines · Olaf Hering
Quoted 7 lines · Hans Reiser
Quoted 5 lines · 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?
Quoted 6 lines
Quoted 1 line
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.
Quoted 30 lines
Index: linux-2.6.12-rc3-olh/fs/reiserfs/Makefile
===================================================================fs/reiserfs/Makefile +1
-7@@ -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 endifIndex: linux-2.6.12-rc3-olh/fs/Kconfig
===================================================================fs/Kconfig +2
-0@@ -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"
fs/Kconfig config REISERFS_FS · line 200
Quoted 10 lines
fs/Kconfig +10
-0+ 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.
Quoted 8 lines · Andrew Morton
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
Signature · 2 lines
Jeff Mahoney SUSE Labs
PATCH 1/1 use gcc -O1 in fs/reiserfs only for ancient gcc versions
In reply to Olaf Heringonly 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.
--- fs/reiserfs/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/reiserfs/Makefile +1 -1
@@ -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: