[PATCH] prepare kconfig inline optimization for all architectures

[PATCH] prepare kconfig inline optimization for all architectures

[Sam Ravnborg]

Fix the OPTIMIZE inline functionality such that it can be used
by all architectures.
To use it the arch maintainers shall add a
"select HAVE_OPTIMIZE_INLINING" in their arch specific
Kconfig file.

This patch enables OPTIMIZE_INLINING for x86, sparc and sparc64.
If all archs by into the approach the requirment for the select
can be dropped.

The indirection with ARCH__SUPPORTS_OPTIMIZED_INLINING was
killed as this is not needed when using select to
decide what archs supports this.

Signed-off-by: Sam Ravnborg <sam-uyr5N9Q2VtJg9hUCZPvPmw@public.gmane.org>
Cc: David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
---
Largely untested.
This patch was made as a consequence of the CONFIG_OPTIMIZE_INLINING
thread on lkml.

	Sam


diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 49590f8..f08509e 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -68,6 +68,7 @@ config SPARC
 	default y
 	select HAVE_IDE
 	select HAVE_OPROFILE
+	select HAVE_OPTIMIZE_INLINING
 
 # Identify this as a Sparc32 build
 config SPARC32
diff --git a/arch/sparc64/Kconfig b/arch/sparc64/Kconfig
index 8acc5cc..01ecd33 100644
--- a/arch/sparc64/Kconfig
+++ b/arch/sparc64/Kconfig
@@ -9,6 +9,7 @@ config SPARC
 	bool
 	default y
 	select HAVE_OPROFILE
+	select HAVE_OPTIMIZE_INLINING
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 2fadf79..d5e22ef 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -20,6 +20,7 @@ config X86
 	def_bool y
 	select HAVE_IDE
 	select HAVE_OPROFILE
+	select HAVE_OPTIMIZE_INLINING
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 	select HAVE_KVM if ((X86_32 && !X86_VOYAGER && !X86_VISWS && !X86_NUMAQ) || X86_64)
@@ -142,9 +143,6 @@ config AUDIT_ARCH
 config ARCH_SUPPORTS_AOUT
 	def_bool y
 
-config ARCH_SUPPORTS_OPTIMIZED_INLINING
-	def_bool y
-
 # Use the generic interrupt handling code in kernel/irq/:
 config GENERIC_HARDIRQS
 	bool
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 5b1979a..23956ea 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -258,15 +258,3 @@ config CPA_DEBUG
 
 endmenu
 
-config OPTIMIZE_INLINING
-	bool "Allow gcc to uninline functions marked 'inline'"
-	default y
-	help
-	  This option determines if the kernel forces gcc to inline the functions
-	  developers have marked 'inline'. Doing so takes away freedom from gcc to
-	  do what it thinks is best, which is desirable for the gcc 3.x series of
-	  compilers. The gcc 4.x series have a rewritten inlining algorithm and
-	  disabling this option will generate a smaller kernel there. Hopefully
-	  this algorithm is so good that allowing gcc4 to make the decision can
-	  become the default in the future, until then this option is there to
-	  test gcc for this.
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index b2fd754..79b7a34 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -31,8 +31,7 @@
 /*
  * Force always-inline if the user requests it so via the .config:
  */
-#if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \
-    !defined(CONFIG_OPTIMIZE_INLINING) && (__GNUC__ >= 4)
+#if !(defined(CONFIG_OPTIMIZE_INLINING) && (__GNUC__ >= 4))
 # define inline		inline		__attribute__((always_inline))
 # define __inline__	__inline__	__attribute__((always_inline))
 # define __inline	__inline	__attribute__((always_inline))
diff --git a/init/Kconfig b/init/Kconfig
index ba3a389..5d8ef89 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -508,6 +508,25 @@ config CC_OPTIMIZE_FOR_SIZE
 
 	  If unsure, say N.
 
+# archs shall "select HAVE_OPTIMIZE_INLINING" to be offered the
+# possibility to enable OPTIMIZE_INLINING
+config HAVE_OPTIMIZE_INLINING
+	def_bool n
+
+config OPTIMIZE_INLINING
+	bool "Enable gcc 4.0 inline algorithm for functions marked 'inline'"
+	depends on HAVE_OPTIMIZE_INLINING
+	help
+	  This option determines if the kernel forces gcc to inline the
+	  functions developers have marked 'inline'. Doing so takes away
+	  freedom from gcc to do what it thinks is best, which is desirable
+	  for the gcc 3.x series of compilers.
+	  The gcc 4.x series have a rewritten inlining algorithm and
+	  enabling this option will generate a smaller kernel there.
+	  Hopefully this algorithm is so good that allowing gcc4 to
+	  make the decision can become the default in the future,
+	  until then this option is there to test gcc for this.
+
 config SYSCTL
 	bool
 

[PATCH] prepare kconfig inline optimization for all architectures

[Sam Ravnborg]

Fix the OPTIMIZE inline functionality such that it can be used
by all architectures.
To use it the arch maintainers shall add a
"select HAVE_OPTIMIZE_INLINING" in their arch specific
Kconfig file.

This patch enables OPTIMIZE_INLINING for x86, sparc and sparc64.
If all archs by into the approach the requirment for the select
can be dropped.

The indirection with ARCH__SUPPORTS_OPTIMIZED_INLINING was
killed as this is not needed when using select to
decide what archs supports this.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: David Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@elte.hu>
---
Largely untested.
This patch was made as a consequence of the CONFIG_OPTIMIZE_INLINING
thread on lkml.

	Sam


diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 49590f8..f08509e 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -68,6 +68,7 @@ config SPARC
 	default y
 	select HAVE_IDE
 	select HAVE_OPROFILE
+	select HAVE_OPTIMIZE_INLINING
 
 # Identify this as a Sparc32 build
 config SPARC32
diff --git a/arch/sparc64/Kconfig b/arch/sparc64/Kconfig
index 8acc5cc..01ecd33 100644
--- a/arch/sparc64/Kconfig
+++ b/arch/sparc64/Kconfig
@@ -9,6 +9,7 @@ config SPARC
 	bool
 	default y
 	select HAVE_OPROFILE
+	select HAVE_OPTIMIZE_INLINING
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 2fadf79..d5e22ef 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -20,6 +20,7 @@ config X86
 	def_bool y
 	select HAVE_IDE
 	select HAVE_OPROFILE
+	select HAVE_OPTIMIZE_INLINING
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 	select HAVE_KVM if ((X86_32 && !X86_VOYAGER && !X86_VISWS && !X86_NUMAQ) || X86_64)
@@ -142,9 +143,6 @@ config AUDIT_ARCH
 config ARCH_SUPPORTS_AOUT
 	def_bool y
 
-config ARCH_SUPPORTS_OPTIMIZED_INLINING
-	def_bool y
-
 # Use the generic interrupt handling code in kernel/irq/:
 config GENERIC_HARDIRQS
 	bool
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 5b1979a..23956ea 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -258,15 +258,3 @@ config CPA_DEBUG
 
 endmenu
 
-config OPTIMIZE_INLINING
-	bool "Allow gcc to uninline functions marked 'inline'"
-	default y
-	help
-	  This option determines if the kernel forces gcc to inline the functions
-	  developers have marked 'inline'. Doing so takes away freedom from gcc to
-	  do what it thinks is best, which is desirable for the gcc 3.x series of
-	  compilers. The gcc 4.x series have a rewritten inlining algorithm and
-	  disabling this option will generate a smaller kernel there. Hopefully
-	  this algorithm is so good that allowing gcc4 to make the decision can
-	  become the default in the future, until then this option is there to
-	  test gcc for this.
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index b2fd754..79b7a34 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -31,8 +31,7 @@
 /*
  * Force always-inline if the user requests it so via the .config:
  */
-#if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \
-    !defined(CONFIG_OPTIMIZE_INLINING) && (__GNUC__ >= 4)
+#if !(defined(CONFIG_OPTIMIZE_INLINING) && (__GNUC__ >= 4))
 # define inline		inline		__attribute__((always_inline))
 # define __inline__	__inline__	__attribute__((always_inline))
 # define __inline	__inline	__attribute__((always_inline))
diff --git a/init/Kconfig b/init/Kconfig
index ba3a389..5d8ef89 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -508,6 +508,25 @@ config CC_OPTIMIZE_FOR_SIZE
 
 	  If unsure, say N.
 
+# archs shall "select HAVE_OPTIMIZE_INLINING" to be offered the
+# possibility to enable OPTIMIZE_INLINING
+config HAVE_OPTIMIZE_INLINING
+	def_bool n
+
+config OPTIMIZE_INLINING
+	bool "Enable gcc 4.0 inline algorithm for functions marked 'inline'"
+	depends on HAVE_OPTIMIZE_INLINING
+	help
+	  This option determines if the kernel forces gcc to inline the
+	  functions developers have marked 'inline'. Doing so takes away
+	  freedom from gcc to do what it thinks is best, which is desirable
+	  for the gcc 3.x series of compilers.
+	  The gcc 4.x series have a rewritten inlining algorithm and
+	  enabling this option will generate a smaller kernel there.
+	  Hopefully this algorithm is so good that allowing gcc4 to
+	  make the decision can become the default in the future,
+	  until then this option is there to test gcc for this.
+
 config SYSCTL
 	bool
 

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Adrian Bunk]

On Sun, Apr 27, 2008 at 12:51:00PM +0200, Sam Ravnborg wrote:
> Fix the OPTIMIZE inline functionality such that it can be used
> by all architectures.
> To use it the arch maintainers shall add a
> "select HAVE_OPTIMIZE_INLINING" in their arch specific
> Kconfig file.
>...

My opinion on this is still:
"OPTIMIZE" means "work around bugs in the kernel".

Bugs of the following kinds:
- usage of "inline" in .c files (it should virtually never be used there)
- too big functions in header files

And this "inline optimization" reverts only the effects of some of these 
bugs.

And if the wrong code gets not inlined we can even get size increases at 
some places.

Denys and Ilpo have already done some work in fixing such bugs.

After the dust of this merge window has settled I'll try to get us a 
similar result without this "optimization".

> 	Sam
>...

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Adrian Bunk]

On Sun, Apr 27, 2008 at 12:51:00PM +0200, Sam Ravnborg wrote:
> Fix the OPTIMIZE inline functionality such that it can be used
> by all architectures.
> To use it the arch maintainers shall add a
> "select HAVE_OPTIMIZE_INLINING" in their arch specific
> Kconfig file.
>...

My opinion on this is still:
"OPTIMIZE" means "work around bugs in the kernel".

Bugs of the following kinds:
- usage of "inline" in .c files (it should virtually never be used there)
- too big functions in header files

And this "inline optimization" reverts only the effects of some of these 
bugs.

And if the wrong code gets not inlined we can even get size increases at 
some places.

Denys and Ilpo have already done some work in fixing such bugs.

After the dust of this merge window has settled I'll try to get us a 
similar result without this "optimization".

> 	Sam
>...

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Sam Ravnborg]

On Sun, Apr 27, 2008 at 02:31:58PM +0300, Adrian Bunk wrote:
> On Sun, Apr 27, 2008 at 12:51:00PM +0200, Sam Ravnborg wrote:
> > Fix the OPTIMIZE inline functionality such that it can be used
> > by all architectures.
> > To use it the arch maintainers shall add a
> > "select HAVE_OPTIMIZE_INLINING" in their arch specific
> > Kconfig file.
> >...
> 
> My opinion on this is still:
> "OPTIMIZE" means "work around bugs in the kernel".

So then let us name it: GCC_INLINING

Suggested patch below.
I named it _GCC_ because this is a gcc specific feature.

	Sam

diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 49590f8..eec5cee 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -68,6 +68,7 @@ config SPARC
 	default y
 	select HAVE_IDE
 	select HAVE_OPROFILE
+	select HAVE_GCC_INLINING
 
 # Identify this as a Sparc32 build
 config SPARC32
diff --git a/arch/sparc64/Kconfig b/arch/sparc64/Kconfig
index 8acc5cc..98a736c 100644
--- a/arch/sparc64/Kconfig
+++ b/arch/sparc64/Kconfig
@@ -9,6 +9,7 @@ config SPARC
 	bool
 	default y
 	select HAVE_OPROFILE
+	select HAVE_GCC_INLINING
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 2fadf79..b6b7795 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -20,6 +20,7 @@ config X86
 	def_bool y
 	select HAVE_IDE
 	select HAVE_OPROFILE
+	select HAVE_GCC_INLINING
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 	select HAVE_KVM if ((X86_32 && !X86_VOYAGER && !X86_VISWS && !X86_NUMAQ) || X86_64)
@@ -142,9 +143,6 @@ config AUDIT_ARCH
 config ARCH_SUPPORTS_AOUT
 	def_bool y
 
-config ARCH_SUPPORTS_OPTIMIZED_INLINING
-	def_bool y
-
 # Use the generic interrupt handling code in kernel/irq/:
 config GENERIC_HARDIRQS
 	bool
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 5b1979a..23956ea 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -258,15 +258,3 @@ config CPA_DEBUG
 
 endmenu
 
-config OPTIMIZE_INLINING
-	bool "Allow gcc to uninline functions marked 'inline'"
-	default y
-	help
-	  This option determines if the kernel forces gcc to inline the functions
-	  developers have marked 'inline'. Doing so takes away freedom from gcc to
-	  do what it thinks is best, which is desirable for the gcc 3.x series of
-	  compilers. The gcc 4.x series have a rewritten inlining algorithm and
-	  disabling this option will generate a smaller kernel there. Hopefully
-	  this algorithm is so good that allowing gcc4 to make the decision can
-	  become the default in the future, until then this option is there to
-	  test gcc for this.
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index b2fd754..9e1cdd5 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -31,8 +31,7 @@
 /*
  * Force always-inline if the user requests it so via the .config:
  */
-#if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \
-    !defined(CONFIG_OPTIMIZE_INLINING) && (__GNUC__ >= 4)
+#if !(defined(CONFIG_GCC_INLINING) && (__GNUC__ >= 4))
 # define inline		inline		__attribute__((always_inline))
 # define __inline__	__inline__	__attribute__((always_inline))
 # define __inline	__inline	__attribute__((always_inline))
diff --git a/init/Kconfig b/init/Kconfig
index ba3a389..8d6eb70 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -508,6 +508,25 @@ config CC_OPTIMIZE_FOR_SIZE
 
 	  If unsure, say N.
 
+# archs shall "select HAVE_GCC_INLINING" to be offered the
+# possibility to enable GCC_INLINING
+config HAVE_GCC_INLINING
+	def_bool n
+
+config GCC_INLINING
+	bool "Enable gcc 4.0 inline algorithm for functions marked 'inline'"
+	depends on HAVE_GCC_INLINING
+	help
+	  This option determines if the kernel forces gcc to inline the
+	  functions developers have marked 'inline'. Doing so takes away
+	  freedom from gcc to do what it thinks is best, which is desirable
+	  for the gcc 3.x series of compilers.
+	  The gcc 4.x series have a rewritten inlining algorithm and
+	  enabling this option will generate a smaller kernel there.
+	  Hopefully this algorithm is so good that allowing gcc4 to
+	  make the decision can become the default in the future,
+	  until then this option is there to test gcc for this.
+
 config SYSCTL
 	bool
 

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Sam Ravnborg]

On Sun, Apr 27, 2008 at 02:31:58PM +0300, Adrian Bunk wrote:
> On Sun, Apr 27, 2008 at 12:51:00PM +0200, Sam Ravnborg wrote:
> > Fix the OPTIMIZE inline functionality such that it can be used
> > by all architectures.
> > To use it the arch maintainers shall add a
> > "select HAVE_OPTIMIZE_INLINING" in their arch specific
> > Kconfig file.
> >...
> 
> My opinion on this is still:
> "OPTIMIZE" means "work around bugs in the kernel".

So then let us name it: GCC_INLINING

Suggested patch below.
I named it _GCC_ because this is a gcc specific feature.

	Sam

diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 49590f8..eec5cee 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -68,6 +68,7 @@ config SPARC
 	default y
 	select HAVE_IDE
 	select HAVE_OPROFILE
+	select HAVE_GCC_INLINING
 
 # Identify this as a Sparc32 build
 config SPARC32
diff --git a/arch/sparc64/Kconfig b/arch/sparc64/Kconfig
index 8acc5cc..98a736c 100644
--- a/arch/sparc64/Kconfig
+++ b/arch/sparc64/Kconfig
@@ -9,6 +9,7 @@ config SPARC
 	bool
 	default y
 	select HAVE_OPROFILE
+	select HAVE_GCC_INLINING
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 2fadf79..b6b7795 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -20,6 +20,7 @@ config X86
 	def_bool y
 	select HAVE_IDE
 	select HAVE_OPROFILE
+	select HAVE_GCC_INLINING
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 	select HAVE_KVM if ((X86_32 && !X86_VOYAGER && !X86_VISWS && !X86_NUMAQ) || X86_64)
@@ -142,9 +143,6 @@ config AUDIT_ARCH
 config ARCH_SUPPORTS_AOUT
 	def_bool y
 
-config ARCH_SUPPORTS_OPTIMIZED_INLINING
-	def_bool y
-
 # Use the generic interrupt handling code in kernel/irq/:
 config GENERIC_HARDIRQS
 	bool
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 5b1979a..23956ea 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -258,15 +258,3 @@ config CPA_DEBUG
 
 endmenu
 
-config OPTIMIZE_INLINING
-	bool "Allow gcc to uninline functions marked 'inline'"
-	default y
-	help
-	  This option determines if the kernel forces gcc to inline the functions
-	  developers have marked 'inline'. Doing so takes away freedom from gcc to
-	  do what it thinks is best, which is desirable for the gcc 3.x series of
-	  compilers. The gcc 4.x series have a rewritten inlining algorithm and
-	  disabling this option will generate a smaller kernel there. Hopefully
-	  this algorithm is so good that allowing gcc4 to make the decision can
-	  become the default in the future, until then this option is there to
-	  test gcc for this.
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index b2fd754..9e1cdd5 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -31,8 +31,7 @@
 /*
  * Force always-inline if the user requests it so via the .config:
  */
-#if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \
-    !defined(CONFIG_OPTIMIZE_INLINING) && (__GNUC__ >= 4)
+#if !(defined(CONFIG_GCC_INLINING) && (__GNUC__ >= 4))
 # define inline		inline		__attribute__((always_inline))
 # define __inline__	__inline__	__attribute__((always_inline))
 # define __inline	__inline	__attribute__((always_inline))
diff --git a/init/Kconfig b/init/Kconfig
index ba3a389..8d6eb70 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -508,6 +508,25 @@ config CC_OPTIMIZE_FOR_SIZE
 
 	  If unsure, say N.
 
+# archs shall "select HAVE_GCC_INLINING" to be offered the
+# possibility to enable GCC_INLINING
+config HAVE_GCC_INLINING
+	def_bool n
+
+config GCC_INLINING
+	bool "Enable gcc 4.0 inline algorithm for functions marked 'inline'"
+	depends on HAVE_GCC_INLINING
+	help
+	  This option determines if the kernel forces gcc to inline the
+	  functions developers have marked 'inline'. Doing so takes away
+	  freedom from gcc to do what it thinks is best, which is desirable
+	  for the gcc 3.x series of compilers.
+	  The gcc 4.x series have a rewritten inlining algorithm and
+	  enabling this option will generate a smaller kernel there.
+	  Hopefully this algorithm is so good that allowing gcc4 to
+	  make the decision can become the default in the future,
+	  until then this option is there to test gcc for this.
+
 config SYSCTL
 	bool
 

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Ingo Molnar]

* Sam Ravnborg <sam-uyr5N9Q2VtJg9hUCZPvPmw@public.gmane.org> wrote:

> > My opinion on this is still:
> > "OPTIMIZE" means "work around bugs in the kernel".
> 
> So then let us name it: GCC_INLINING

i object to that - please keep it named the way we came up with: 
OPTIMIZE_INLINING=y - because that's what it does.

It goes along the same line as CC_OPTIMIZE_FOR_SIZE=y. Perhaps rename it 
to CC_OPTIMIZE_INLINING=y?

	Ingo

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Ingo Molnar]

* Sam Ravnborg <sam@ravnborg.org> wrote:

> > My opinion on this is still:
> > "OPTIMIZE" means "work around bugs in the kernel".
> 
> So then let us name it: GCC_INLINING

i object to that - please keep it named the way we came up with: 
OPTIMIZE_INLINING=y - because that's what it does.

It goes along the same line as CC_OPTIMIZE_FOR_SIZE=y. Perhaps rename it 
to CC_OPTIMIZE_INLINING=y?

	Ingo

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Ingo Molnar]

* Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org> wrote:

> It goes along the same line as CC_OPTIMIZE_FOR_SIZE=y. Perhaps rename 
> it to CC_OPTIMIZE_INLINING=y?

i.e. like the patch below.

	Ingo

-------------------->
Subject: enable gcc inline optimization for all architectures
From: Sam Ravnborg <sam-uyr5N9Q2VtJg9hUCZPvPmw@public.gmane.org>
Date: Sun, 27 Apr 2008 13:56:04 +0200

architectures that want to enable this feature should select 
HAVE_CC_OPTIMIZE_INLINING.

Signed-off-by: Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
---
 arch/sparc/Kconfig           |    1 +
 arch/sparc64/Kconfig         |    1 +
 arch/x86/Kconfig             |    4 +---
 arch/x86/Kconfig.debug       |   12 ------------
 include/linux/compiler-gcc.h |    3 +--
 init/Kconfig                 |   19 +++++++++++++++++++
 6 files changed, 23 insertions(+), 17 deletions(-)

Index: linux/arch/sparc/Kconfig
===================================================================
--- linux.orig/arch/sparc/Kconfig
+++ linux/arch/sparc/Kconfig
@@ -68,6 +68,7 @@ config SPARC
 	default y
 	select HAVE_IDE
 	select HAVE_OPROFILE
+	select HAVE_CC_OPTIMIZE_INLINING
 
 # Identify this as a Sparc32 build
 config SPARC32
Index: linux/arch/sparc64/Kconfig
===================================================================
--- linux.orig/arch/sparc64/Kconfig
+++ linux/arch/sparc64/Kconfig
@@ -9,6 +9,7 @@ config SPARC
 	bool
 	default y
 	select HAVE_OPROFILE
+	select HAVE_CC_OPTIMIZE_INLINING
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 
Index: linux/arch/x86/Kconfig
===================================================================
--- linux.orig/arch/x86/Kconfig
+++ linux/arch/x86/Kconfig
@@ -20,6 +20,7 @@ config X86
 	def_bool y
 	select HAVE_IDE
 	select HAVE_OPROFILE
+	select HAVE_CC_OPTIMIZE_INLINING
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 	select HAVE_KVM if ((X86_32 && !X86_VOYAGER && !X86_VISWS && !X86_NUMAQ) || X86_64)
@@ -142,9 +143,6 @@ config AUDIT_ARCH
 config ARCH_SUPPORTS_AOUT
 	def_bool y
 
-config ARCH_SUPPORTS_OPTIMIZED_INLINING
-	def_bool y
-
 # Use the generic interrupt handling code in kernel/irq/:
 config GENERIC_HARDIRQS
 	bool
Index: linux/arch/x86/Kconfig.debug
===================================================================
--- linux.orig/arch/x86/Kconfig.debug
+++ linux/arch/x86/Kconfig.debug
@@ -258,15 +258,3 @@ config CPA_DEBUG
 
 endmenu
 
-config OPTIMIZE_INLINING
-	bool "Allow gcc to uninline functions marked 'inline'"
-	default y
-	help
-	  This option determines if the kernel forces gcc to inline the functions
-	  developers have marked 'inline'. Doing so takes away freedom from gcc to
-	  do what it thinks is best, which is desirable for the gcc 3.x series of
-	  compilers. The gcc 4.x series have a rewritten inlining algorithm and
-	  disabling this option will generate a smaller kernel there. Hopefully
-	  this algorithm is so good that allowing gcc4 to make the decision can
-	  become the default in the future, until then this option is there to
-	  test gcc for this.
Index: linux/include/linux/compiler-gcc.h
===================================================================
--- linux.orig/include/linux/compiler-gcc.h
+++ linux/include/linux/compiler-gcc.h
@@ -31,8 +31,7 @@
 /*
  * Force always-inline if the user requests it so via the .config:
  */
-#if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \
-    !defined(CONFIG_OPTIMIZE_INLINING) && (__GNUC__ >= 4)
+#if !(defined(CONFIG_CC_OPTIMIZE_INLINING) && (__GNUC__ >= 4))
 # define inline		inline		__attribute__((always_inline))
 # define __inline__	__inline__	__attribute__((always_inline))
 # define __inline	__inline	__attribute__((always_inline))
Index: linux/init/Kconfig
===================================================================
--- linux.orig/init/Kconfig
+++ linux/init/Kconfig
@@ -508,6 +508,25 @@ config CC_OPTIMIZE_FOR_SIZE
 
 	  If unsure, say N.
 
+# archs shall "select HAVE_CC_OPTIMIZE_INLINING" to be offered the
+# possibility to enable CC_OPTIMIZE_INLINING
+config HAVE_CC_OPTIMIZE_INLINING
+	def_bool n
+
+config CC_OPTIMIZE_INLINING
+	bool "Allow gcc to uninline functions marked 'inline'"
+	depends on HAVE_CC_OPTIMIZE_INLINING
+	help
+	  This option determines if the kernel forces gcc to inline the
+	  functions developers have marked 'inline'. Doing so takes away
+	  freedom from gcc to do what it thinks is best, which is desirable
+	  for the gcc 3.x series of compilers.
+	  The gcc 4.x series have a rewritten inlining algorithm and
+	  enabling this option will generate a smaller kernel there.
+	  Hopefully this algorithm is so good that allowing gcc4 to
+	  make the decision can become the default in the future,
+	  until then this option is there to test gcc for this.
+
 config SYSCTL
 	bool
 

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> It goes along the same line as CC_OPTIMIZE_FOR_SIZE=y. Perhaps rename 
> it to CC_OPTIMIZE_INLINING=y?

i.e. like the patch below.

	Ingo

-------------------->
Subject: enable gcc inline optimization for all architectures
From: Sam Ravnborg <sam@ravnborg.org>
Date: Sun, 27 Apr 2008 13:56:04 +0200

architectures that want to enable this feature should select 
HAVE_CC_OPTIMIZE_INLINING.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/sparc/Kconfig           |    1 +
 arch/sparc64/Kconfig         |    1 +
 arch/x86/Kconfig             |    4 +---
 arch/x86/Kconfig.debug       |   12 ------------
 include/linux/compiler-gcc.h |    3 +--
 init/Kconfig                 |   19 +++++++++++++++++++
 6 files changed, 23 insertions(+), 17 deletions(-)

Index: linux/arch/sparc/Kconfig
===================================================================
--- linux.orig/arch/sparc/Kconfig
+++ linux/arch/sparc/Kconfig
@@ -68,6 +68,7 @@ config SPARC
 	default y
 	select HAVE_IDE
 	select HAVE_OPROFILE
+	select HAVE_CC_OPTIMIZE_INLINING
 
 # Identify this as a Sparc32 build
 config SPARC32
Index: linux/arch/sparc64/Kconfig
===================================================================
--- linux.orig/arch/sparc64/Kconfig
+++ linux/arch/sparc64/Kconfig
@@ -9,6 +9,7 @@ config SPARC
 	bool
 	default y
 	select HAVE_OPROFILE
+	select HAVE_CC_OPTIMIZE_INLINING
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 
Index: linux/arch/x86/Kconfig
===================================================================
--- linux.orig/arch/x86/Kconfig
+++ linux/arch/x86/Kconfig
@@ -20,6 +20,7 @@ config X86
 	def_bool y
 	select HAVE_IDE
 	select HAVE_OPROFILE
+	select HAVE_CC_OPTIMIZE_INLINING
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 	select HAVE_KVM if ((X86_32 && !X86_VOYAGER && !X86_VISWS && !X86_NUMAQ) || X86_64)
@@ -142,9 +143,6 @@ config AUDIT_ARCH
 config ARCH_SUPPORTS_AOUT
 	def_bool y
 
-config ARCH_SUPPORTS_OPTIMIZED_INLINING
-	def_bool y
-
 # Use the generic interrupt handling code in kernel/irq/:
 config GENERIC_HARDIRQS
 	bool
Index: linux/arch/x86/Kconfig.debug
===================================================================
--- linux.orig/arch/x86/Kconfig.debug
+++ linux/arch/x86/Kconfig.debug
@@ -258,15 +258,3 @@ config CPA_DEBUG
 
 endmenu
 
-config OPTIMIZE_INLINING
-	bool "Allow gcc to uninline functions marked 'inline'"
-	default y
-	help
-	  This option determines if the kernel forces gcc to inline the functions
-	  developers have marked 'inline'. Doing so takes away freedom from gcc to
-	  do what it thinks is best, which is desirable for the gcc 3.x series of
-	  compilers. The gcc 4.x series have a rewritten inlining algorithm and
-	  disabling this option will generate a smaller kernel there. Hopefully
-	  this algorithm is so good that allowing gcc4 to make the decision can
-	  become the default in the future, until then this option is there to
-	  test gcc for this.
Index: linux/include/linux/compiler-gcc.h
===================================================================
--- linux.orig/include/linux/compiler-gcc.h
+++ linux/include/linux/compiler-gcc.h
@@ -31,8 +31,7 @@
 /*
  * Force always-inline if the user requests it so via the .config:
  */
-#if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \
-    !defined(CONFIG_OPTIMIZE_INLINING) && (__GNUC__ >= 4)
+#if !(defined(CONFIG_CC_OPTIMIZE_INLINING) && (__GNUC__ >= 4))
 # define inline		inline		__attribute__((always_inline))
 # define __inline__	__inline__	__attribute__((always_inline))
 # define __inline	__inline	__attribute__((always_inline))
Index: linux/init/Kconfig
===================================================================
--- linux.orig/init/Kconfig
+++ linux/init/Kconfig
@@ -508,6 +508,25 @@ config CC_OPTIMIZE_FOR_SIZE
 
 	  If unsure, say N.
 
+# archs shall "select HAVE_CC_OPTIMIZE_INLINING" to be offered the
+# possibility to enable CC_OPTIMIZE_INLINING
+config HAVE_CC_OPTIMIZE_INLINING
+	def_bool n
+
+config CC_OPTIMIZE_INLINING
+	bool "Allow gcc to uninline functions marked 'inline'"
+	depends on HAVE_CC_OPTIMIZE_INLINING
+	help
+	  This option determines if the kernel forces gcc to inline the
+	  functions developers have marked 'inline'. Doing so takes away
+	  freedom from gcc to do what it thinks is best, which is desirable
+	  for the gcc 3.x series of compilers.
+	  The gcc 4.x series have a rewritten inlining algorithm and
+	  enabling this option will generate a smaller kernel there.
+	  Hopefully this algorithm is so good that allowing gcc4 to
+	  make the decision can become the default in the future,
+	  until then this option is there to test gcc for this.
+
 config SYSCTL
 	bool
 

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Linus Torvalds]

On Sun, 27 Apr 2008, Adrian Bunk wrote:
> 
> My opinion on this is still:
> "OPTIMIZE" means "work around bugs in the kernel".

No.

It means that

 - gcc used to (long ago) always honor "inline", and we had kernel code 
   that depended on that in various ways (ie required that there was no 
   return etc).

   We've been mostly replacing the ones we know about with 
   "__always_inline", but there may be some that remain. We'll find out, I 
   guess.

 - gcc was a total and utter piece of horrible crap in the inlining 
   department, doign insane things and changing their documentation to 
   match the new behaviour (and some people then claimed that it was 
   always documented that way).

   It would not inline big functions even when they statically collapsed 
   to nothing, etc.

As a result, we really couldn't afford to let gcc make any inlining 
decisions, because the compiler was simply *broken*.

			Linus

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Linus Torvalds]

On Sun, 27 Apr 2008, Adrian Bunk wrote:
> 
> My opinion on this is still:
> "OPTIMIZE" means "work around bugs in the kernel".

No.

It means that

 - gcc used to (long ago) always honor "inline", and we had kernel code 
   that depended on that in various ways (ie required that there was no 
   return etc).

   We've been mostly replacing the ones we know about with 
   "__always_inline", but there may be some that remain. We'll find out, I 
   guess.

 - gcc was a total and utter piece of horrible crap in the inlining 
   department, doign insane things and changing their documentation to 
   match the new behaviour (and some people then claimed that it was 
   always documented that way).

   It would not inline big functions even when they statically collapsed 
   to nothing, etc.

As a result, we really couldn't afford to let gcc make any inlining 
decisions, because the compiler was simply *broken*.

			Linus

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Adrian Bunk]

On Sun, Apr 27, 2008 at 10:06:41AM -0700, Linus Torvalds wrote:
> 
> 
> On Sun, 27 Apr 2008, Adrian Bunk wrote:
> > 
> > My opinion on this is still:
> > "OPTIMIZE" means "work around bugs in the kernel".
> 
> No.
> 
> It means that
> 
>  - gcc used to (long ago) always honor "inline", and we had kernel code 
>    that depended on that in various ways (ie required that there was no 
>    return etc).
> 
>    We've been mostly replacing the ones we know about with 
>    "__always_inline", but there may be some that remain. We'll find out, I 
>    guess.
> 
>  - gcc was a total and utter piece of horrible crap in the inlining 
>    department, doign insane things and changing their documentation to 
>    match the new behaviour (and some people then claimed that it was 
>    always documented that way).
> 
>    It would not inline big functions even when they statically collapsed 
>    to nothing, etc.
> 
> As a result, we really couldn't afford to let gcc make any inlining 
> decisions, because the compiler was simply *broken*.

I'm looking at it from a different angle, all code in the kernel should 
follow the following rules [1]:
- no functions in .c files should be marked inline
- all functions in headers should be static inline
- all functions in headers should either be very small or collapse
  to become very small after inlining

I can simply not see any usecase for a non-forced inline in the kernel,
and fixing the kernel should give a superset of the space savings of 
this "inline optimization".

> 			Linus

cu
Adrian

[1] there might be rare exceptions

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Adrian Bunk]

On Sun, Apr 27, 2008 at 10:06:41AM -0700, Linus Torvalds wrote:
> 
> 
> On Sun, 27 Apr 2008, Adrian Bunk wrote:
> > 
> > My opinion on this is still:
> > "OPTIMIZE" means "work around bugs in the kernel".
> 
> No.
> 
> It means that
> 
>  - gcc used to (long ago) always honor "inline", and we had kernel code 
>    that depended on that in various ways (ie required that there was no 
>    return etc).
> 
>    We've been mostly replacing the ones we know about with 
>    "__always_inline", but there may be some that remain. We'll find out, I 
>    guess.
> 
>  - gcc was a total and utter piece of horrible crap in the inlining 
>    department, doign insane things and changing their documentation to 
>    match the new behaviour (and some people then claimed that it was 
>    always documented that way).
> 
>    It would not inline big functions even when they statically collapsed 
>    to nothing, etc.
> 
> As a result, we really couldn't afford to let gcc make any inlining 
> decisions, because the compiler was simply *broken*.

I'm looking at it from a different angle, all code in the kernel should 
follow the following rules [1]:
- no functions in .c files should be marked inline
- all functions in headers should be static inline
- all functions in headers should either be very small or collapse
  to become very small after inlining

I can simply not see any usecase for a non-forced inline in the kernel,
and fixing the kernel should give a superset of the space savings of 
this "inline optimization".

> 			Linus

cu
Adrian

[1] there might be rare exceptions

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Linus Torvalds]

On Sun, 27 Apr 2008, Adrian Bunk wrote:
> 
> I'm looking at it from a different angle, all code in the kernel should 
> follow the following rules [1]:
> - no functions in .c files should be marked inline
> - all functions in headers should be static inline
> - all functions in headers should either be very small or collapse
>   to become very small after inlining
> 
> I can simply not see any usecase for a non-forced inline in the kernel,
> and fixing the kernel should give a superset of the space savings of 
> this "inline optimization".

Your whole argument is premised on the assumption that the compiler does 
the right thing.

That's a *known*to*be*bogus* assumption.

Modern versions of gcc may do the right thing. Note the two very important 
code-words: "modern" and "may".

I'm just telling you that

 - older versions of gcc (and by "older" I do not mean "really ancient" or 
   "deprecated", but stuff that is still in use) are known to be total and 
   utter crap when it comes to inlining

 - even absent that, there are historical reasons stemming from even more 
   ancient versions of gcc that are no longer in use.

In other words, my arguments have nothing to do with "I wish". They are 
plain facts. Why argue with them?

			Linus

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Linus Torvalds]

On Sun, 27 Apr 2008, Adrian Bunk wrote:
> 
> I'm looking at it from a different angle, all code in the kernel should 
> follow the following rules [1]:
> - no functions in .c files should be marked inline
> - all functions in headers should be static inline
> - all functions in headers should either be very small or collapse
>   to become very small after inlining
> 
> I can simply not see any usecase for a non-forced inline in the kernel,
> and fixing the kernel should give a superset of the space savings of 
> this "inline optimization".

Your whole argument is premised on the assumption that the compiler does 
the right thing.

That's a *known*to*be*bogus* assumption.

Modern versions of gcc may do the right thing. Note the two very important 
code-words: "modern" and "may".

I'm just telling you that

 - older versions of gcc (and by "older" I do not mean "really ancient" or 
   "deprecated", but stuff that is still in use) are known to be total and 
   utter crap when it comes to inlining

 - even absent that, there are historical reasons stemming from even more 
   ancient versions of gcc that are no longer in use.

In other words, my arguments have nothing to do with "I wish". They are 
plain facts. Why argue with them?

			Linus

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Adrian Bunk]

On Sun, Apr 27, 2008 at 10:32:28AM -0700, Linus Torvalds wrote:
> 
> 
> On Sun, 27 Apr 2008, Adrian Bunk wrote:
> > 
> > I'm looking at it from a different angle, all code in the kernel should 
> > follow the following rules [1]:
> > - no functions in .c files should be marked inline
> > - all functions in headers should be static inline
> > - all functions in headers should either be very small or collapse
> >   to become very small after inlining
> > 
> > I can simply not see any usecase for a non-forced inline in the kernel,
> > and fixing the kernel should give a superset of the space savings of 
> > this "inline optimization".
> 
> Your whole argument is premised on the assumption that the compiler does 
> the right thing.
>...

No, you seem to be misunderstanding what I am saying.

Status Quo as of 2.6.25:
- we force the compiler to always inline with "inline"
- we have inline's in .c files and too big functions in headers, and
  both of them are wrong

"inline optimization":
- we leave the compiler the choice whether or not to inline with "inline"
- we still have inline's in .c files and too big functions in headers, 
  and both of them are wrong, but this "inline optimization" works 
  around these in some of the cases

What I want instead:
- we continue to force the compiler to always inline with "inline"
- we remove the inline's in .c files and make too big functions in 
  headers out-of-line

> 			Linus

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Adrian Bunk]

On Sun, Apr 27, 2008 at 10:32:28AM -0700, Linus Torvalds wrote:
> 
> 
> On Sun, 27 Apr 2008, Adrian Bunk wrote:
> > 
> > I'm looking at it from a different angle, all code in the kernel should 
> > follow the following rules [1]:
> > - no functions in .c files should be marked inline
> > - all functions in headers should be static inline
> > - all functions in headers should either be very small or collapse
> >   to become very small after inlining
> > 
> > I can simply not see any usecase for a non-forced inline in the kernel,
> > and fixing the kernel should give a superset of the space savings of 
> > this "inline optimization".
> 
> Your whole argument is premised on the assumption that the compiler does 
> the right thing.
>...

No, you seem to be misunderstanding what I am saying.

Status Quo as of 2.6.25:
- we force the compiler to always inline with "inline"
- we have inline's in .c files and too big functions in headers, and
  both of them are wrong

"inline optimization":
- we leave the compiler the choice whether or not to inline with "inline"
- we still have inline's in .c files and too big functions in headers, 
  and both of them are wrong, but this "inline optimization" works 
  around these in some of the cases

What I want instead:
- we continue to force the compiler to always inline with "inline"
- we remove the inline's in .c files and make too big functions in 
  headers out-of-line

> 			Linus

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Christoph Hellwig]

On Sun, Apr 27, 2008 at 08:47:14PM +0300, Adrian Bunk wrote:
> What I want instead:
> - we continue to force the compiler to always inline with "inline"
> - we remove the inline's in .c files and make too big functions in 
>   headers out-of-line

It doesn't matter whether we use inline or always_inline to force
inlining of a function, that's just syntactical sugar.  What is rather
annoying is that with the config option we'd have something marked
inline without actually meaning it's inline.  And what's even worse
is that this is depending on a user-visible config option which is
entirely stupid.

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Christoph Hellwig]

On Sun, Apr 27, 2008 at 08:47:14PM +0300, Adrian Bunk wrote:
> What I want instead:
> - we continue to force the compiler to always inline with "inline"
> - we remove the inline's in .c files and make too big functions in 
>   headers out-of-line

It doesn't matter whether we use inline or always_inline to force
inlining of a function, that's just syntactical sugar.  What is rather
annoying is that with the config option we'd have something marked
inline without actually meaning it's inline.  And what's even worse
is that this is depending on a user-visible config option which is
entirely stupid.

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Sam Ravnborg]

On Sun, Apr 27, 2008 at 02:00:07PM -0400, Christoph Hellwig wrote:
> On Sun, Apr 27, 2008 at 08:47:14PM +0300, Adrian Bunk wrote:
> > What I want instead:
> > - we continue to force the compiler to always inline with "inline"
> > - we remove the inline's in .c files and make too big functions in 
> >   headers out-of-line
> 
> It doesn't matter whether we use inline or always_inline to force
> inlining of a function, that's just syntactical sugar.

inline => the developer hints the compiler that it may
          be a good idea to inline this function

always_inline => the developer tell gcc that for some obscure reasons
                 MUST always inline uses of this function

> What is rather
> annoying is that with the config option we'd have something marked
> inline without actually meaning it's inline.
With the config option we pass the inline hint to gcc (if enabled).
So with the config option we have the possibility to pass a _hint_ to
gcc about inlining.

Before the config option there were no difference between
static int alwyas_inline foo() {}
and
static int inline foo() {}

With the config option we now have a situation where they actually
differ as they should do (assuming gcc > 4.x).

> And what's even worse
> is that this is depending on a user-visible config option which is
> entirely stupid.
So you say that it is safe to assume all places where we really need
always_inline are annotedted such - and we do not need a simple
config option that the user can uncheck.
Fine by me - I prefer the simpler solution.

	Sam

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Sam Ravnborg]

On Sun, Apr 27, 2008 at 02:00:07PM -0400, Christoph Hellwig wrote:
> On Sun, Apr 27, 2008 at 08:47:14PM +0300, Adrian Bunk wrote:
> > What I want instead:
> > - we continue to force the compiler to always inline with "inline"
> > - we remove the inline's in .c files and make too big functions in 
> >   headers out-of-line
> 
> It doesn't matter whether we use inline or always_inline to force
> inlining of a function, that's just syntactical sugar.

inline => the developer hints the compiler that it may
          be a good idea to inline this function

always_inline => the developer tell gcc that for some obscure reasons
                 MUST always inline uses of this function

> What is rather
> annoying is that with the config option we'd have something marked
> inline without actually meaning it's inline.
With the config option we pass the inline hint to gcc (if enabled).
So with the config option we have the possibility to pass a _hint_ to
gcc about inlining.

Before the config option there were no difference between
static int alwyas_inline foo() {}
and
static int inline foo() {}

With the config option we now have a situation where they actually
differ as they should do (assuming gcc > 4.x).

> And what's even worse
> is that this is depending on a user-visible config option which is
> entirely stupid.
So you say that it is safe to assume all places where we really need
always_inline are annotedted such - and we do not need a simple
config option that the user can uncheck.
Fine by me - I prefer the simpler solution.

	Sam

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Christoph Hellwig]

On Sun, Apr 27, 2008 at 08:09:57PM +0200, Sam Ravnborg wrote:
> With the config option we pass the inline hint to gcc (if enabled).
> So with the config option we have the possibility to pass a _hint_ to
> gcc about inlining.
> 
> Before the config option there were no difference between
> static int alwyas_inline foo() {}
> and
> static int inline foo() {}
> 
> With the config option we now have a situation where they actually
> differ as they should do (assuming gcc > 4.x).

As Linus mentioned the hint doesn't make any sense because gcc will
get it wrong anyway.  In fact when you look at kernel code it tends
to inline the everything and the kitchensink as long as there's just
one caller and this bloat the stack but doesn't inline where it needs
to.  Better don't try to mess with that and do it explicit.

> So you say that it is safe to assume all places where we really need
> always_inline are annotedted such - and we do not need a simple
> config option that the user can uncheck.

I don't say it is that, it certainly isn't on powerpc and probably most
other architectures right now, because only x86 got the fixup so far.
But making it a user-visible option instead of an architecture opt
in/out selection doesn't make any sense.

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Christoph Hellwig]

On Sun, Apr 27, 2008 at 08:09:57PM +0200, Sam Ravnborg wrote:
> With the config option we pass the inline hint to gcc (if enabled).
> So with the config option we have the possibility to pass a _hint_ to
> gcc about inlining.
> 
> Before the config option there were no difference between
> static int alwyas_inline foo() {}
> and
> static int inline foo() {}
> 
> With the config option we now have a situation where they actually
> differ as they should do (assuming gcc > 4.x).

As Linus mentioned the hint doesn't make any sense because gcc will
get it wrong anyway.  In fact when you look at kernel code it tends
to inline the everything and the kitchensink as long as there's just
one caller and this bloat the stack but doesn't inline where it needs
to.  Better don't try to mess with that and do it explicit.

> So you say that it is safe to assume all places where we really need
> always_inline are annotedted such - and we do not need a simple
> config option that the user can uncheck.

I don't say it is that, it certainly isn't on powerpc and probably most
other architectures right now, because only x86 got the fixup so far.
But making it a user-visible option instead of an architecture opt
in/out selection doesn't make any sense.

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Linus Torvalds]

On Sun, 27 Apr 2008, Christoph Hellwig wrote:
> 
> As Linus mentioned the hint doesn't make any sense because gcc will
> get it wrong anyway.  In fact when you look at kernel code it tends
> to inline the everything and the kitchensink as long as there's just
> one caller and this bloat the stack but doesn't inline where it needs
> to.  Better don't try to mess with that and do it explicit.

The thing is, the "inline" vs "always_inline" thing _could_ make sense, 
but sadly doesn't much.

Part of it is that gcc imnsho inlines too aggressively anyway in the 
absense of "inline", so there's no way "inline" can mean "you might 
inline" this, because gcc will do that anyway even without it. As a 
result, in _practice_ "inline" and "always_inline" end up being very close 
to each other - perhaps more so than they should.

I do obviously think that we're right to move into the direction that 
"inline" should be a hint. In fact, the biggest issue I have with the new 
kconfig option is that I think it should probably be unconditional, but I 
suspect that compiler issues and architecture issues make that not be a 
good idea.

It will take time before we've sorted out all the fall-out, because I bet 
there is still code out there that _should_ use __always_inline, but 
doesn't.

			Linus

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Linus Torvalds]

On Sun, 27 Apr 2008, Christoph Hellwig wrote:
> 
> As Linus mentioned the hint doesn't make any sense because gcc will
> get it wrong anyway.  In fact when you look at kernel code it tends
> to inline the everything and the kitchensink as long as there's just
> one caller and this bloat the stack but doesn't inline where it needs
> to.  Better don't try to mess with that and do it explicit.

The thing is, the "inline" vs "always_inline" thing _could_ make sense, 
but sadly doesn't much.

Part of it is that gcc imnsho inlines too aggressively anyway in the 
absense of "inline", so there's no way "inline" can mean "you might 
inline" this, because gcc will do that anyway even without it. As a 
result, in _practice_ "inline" and "always_inline" end up being very close 
to each other - perhaps more so than they should.

I do obviously think that we're right to move into the direction that 
"inline" should be a hint. In fact, the biggest issue I have with the new 
kconfig option is that I think it should probably be unconditional, but I 
suspect that compiler issues and architecture issues make that not be a 
good idea.

It will take time before we've sorted out all the fall-out, because I bet 
there is still code out there that _should_ use __always_inline, but 
doesn't.

			Linus

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Christoph Hellwig]

On Sun, Apr 27, 2008 at 11:24:28AM -0700, Linus Torvalds wrote:
> The thing is, the "inline" vs "always_inline" thing _could_ make sense, 
> but sadly doesn't much.
> 
> Part of it is that gcc imnsho inlines too aggressively anyway in the 
> absense of "inline", so there's no way "inline" can mean "you might 
> inline" this, because gcc will do that anyway even without it. As a 
> result, in _practice_ "inline" and "always_inline" end up being very close 
> to each other - perhaps more so than they should.

Yes, absolutely.   In recent gcc versions it's in fact so bad that xfs
had to add a hack to mark every function not explicitly marker inline
noinline because otherwise gcc would inline gazillions of big functions
which combines with it's horrible liveness analysis led to enormous
stack useage.  (I think the liveness part is getting better these days
to be fair)

> It will take time before we've sorted out all the fall-out, because I bet 
> there is still code out there that _should_ use __always_inline, but 
> doesn't.

Given that __always_inline only shows up in non-x86 arch code in
homeopathic quantity it's a fair bet that there's a lot of breakage
waiting there.  Which was one of the initial complains about the way
it's done..

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Christoph Hellwig]

On Sun, Apr 27, 2008 at 11:24:28AM -0700, Linus Torvalds wrote:
> The thing is, the "inline" vs "always_inline" thing _could_ make sense, 
> but sadly doesn't much.
> 
> Part of it is that gcc imnsho inlines too aggressively anyway in the 
> absense of "inline", so there's no way "inline" can mean "you might 
> inline" this, because gcc will do that anyway even without it. As a 
> result, in _practice_ "inline" and "always_inline" end up being very close 
> to each other - perhaps more so than they should.

Yes, absolutely.   In recent gcc versions it's in fact so bad that xfs
had to add a hack to mark every function not explicitly marker inline
noinline because otherwise gcc would inline gazillions of big functions
which combines with it's horrible liveness analysis led to enormous
stack useage.  (I think the liveness part is getting better these days
to be fair)

> It will take time before we've sorted out all the fall-out, because I bet 
> there is still code out there that _should_ use __always_inline, but 
> doesn't.

Given that __always_inline only shows up in non-x86 arch code in
homeopathic quantity it's a fair bet that there's a lot of breakage
waiting there.  Which was one of the initial complains about the way
it's done..

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Ingo Molnar]

* Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:

> I do obviously think that we're right to move into the direction that 
> "inline" should be a hint. In fact, the biggest issue I have with the 
> new kconfig option is that I think it should probably be 
> unconditional, but I suspect that compiler issues and architecture 
> issues make that not be a good idea.
> 
> It will take time before we've sorted out all the fall-out, because I 
> bet there is still code out there that _should_ use __always_inline, 
> but doesn't.

yeah, we i think had one such issue in x86 (merged upstream prior and 
independently of this feature iirc). We also know that something on 
powerpc doesnt work right out of box with inline being a hint. And 
that's OK and expected - for years we had "inline == always_inline" and 
there was just no force that moved against that. It was in fact 
surprising how well it all worked out on x86 when we reintroduced this 
feature.

btw., another new aspect of this whole area is that a number of major 
distributions now do CONFIG_CC_OPTIMIZE_FOR_SIZE=y in their kernels, and 
that seems to have brought new life into -Os development and with every 
new gcc generation we get denser and denser code generated. It is an 
interesting fact that generating good _small_ code needs a very good 
compiler that understands the code flow very accurately and is able to 
find the smallest possible representation for that.

A side-effect of that is if we do -Os _and_ turn inline into a hint via 
CONFIG_CC_OPTIMIZE_INLINING=y, gcc tends to do a better job than before. 
It cannot just go and inline stuff agressively in stark contrast to the 
-Os goal that was given to it. It still does not do a perfect job though 
- but now at least it has a _chance_ to improve. With always_inline we 
gave gcc no leeway at all.

a second aspect is that there's always the chance that some other 
compiler comes along that does things better than gcc. Again, with 
inline being a necessity we make it impossible for even sane compilers 
to do the right thing.

and a third aspect is that in the current -git kernel we have exactly 
11801 inlines in non-include files. Fighting the constant influx of them 
was a futile act 3 years ago when Arjan and me created this feature for 
the first time and it is futile today. It is a compiler domain problem 
and we should not waste any manpower "fighting" them. Life is too short :)

	Ingo

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> I do obviously think that we're right to move into the direction that 
> "inline" should be a hint. In fact, the biggest issue I have with the 
> new kconfig option is that I think it should probably be 
> unconditional, but I suspect that compiler issues and architecture 
> issues make that not be a good idea.
> 
> It will take time before we've sorted out all the fall-out, because I 
> bet there is still code out there that _should_ use __always_inline, 
> but doesn't.

yeah, we i think had one such issue in x86 (merged upstream prior and 
independently of this feature iirc). We also know that something on 
powerpc doesnt work right out of box with inline being a hint. And 
that's OK and expected - for years we had "inline == always_inline" and 
there was just no force that moved against that. It was in fact 
surprising how well it all worked out on x86 when we reintroduced this 
feature.

btw., another new aspect of this whole area is that a number of major 
distributions now do CONFIG_CC_OPTIMIZE_FOR_SIZE=y in their kernels, and 
that seems to have brought new life into -Os development and with every 
new gcc generation we get denser and denser code generated. It is an 
interesting fact that generating good _small_ code needs a very good 
compiler that understands the code flow very accurately and is able to 
find the smallest possible representation for that.

A side-effect of that is if we do -Os _and_ turn inline into a hint via 
CONFIG_CC_OPTIMIZE_INLINING=y, gcc tends to do a better job than before. 
It cannot just go and inline stuff agressively in stark contrast to the 
-Os goal that was given to it. It still does not do a perfect job though 
- but now at least it has a _chance_ to improve. With always_inline we 
gave gcc no leeway at all.

a second aspect is that there's always the chance that some other 
compiler comes along that does things better than gcc. Again, with 
inline being a necessity we make it impossible for even sane compilers 
to do the right thing.

and a third aspect is that in the current -git kernel we have exactly 
11801 inlines in non-include files. Fighting the constant influx of them 
was a futile act 3 years ago when Arjan and me created this feature for 
the first time and it is futile today. It is a compiler domain problem 
and we should not waste any manpower "fighting" them. Life is too short :)

	Ingo

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Adrian Bunk]

On Sun, Apr 27, 2008 at 09:36:27PM +0200, Ingo Molnar wrote:
>...
> and a third aspect is that in the current -git kernel we have exactly 
> 11801 inlines in non-include files. Fighting the constant influx of them 
> was a futile act 3 years ago when Arjan and me created this feature for 
> the first time and it is futile today. It is a compiler domain problem 
> and we should not waste any manpower "fighting" them. Life is too short :)

It is a kernel bug that we have inline's at the wrong places.

I will "waste" my manpower on fixing these bugs.

It is a well-known fact that much code that gets added to the kernel is 
crap, but that shouldn't be an excuse for not fixing bugs in the kernel.

> 	Ingo

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Adrian Bunk]

On Sun, Apr 27, 2008 at 09:36:27PM +0200, Ingo Molnar wrote:
>...
> and a third aspect is that in the current -git kernel we have exactly 
> 11801 inlines in non-include files. Fighting the constant influx of them 
> was a futile act 3 years ago when Arjan and me created this feature for 
> the first time and it is futile today. It is a compiler domain problem 
> and we should not waste any manpower "fighting" them. Life is too short :)

It is a kernel bug that we have inline's at the wrong places.

I will "waste" my manpower on fixing these bugs.

It is a well-known fact that much code that gets added to the kernel is 
crap, but that shouldn't be an excuse for not fixing bugs in the kernel.

> 	Ingo

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Arjan van de Ven]

On Sun, 27 Apr 2008 11:24:28 -0700 (PDT)
Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:

> 
> 
> On Sun, 27 Apr 2008, Christoph Hellwig wrote:
> > 
> > As Linus mentioned the hint doesn't make any sense because gcc will
> > get it wrong anyway.  In fact when you look at kernel code it tends
> > to inline the everything and the kitchensink as long as there's just
> > one caller and this bloat the stack but doesn't inline where it
> > needs to.  Better don't try to mess with that and do it explicit.
> 
> The thing is, the "inline" vs "always_inline" thing _could_ make
> sense, but sadly doesn't much.
> 
> Part of it is that gcc imnsho inlines too aggressively anyway in the 
> absense of "inline",


yup.. but this patch doesn't change that... the "no inline keyword" case
is totally unaffected...


(actually, other than some obscure commandline options, the only sane way to
avoid gcc doing this too agressive is using -Os)

-- 
If you want to reach me at my work email, use arjan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Arjan van de Ven]

On Sun, 27 Apr 2008 11:24:28 -0700 (PDT)
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> 
> 
> On Sun, 27 Apr 2008, Christoph Hellwig wrote:
> > 
> > As Linus mentioned the hint doesn't make any sense because gcc will
> > get it wrong anyway.  In fact when you look at kernel code it tends
> > to inline the everything and the kitchensink as long as there's just
> > one caller and this bloat the stack but doesn't inline where it
> > needs to.  Better don't try to mess with that and do it explicit.
> 
> The thing is, the "inline" vs "always_inline" thing _could_ make
> sense, but sadly doesn't much.
> 
> Part of it is that gcc imnsho inlines too aggressively anyway in the 
> absense of "inline",


yup.. but this patch doesn't change that... the "no inline keyword" case
is totally unaffected...


(actually, other than some obscure commandline options, the only sane way to
avoid gcc doing this too agressive is using -Os)

-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Linus Torvalds]

On Sun, 27 Apr 2008, Arjan van de Ven wrote:
> 
> (actually, other than some obscure commandline options, the only sane way to
> avoid gcc doing this too agressive is using -Os)

Well, CC_OPTIMIZE_FOR_SIZE has been defaulting to 'y' for a *loong* time, 
but it's hidden behind a EXPERIMENTAL (unless you were on some embedded 
architectures), so many people won't see it.

Perhaps it is time to remove the EXPERIMENTAL? I think the gcc warnings 
were mostly bogus - it's not as if there haven't been compiler bugs 
without -Os too..

		Linus

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Linus Torvalds]

On Sun, 27 Apr 2008, Arjan van de Ven wrote:
> 
> (actually, other than some obscure commandline options, the only sane way to
> avoid gcc doing this too agressive is using -Os)

Well, CC_OPTIMIZE_FOR_SIZE has been defaulting to 'y' for a *loong* time, 
but it's hidden behind a EXPERIMENTAL (unless you were on some embedded 
architectures), so many people won't see it.

Perhaps it is time to remove the EXPERIMENTAL? I think the gcc warnings 
were mostly bogus - it's not as if there haven't been compiler bugs 
without -Os too..

		Linus

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Ingo Molnar]

* Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:

> On Sun, 27 Apr 2008, Arjan van de Ven wrote:
> > 
> > (actually, other than some obscure commandline options, the only 
> > sane way to avoid gcc doing this too agressive is using -Os)
> 
> Well, CC_OPTIMIZE_FOR_SIZE has been defaulting to 'y' for a *loong* 
> time, but it's hidden behind a EXPERIMENTAL (unless you were on some 
> embedded architectures), so many people won't see it.
> 
> Perhaps it is time to remove the EXPERIMENTAL? I think the gcc 
> warnings were mostly bogus - it's not as if there haven't been 
> compiler bugs without -Os too..

i was wondering about that when doing the OPTIMIZE_INLINING change - 
lets do something like the patch below?

	Ingo

-------------->
Subject: make CC_OPTIMIZE_FOR_SIZE non-experimental
From: Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
Date: Mon Apr 28 01:39:43 CEST 2008

this option has been the default on a wide range of distributions
for a long time - time to make it non-experimental.

Signed-off-by: Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
---
 init/Kconfig |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Index: linux-x86.q/init/Kconfig
===================================================================
--- linux-x86.q.orig/init/Kconfig
+++ linux-x86.q/init/Kconfig
@@ -496,16 +496,12 @@ source "usr/Kconfig"
 endif
 
 config CC_OPTIMIZE_FOR_SIZE
-	bool "Optimize for size (Look out for broken compilers!)"
+	bool "Optimize for size"
 	default y
-	depends on ARM || H8300 || SUPERH || EXPERIMENTAL
 	help
 	  Enabling this option will pass "-Os" instead of "-O2" to gcc
 	  resulting in a smaller kernel.
 
-	  WARNING: some versions of gcc may generate incorrect code with this
-	  option.  If problems are observed, a gcc upgrade may be needed.
-
 	  If unsure, say N.
 
 config SYSCTL

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Sun, 27 Apr 2008, Arjan van de Ven wrote:
> > 
> > (actually, other than some obscure commandline options, the only 
> > sane way to avoid gcc doing this too agressive is using -Os)
> 
> Well, CC_OPTIMIZE_FOR_SIZE has been defaulting to 'y' for a *loong* 
> time, but it's hidden behind a EXPERIMENTAL (unless you were on some 
> embedded architectures), so many people won't see it.
> 
> Perhaps it is time to remove the EXPERIMENTAL? I think the gcc 
> warnings were mostly bogus - it's not as if there haven't been 
> compiler bugs without -Os too..

i was wondering about that when doing the OPTIMIZE_INLINING change - 
lets do something like the patch below?

	Ingo

-------------->
Subject: make CC_OPTIMIZE_FOR_SIZE non-experimental
From: Ingo Molnar <mingo@elte.hu>
Date: Mon Apr 28 01:39:43 CEST 2008

this option has been the default on a wide range of distributions
for a long time - time to make it non-experimental.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 init/Kconfig |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Index: linux-x86.q/init/Kconfig
===================================================================
--- linux-x86.q.orig/init/Kconfig
+++ linux-x86.q/init/Kconfig
@@ -496,16 +496,12 @@ source "usr/Kconfig"
 endif
 
 config CC_OPTIMIZE_FOR_SIZE
-	bool "Optimize for size (Look out for broken compilers!)"
+	bool "Optimize for size"
 	default y
-	depends on ARM || H8300 || SUPERH || EXPERIMENTAL
 	help
 	  Enabling this option will pass "-Os" instead of "-O2" to gcc
 	  resulting in a smaller kernel.
 
-	  WARNING: some versions of gcc may generate incorrect code with this
-	  option.  If problems are observed, a gcc upgrade may be needed.
-
 	  If unsure, say N.
 
 config SYSCTL

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Sam Ravnborg]

On Sun, Apr 27, 2008 at 02:14:12PM -0400, Christoph Hellwig wrote:
> On Sun, Apr 27, 2008 at 08:09:57PM +0200, Sam Ravnborg wrote:
> > With the config option we pass the inline hint to gcc (if enabled).
> > So with the config option we have the possibility to pass a _hint_ to
> > gcc about inlining.
> > 
> > Before the config option there were no difference between
> > static int alwyas_inline foo() {}
> > and
> > static int inline foo() {}
> > 
> > With the config option we now have a situation where they actually
> > differ as they should do (assuming gcc > 4.x).
> 
> As Linus mentioned the hint doesn't make any sense because gcc will
> get it wrong anyway.  In fact when you look at kernel code it tends
> to inline the everything and the kitchensink as long as there's just
> one caller and this bloat the stack but doesn't inline where it needs
> to.  Better don't try to mess with that and do it explicit.

The tendency is that gcc inline *more than we whish* - not less.
Which is why we have noinline - to cover the cases where we do not
want stuff inlined.

What the patch in question does is to make a difference
between always_inle and inline.
Previously they were the same. With the patch applied and
with a gcc > 4.0 inline is now a hint.

Did you actually read the patch?

> 
> > So you say that it is safe to assume all places where we really need
> > always_inline are annotedted such - and we do not need a simple
> > config option that the user can uncheck.
> 
> I don't say it is that, it certainly isn't on powerpc and probably most
> other architectures right now, because only x86 got the fixup so far.
> But making it a user-visible option instead of an architecture opt
> in/out selection doesn't make any sense.
If you read the patch you will see that the architectures that
want to enable this has to do an explicit HAVE_CC_INLINE_HINT
so powerpc is not impacted by this until they request it.
Exactly the reason why this was not widely enabled in the
first place (but implemnted in a too x86 specific way).


Updated patch below. I btw. renamed the option to say this is
about a INLINE_HINT.

	Sam

diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 49590f8..9645a6f 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -68,6 +68,7 @@ config SPARC
 	default y
 	select HAVE_IDE
 	select HAVE_OPROFILE
+	select HAVE_CC_INLINE_HINT
 
 # Identify this as a Sparc32 build
 config SPARC32
diff --git a/arch/sparc64/Kconfig b/arch/sparc64/Kconfig
index 8acc5cc..3c8ba36 100644
--- a/arch/sparc64/Kconfig
+++ b/arch/sparc64/Kconfig
@@ -9,6 +9,7 @@ config SPARC
 	bool
 	default y
 	select HAVE_OPROFILE
+	select HAVE_CC_INLINE_HINT
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 2fadf79..48e352b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -20,6 +20,7 @@ config X86
 	def_bool y
 	select HAVE_IDE
 	select HAVE_OPROFILE
+	select HAVE_CC_INLINE_HINT
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 	select HAVE_KVM if ((X86_32 && !X86_VOYAGER && !X86_VISWS && !X86_NUMAQ) || X86_64)
@@ -142,9 +143,6 @@ config AUDIT_ARCH
 config ARCH_SUPPORTS_AOUT
 	def_bool y
 
-config ARCH_SUPPORTS_OPTIMIZED_INLINING
-	def_bool y
-
 # Use the generic interrupt handling code in kernel/irq/:
 config GENERIC_HARDIRQS
 	bool
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 5b1979a..23956ea 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -258,15 +258,3 @@ config CPA_DEBUG
 
 endmenu
 
-config OPTIMIZE_INLINING
-	bool "Allow gcc to uninline functions marked 'inline'"
-	default y
-	help
-	  This option determines if the kernel forces gcc to inline the functions
-	  developers have marked 'inline'. Doing so takes away freedom from gcc to
-	  do what it thinks is best, which is desirable for the gcc 3.x series of
-	  compilers. The gcc 4.x series have a rewritten inlining algorithm and
-	  disabling this option will generate a smaller kernel there. Hopefully
-	  this algorithm is so good that allowing gcc4 to make the decision can
-	  become the default in the future, until then this option is there to
-	  test gcc for this.
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index b2fd754..e49fade 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -31,8 +31,7 @@
 /*
  * Force always-inline if the user requests it so via the .config:
  */
-#if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \
-    !defined(CONFIG_OPTIMIZE_INLINING) && (__GNUC__ >= 4)
+#if !(defined(CONFIG_HAVE_CC_INLINE_HINT) && (__GNUC__ >= 4))
 # define inline		inline		__attribute__((always_inline))
 # define __inline__	__inline__	__attribute__((always_inline))
 # define __inline	__inline	__attribute__((always_inline))
diff --git a/init/Kconfig b/init/Kconfig
index ba3a389..3568647 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -508,6 +508,13 @@ config CC_OPTIMIZE_FOR_SIZE
 
 	  If unsure, say N.
 
+# archs shall "select HAVE_CC_INLINE_HINT"
+# to avoid forcing all function annotated inline
+# to be inline.
+# Note: This is only enforced for gcc > 4.0
+config HAVE_CC_INLINE_HINT
+	def_bool n
+
 config SYSCTL
 	bool
 

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Sam Ravnborg]

On Sun, Apr 27, 2008 at 02:14:12PM -0400, Christoph Hellwig wrote:
> On Sun, Apr 27, 2008 at 08:09:57PM +0200, Sam Ravnborg wrote:
> > With the config option we pass the inline hint to gcc (if enabled).
> > So with the config option we have the possibility to pass a _hint_ to
> > gcc about inlining.
> > 
> > Before the config option there were no difference between
> > static int alwyas_inline foo() {}
> > and
> > static int inline foo() {}
> > 
> > With the config option we now have a situation where they actually
> > differ as they should do (assuming gcc > 4.x).
> 
> As Linus mentioned the hint doesn't make any sense because gcc will
> get it wrong anyway.  In fact when you look at kernel code it tends
> to inline the everything and the kitchensink as long as there's just
> one caller and this bloat the stack but doesn't inline where it needs
> to.  Better don't try to mess with that and do it explicit.

The tendency is that gcc inline *more than we whish* - not less.
Which is why we have noinline - to cover the cases where we do not
want stuff inlined.

What the patch in question does is to make a difference
between always_inle and inline.
Previously they were the same. With the patch applied and
with a gcc > 4.0 inline is now a hint.

Did you actually read the patch?

> 
> > So you say that it is safe to assume all places where we really need
> > always_inline are annotedted such - and we do not need a simple
> > config option that the user can uncheck.
> 
> I don't say it is that, it certainly isn't on powerpc and probably most
> other architectures right now, because only x86 got the fixup so far.
> But making it a user-visible option instead of an architecture opt
> in/out selection doesn't make any sense.
If you read the patch you will see that the architectures that
want to enable this has to do an explicit HAVE_CC_INLINE_HINT
so powerpc is not impacted by this until they request it.
Exactly the reason why this was not widely enabled in the
first place (but implemnted in a too x86 specific way).


Updated patch below. I btw. renamed the option to say this is
about a INLINE_HINT.

	Sam

diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 49590f8..9645a6f 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -68,6 +68,7 @@ config SPARC
 	default y
 	select HAVE_IDE
 	select HAVE_OPROFILE
+	select HAVE_CC_INLINE_HINT
 
 # Identify this as a Sparc32 build
 config SPARC32
diff --git a/arch/sparc64/Kconfig b/arch/sparc64/Kconfig
index 8acc5cc..3c8ba36 100644
--- a/arch/sparc64/Kconfig
+++ b/arch/sparc64/Kconfig
@@ -9,6 +9,7 @@ config SPARC
 	bool
 	default y
 	select HAVE_OPROFILE
+	select HAVE_CC_INLINE_HINT
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 2fadf79..48e352b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -20,6 +20,7 @@ config X86
 	def_bool y
 	select HAVE_IDE
 	select HAVE_OPROFILE
+	select HAVE_CC_INLINE_HINT
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 	select HAVE_KVM if ((X86_32 && !X86_VOYAGER && !X86_VISWS && !X86_NUMAQ) || X86_64)
@@ -142,9 +143,6 @@ config AUDIT_ARCH
 config ARCH_SUPPORTS_AOUT
 	def_bool y
 
-config ARCH_SUPPORTS_OPTIMIZED_INLINING
-	def_bool y
-
 # Use the generic interrupt handling code in kernel/irq/:
 config GENERIC_HARDIRQS
 	bool
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 5b1979a..23956ea 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -258,15 +258,3 @@ config CPA_DEBUG
 
 endmenu
 
-config OPTIMIZE_INLINING
-	bool "Allow gcc to uninline functions marked 'inline'"
-	default y
-	help
-	  This option determines if the kernel forces gcc to inline the functions
-	  developers have marked 'inline'. Doing so takes away freedom from gcc to
-	  do what it thinks is best, which is desirable for the gcc 3.x series of
-	  compilers. The gcc 4.x series have a rewritten inlining algorithm and
-	  disabling this option will generate a smaller kernel there. Hopefully
-	  this algorithm is so good that allowing gcc4 to make the decision can
-	  become the default in the future, until then this option is there to
-	  test gcc for this.
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index b2fd754..e49fade 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -31,8 +31,7 @@
 /*
  * Force always-inline if the user requests it so via the .config:
  */
-#if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \
-    !defined(CONFIG_OPTIMIZE_INLINING) && (__GNUC__ >= 4)
+#if !(defined(CONFIG_HAVE_CC_INLINE_HINT) && (__GNUC__ >= 4))
 # define inline		inline		__attribute__((always_inline))
 # define __inline__	__inline__	__attribute__((always_inline))
 # define __inline	__inline	__attribute__((always_inline))
diff --git a/init/Kconfig b/init/Kconfig
index ba3a389..3568647 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -508,6 +508,13 @@ config CC_OPTIMIZE_FOR_SIZE
 
 	  If unsure, say N.
 
+# archs shall "select HAVE_CC_INLINE_HINT"
+# to avoid forcing all function annotated inline
+# to be inline.
+# Note: This is only enforced for gcc > 4.0
+config HAVE_CC_INLINE_HINT
+	def_bool n
+
 config SYSCTL
 	bool
 

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Christoph Hellwig]

On Sun, Apr 27, 2008 at 08:31:31PM +0200, Sam Ravnborg wrote:
> The tendency is that gcc inline *more than we whish* - not less.
> Which is why we have noinline - to cover the cases where we do not
> want stuff inlined.

Which is exactly the wrong way around.  noinline should be the default.
But that's probaby no fixable without touching gcc or doing ugly hacks
as in xfs.

> What the patch in question does is to make a difference
> between always_inle and inline.
> Previously they were the same. With the patch applied and
> with a gcc > 4.0 inline is now a hint.
> 
> Did you actually read the patch?

Yes.  As as I said I think having them the same, or rather only
having one of them makes more sense than having a hint in addition to
force.

> If you read the patch you will see that the architectures that
> want to enable this has to do an explicit HAVE_CC_INLINE_HINT
> so powerpc is not impacted by this until they request it.
> Exactly the reason why this was not widely enabled in the
> first place (but implemnted in a too x86 specific way).

Yes, and that part of your patch is a good thing, and makes it at
least sensible unlike ingos.  The patch in this mail is the one
we should probably go for short term, although the right way to
sort it out is the long one, aka goign through all users of inline
and deciding what they really want.

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Christoph Hellwig]

On Sun, Apr 27, 2008 at 08:31:31PM +0200, Sam Ravnborg wrote:
> The tendency is that gcc inline *more than we whish* - not less.
> Which is why we have noinline - to cover the cases where we do not
> want stuff inlined.

Which is exactly the wrong way around.  noinline should be the default.
But that's probaby no fixable without touching gcc or doing ugly hacks
as in xfs.

> What the patch in question does is to make a difference
> between always_inle and inline.
> Previously they were the same. With the patch applied and
> with a gcc > 4.0 inline is now a hint.
> 
> Did you actually read the patch?

Yes.  As as I said I think having them the same, or rather only
having one of them makes more sense than having a hint in addition to
force.

> If you read the patch you will see that the architectures that
> want to enable this has to do an explicit HAVE_CC_INLINE_HINT
> so powerpc is not impacted by this until they request it.
> Exactly the reason why this was not widely enabled in the
> first place (but implemnted in a too x86 specific way).

Yes, and that part of your patch is a good thing, and makes it at
least sensible unlike ingos.  The patch in this mail is the one
we should probably go for short term, although the right way to
sort it out is the long one, aka goign through all users of inline
and deciding what they really want.

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Linus Torvalds]

On Sun, 27 Apr 2008, Adrian Bunk wrote:
>
> What I want instead:
> - we continue to force the compiler to always inline with "inline"
> - we remove the inline's in .c files and make too big functions in 
>   headers out-of-line

Sure, I can agree with that as a mostly good goal, but you're still 
ignoring the fact that nobody should really expect the compiler to always 
do a good job at deciding high-level issues.

For example, what's wrong with having "inline" on functions in .c files if 
the author thinks they are small enough? He's likely right. Considering 
past behaviour, he's quite often more right than the compiler.

Just as an example of this: gcc will often inline even big functions, if 
they are called from only one call-site. In fact, ask a compiler guy, and 
he'll likely say that that is obviously a good thing.

But ask somebody who debugs the resulting oops reports, and he may well 
disagree violently.

In other words, inlining is about much more than pure optimization. 

Sometimes it's about forcing it (or not forcing it) for simple correctness 
issues when the compiler doesn't understand that the code in question has 
specific rules (for example, we sometimes want to *force* certain 
functions to be in specific segments).

And sometimes it's about debugging. For the kernel, backtraces posted by 
random users are one of the main debug facilities, and unlike many other 
projects, it's not reasonable to ask people to recompile with "-O0 -g" to 
get better backtraces. The bulk of all reports will come from people who 
use precompiled images from a distribution.

And that means that inlining has a *huge* impact on debuggability.

I have vey often cursed gcc inlining some biggish function - who the f*ck 
cares if a thousand-instruction function can shave a couple of 
instructions of call overhead, when it then causes the call trace to be 
really hard to read?

So quite frankly, my preferred optimization would be:

 - Heavily discourage gcc from inlining functions that aren't marked 
   "inline". I suspect it hurts kernel debugging more than many other 
   projects (because other projects aren't as dependent on the traces)

 - I do agree 100% with you that header file functions should be small 
   (unless they use __builtin_constant_p() or other tricks to guarantee a 
   much smaller static footprint than dynamic one)

 - I also suspect we should have some way for developers to ask fo *hints* 
   from the compiler, ie instead of having gcc inline on its own by 
   default, have the people who care about it ask the compiler to warn 
   about cases where inlining would be a big win.

 - Make "inline" mean "you may want to inline this", and "forced_inline" 
   mean "you *have* to inline this". Ie the "inline" is where the compiler 
   can make a subtle choice (and we need that, because sometimes 
   architecture or config options means that the programmer should not 
   make the choice statically!)

In short, in general I actually wish we'd inline much much less than we 
do. And yes, part of that is that we have way too much code in our header 
files.

		Linus

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Linus Torvalds]

On Sun, 27 Apr 2008, Adrian Bunk wrote:
>
> What I want instead:
> - we continue to force the compiler to always inline with "inline"
> - we remove the inline's in .c files and make too big functions in 
>   headers out-of-line

Sure, I can agree with that as a mostly good goal, but you're still 
ignoring the fact that nobody should really expect the compiler to always 
do a good job at deciding high-level issues.

For example, what's wrong with having "inline" on functions in .c files if 
the author thinks they are small enough? He's likely right. Considering 
past behaviour, he's quite often more right than the compiler.

Just as an example of this: gcc will often inline even big functions, if 
they are called from only one call-site. In fact, ask a compiler guy, and 
he'll likely say that that is obviously a good thing.

But ask somebody who debugs the resulting oops reports, and he may well 
disagree violently.

In other words, inlining is about much more than pure optimization. 

Sometimes it's about forcing it (or not forcing it) for simple correctness 
issues when the compiler doesn't understand that the code in question has 
specific rules (for example, we sometimes want to *force* certain 
functions to be in specific segments).

And sometimes it's about debugging. For the kernel, backtraces posted by 
random users are one of the main debug facilities, and unlike many other 
projects, it's not reasonable to ask people to recompile with "-O0 -g" to 
get better backtraces. The bulk of all reports will come from people who 
use precompiled images from a distribution.

And that means that inlining has a *huge* impact on debuggability.

I have vey often cursed gcc inlining some biggish function - who the f*ck 
cares if a thousand-instruction function can shave a couple of 
instructions of call overhead, when it then causes the call trace to be 
really hard to read?

So quite frankly, my preferred optimization would be:

 - Heavily discourage gcc from inlining functions that aren't marked 
   "inline". I suspect it hurts kernel debugging more than many other 
   projects (because other projects aren't as dependent on the traces)

 - I do agree 100% with you that header file functions should be small 
   (unless they use __builtin_constant_p() or other tricks to guarantee a 
   much smaller static footprint than dynamic one)

 - I also suspect we should have some way for developers to ask fo *hints* 
   from the compiler, ie instead of having gcc inline on its own by 
   default, have the people who care about it ask the compiler to warn 
   about cases where inlining would be a big win.

 - Make "inline" mean "you may want to inline this", and "forced_inline" 
   mean "you *have* to inline this". Ie the "inline" is where the compiler 
   can make a subtle choice (and we need that, because sometimes 
   architecture or config options means that the programmer should not 
   make the choice statically!)

In short, in general I actually wish we'd inline much much less than we 
do. And yes, part of that is that we have way too much code in our header 
files.

		Linus

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Christoph Hellwig]

On Sun, Apr 27, 2008 at 11:11:27AM -0700, Linus Torvalds wrote:
> For example, what's wrong with having "inline" on functions in .c files if 
> the author thinks they are small enough? He's likely right. Considering 
> past behaviour, he's quite often more right than the compiler.

Actually looking at older code in the tree he's most likely wrong :)
Probably as bad as the compiler.  But the nice part about the code is
that we can fix it easily.

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Christoph Hellwig]

On Sun, Apr 27, 2008 at 11:11:27AM -0700, Linus Torvalds wrote:
> For example, what's wrong with having "inline" on functions in .c files if 
> the author thinks they are small enough? He's likely right. Considering 
> past behaviour, he's quite often more right than the compiler.

Actually looking at older code in the tree he's most likely wrong :)
Probably as bad as the compiler.  But the nice part about the code is
that we can fix it easily.

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Linus Torvalds]

On Sun, 27 Apr 2008, Christoph Hellwig wrote:
> 
> Actually looking at older code in the tree he's most likely wrong :)
> Probably as bad as the compiler.  But the nice part about the code is
> that we can fix it easily.

Good point.

It *would* be really interesting to have some way to check our assumptions 
(both ways - warn about over-large inlines and small-and-hot non-inlines). 

I considered making sparse give some size estimate for inlines and warn 
about ones that generate a lot of code, but I was never able to do it 
sanely.

			Linus

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Linus Torvalds]

On Sun, 27 Apr 2008, Christoph Hellwig wrote:
> 
> Actually looking at older code in the tree he's most likely wrong :)
> Probably as bad as the compiler.  But the nice part about the code is
> that we can fix it easily.

Good point.

It *would* be really interesting to have some way to check our assumptions 
(both ways - warn about over-large inlines and small-and-hot non-inlines). 

I considered making sparse give some size estimate for inlines and warn 
about ones that generate a lot of code, but I was never able to do it 
sanely.

			Linus

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Adrian Bunk]

On Sun, Apr 27, 2008 at 11:11:27AM -0700, Linus Torvalds wrote:
> 
> 
> On Sun, 27 Apr 2008, Adrian Bunk wrote:
> >
> > What I want instead:
> > - we continue to force the compiler to always inline with "inline"
> > - we remove the inline's in .c files and make too big functions in 
> >   headers out-of-line
> 
> Sure, I can agree with that as a mostly good goal, but you're still 
> ignoring the fact that nobody should really expect the compiler to always 
> do a good job at deciding high-level issues.
> 
> For example, what's wrong with having "inline" on functions in .c files if 
> the author thinks they are small enough? He's likely right. Considering 
> past behaviour, he's quite often more right than the compiler.
>...

Ingo's commit in your tree just broke this assumption.

And he justified breaking it with a 2.3% size decrease.

And after
- grep'ing for "inline" through drivers/
- remembering how driver authors use likely/unlikely
- remembering the practically non-existing quality criteria you set
  for new drivers [1]
"He's likely right." is likely not right for the average driver author.

You might be smarter than gcc, but most kernel developers (including me) 
are not.

> 		Linus

cu
Adrian

[1] you said you really wanted this driver merged in this state in 2.6.25:

      grep -C4 volatile drivers/infiniband/hw/nes/nes_nic.c


-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Adrian Bunk]

On Sun, Apr 27, 2008 at 11:11:27AM -0700, Linus Torvalds wrote:
> 
> 
> On Sun, 27 Apr 2008, Adrian Bunk wrote:
> >
> > What I want instead:
> > - we continue to force the compiler to always inline with "inline"
> > - we remove the inline's in .c files and make too big functions in 
> >   headers out-of-line
> 
> Sure, I can agree with that as a mostly good goal, but you're still 
> ignoring the fact that nobody should really expect the compiler to always 
> do a good job at deciding high-level issues.
> 
> For example, what's wrong with having "inline" on functions in .c files if 
> the author thinks they are small enough? He's likely right. Considering 
> past behaviour, he's quite often more right than the compiler.
>...

Ingo's commit in your tree just broke this assumption.

And he justified breaking it with a 2.3% size decrease.

And after
- grep'ing for "inline" through drivers/
- remembering how driver authors use likely/unlikely
- remembering the practically non-existing quality criteria you set
  for new drivers [1]
"He's likely right." is likely not right for the average driver author.

You might be smarter than gcc, but most kernel developers (including me) 
are not.

> 		Linus

cu
Adrian

[1] you said you really wanted this driver merged in this state in 2.6.25:

      grep -C4 volatile drivers/infiniband/hw/nes/nes_nic.c


-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Linus Torvalds]

On Sun, 27 Apr 2008, Adrian Bunk wrote:

> On Sun, Apr 27, 2008 at 11:11:27AM -0700, Linus Torvalds wrote:
> > 
> > For example, what's wrong with having "inline" on functions in .c files if 
> > the author thinks they are small enough? He's likely right. Considering 
> > past behaviour, he's quite often more right than the compiler.
> >...
> 
> Ingo's commit in your tree just broke this assumption.

Note that our problem is too much inlining, not too little.

I'm actually happier with gcc not deciding to inline (despite having an 
"inline") than I am with gcc deciding to inline (in violation of _not_ 
having an "inline").

So I don't disagree with Ingo's commit per se.

The only problem with not inlining is a historical one: exactly because 
gcc _used_ to always do what people asked for, Linux has historically 
treated "inline" as a "force_inline". And I was very unhappy when gcc 
changed that, just because it broke historically good code.

In many ways, it might have been better if we had a "__may_inline" thing 
to tell the compiler "you can inline this if you think it's worth it"). 
Both gcc (long ago) and Ingo (now) decided to just make plain "inline" 
mean that, but with a pretty strong bias. It was wrong for gcc to do so, 
imho, and it may have been wrong for this OPTIMIZE_INLINE thing too.

			Linus

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Linus Torvalds]

On Sun, 27 Apr 2008, Adrian Bunk wrote:

> On Sun, Apr 27, 2008 at 11:11:27AM -0700, Linus Torvalds wrote:
> > 
> > For example, what's wrong with having "inline" on functions in .c files if 
> > the author thinks they are small enough? He's likely right. Considering 
> > past behaviour, he's quite often more right than the compiler.
> >...
> 
> Ingo's commit in your tree just broke this assumption.

Note that our problem is too much inlining, not too little.

I'm actually happier with gcc not deciding to inline (despite having an 
"inline") than I am with gcc deciding to inline (in violation of _not_ 
having an "inline").

So I don't disagree with Ingo's commit per se.

The only problem with not inlining is a historical one: exactly because 
gcc _used_ to always do what people asked for, Linux has historically 
treated "inline" as a "force_inline". And I was very unhappy when gcc 
changed that, just because it broke historically good code.

In many ways, it might have been better if we had a "__may_inline" thing 
to tell the compiler "you can inline this if you think it's worth it"). 
Both gcc (long ago) and Ingo (now) decided to just make plain "inline" 
mean that, but with a pretty strong bias. It was wrong for gcc to do so, 
imho, and it may have been wrong for this OPTIMIZE_INLINE thing too.

			Linus

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Mike Galbraith]

On Sun, 2008-04-27 at 20:47 +0300, Adrian Bunk wrote:
> On Sun, Apr 27, 2008 at 10:32:28AM -0700, Linus Torvalds wrote:
> > 
> > 
> > On Sun, 27 Apr 2008, Adrian Bunk wrote:
> > > 
> > > I'm looking at it from a different angle, all code in the kernel should 
> > > follow the following rules [1]:
> > > - no functions in .c files should be marked inline
> > > - all functions in headers should be static inline
> > > - all functions in headers should either be very small or collapse
> > >   to become very small after inlining
> > > 
> > > I can simply not see any usecase for a non-forced inline in the kernel,
> > > and fixing the kernel should give a superset of the space savings of 
> > > this "inline optimization".
> > 
> > Your whole argument is premised on the assumption that the compiler does 
> > the right thing.
> >...
> 
> No, you seem to be misunderstanding what I am saying.
> 
> Status Quo as of 2.6.25:
> - we force the compiler to always inline with "inline"

What is wrong with that?  I believe the term is 'directive'.

> - we have inline's in .c files and too big functions in headers, and
>   both of them are wrong

Yes, correct the source.

> "inline optimization":
> - we leave the compiler the choice whether or not to inline with "inline"

How did it come to pass that we invented such a thing as an optional
directive?

	-Mike

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Mike Galbraith]

On Sun, 2008-04-27 at 20:47 +0300, Adrian Bunk wrote:
> On Sun, Apr 27, 2008 at 10:32:28AM -0700, Linus Torvalds wrote:
> > 
> > 
> > On Sun, 27 Apr 2008, Adrian Bunk wrote:
> > > 
> > > I'm looking at it from a different angle, all code in the kernel should 
> > > follow the following rules [1]:
> > > - no functions in .c files should be marked inline
> > > - all functions in headers should be static inline
> > > - all functions in headers should either be very small or collapse
> > >   to become very small after inlining
> > > 
> > > I can simply not see any usecase for a non-forced inline in the kernel,
> > > and fixing the kernel should give a superset of the space savings of 
> > > this "inline optimization".
> > 
> > Your whole argument is premised on the assumption that the compiler does 
> > the right thing.
> >...
> 
> No, you seem to be misunderstanding what I am saying.
> 
> Status Quo as of 2.6.25:
> - we force the compiler to always inline with "inline"

What is wrong with that?  I believe the term is 'directive'.

> - we have inline's in .c files and too big functions in headers, and
>   both of them are wrong

Yes, correct the source.

> "inline optimization":
> - we leave the compiler the choice whether or not to inline with "inline"

How did it come to pass that we invented such a thing as an optional
directive?

	-Mike

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Christoph Hellwig]

On Sun, Apr 27, 2008 at 10:32:28AM -0700, Linus Torvalds wrote:
> Modern versions of gcc may do the right thing. Note the two very important 
> code-words: "modern" and "may".

It's even worse than that, that may not even get it right.  That's why
we need always_inline to force it when we know we need it.  Now for the
cases where we don't know know that we either need or really really
strongly want it we should just not mark it inline at all.

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Christoph Hellwig]

On Sun, Apr 27, 2008 at 10:32:28AM -0700, Linus Torvalds wrote:
> Modern versions of gcc may do the right thing. Note the two very important 
> code-words: "modern" and "may".

It's even worse than that, that may not even get it right.  That's why
we need always_inline to force it when we know we need it.  Now for the
cases where we don't know know that we either need or really really
strongly want it we should just not mark it inline at all.

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Matthew Wilcox]

On Sun, Apr 27, 2008 at 08:22:35PM +0300, Adrian Bunk wrote:
> I'm looking at it from a different angle, all code in the kernel should 
> follow the following rules [1]:
> - no functions in .c files should be marked inline
> - all functions in headers should be static inline
> - all functions in headers should either be very small or collapse
>   to become very small after inlining
> 
> I can simply not see any usecase for a non-forced inline in the kernel,
> and fixing the kernel should give a superset of the space savings of 
> this "inline optimization".

Here's a good counterexample: kernel/mutex.c.

__mutex_lock_common wants to be inlined into __mutex_lock_*_slowpath.
and *_slowpath *shouldn't* be inlined into mutex_lock_*.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Matthew Wilcox]

On Sun, Apr 27, 2008 at 08:22:35PM +0300, Adrian Bunk wrote:
> I'm looking at it from a different angle, all code in the kernel should 
> follow the following rules [1]:
> - no functions in .c files should be marked inline
> - all functions in headers should be static inline
> - all functions in headers should either be very small or collapse
>   to become very small after inlining
> 
> I can simply not see any usecase for a non-forced inline in the kernel,
> and fixing the kernel should give a superset of the space savings of 
> this "inline optimization".

Here's a good counterexample: kernel/mutex.c.

__mutex_lock_common wants to be inlined into __mutex_lock_*_slowpath.
and *_slowpath *shouldn't* be inlined into mutex_lock_*.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Adrian Bunk]

On Sun, Apr 27, 2008 at 11:40:56AM -0600, Matthew Wilcox wrote:
> On Sun, Apr 27, 2008 at 08:22:35PM +0300, Adrian Bunk wrote:
> > I'm looking at it from a different angle, all code in the kernel should 
> > follow the following rules [1]:
> > - no functions in .c files should be marked inline
> > - all functions in headers should be static inline
> > - all functions in headers should either be very small or collapse
> >   to become very small after inlining
> > 
> > I can simply not see any usecase for a non-forced inline in the kernel,
> > and fixing the kernel should give a superset of the space savings of 
> > this "inline optimization".
> 
> Here's a good counterexample: kernel/mutex.c.
> 
> __mutex_lock_common wants to be inlined into __mutex_lock_*_slowpath.

If we really want to force gcc to emit 6 copies of this not so small 
function then Ingo's commit in Linus' tree has already broken it on x86.

> and *_slowpath *shouldn't* be inlined into mutex_lock_*.

"noinline" is nothing anyone wants to change.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] prepare kconfig inline optimization for all architectures

[Adrian Bunk]

On Sun, Apr 27, 2008 at 11:40:56AM -0600, Matthew Wilcox wrote:
> On Sun, Apr 27, 2008 at 08:22:35PM +0300, Adrian Bunk wrote:
> > I'm looking at it from a different angle, all code in the kernel should 
> > follow the following rules [1]:
> > - no functions in .c files should be marked inline
> > - all functions in headers should be static inline
> > - all functions in headers should either be very small or collapse
> >   to become very small after inlining
> > 
> > I can simply not see any usecase for a non-forced inline in the kernel,
> > and fixing the kernel should give a superset of the space savings of 
> > this "inline optimization".
> 
> Here's a good counterexample: kernel/mutex.c.
> 
> __mutex_lock_common wants to be inlined into __mutex_lock_*_slowpath.

If we really want to force gcc to emit 6 copies of this not so small 
function then Ingo's commit in Linus' tree has already broken it on x86.

> and *_slowpath *shouldn't* be inlined into mutex_lock_*.

"noinline" is nothing anyone wants to change.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed