This patch makes the following functions static instead of global inline:
- acpi_battery_present()
- acpi_battery_units()
Signed-off-by: Adrian Bunk <[email protected]>
---
drivers/acpi/battery.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
9cfc2b0a3162d8e2f23537faefb6937c97513f38 foobar
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index f6215e8..d941c5a 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -116,7 +116,7 @@ struct acpi_battery {
#define to_acpi_battery(x) container_of(x, struct acpi_battery, bat);
-inline int acpi_battery_present(struct acpi_battery *battery)
+static int acpi_battery_present(struct acpi_battery *battery)
{
return battery->device->status.battery_present;
}
@@ -235,7 +235,7 @@ static enum power_supply_property energy_battery_props[] = {
#endif
#ifdef CONFIG_ACPI_PROCFS_POWER
-inline char *acpi_battery_units(struct acpi_battery *battery)
+static char *acpi_battery_units(struct acpi_battery *battery)
{
return (battery->power_unit)?"mA":"mW";
}
May I keep them inline?
Thanks,
Alex.
Adrian Bunk wrote:
> This patch makes the following functions static instead of global inline:
> - acpi_battery_present()
> - acpi_battery_units()
>
> Signed-off-by: Adrian Bunk <[email protected]>
>
> ---
>
> drivers/acpi/battery.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> 9cfc2b0a3162d8e2f23537faefb6937c97513f38 foobar
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index f6215e8..d941c5a 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -116,7 +116,7 @@ struct acpi_battery {
>
> #define to_acpi_battery(x) container_of(x, struct acpi_battery, bat);
>
> -inline int acpi_battery_present(struct acpi_battery *battery)
> +static int acpi_battery_present(struct acpi_battery *battery)
> {
> return battery->device->status.battery_present;
> }
> @@ -235,7 +235,7 @@ static enum power_supply_property energy_battery_props[] = {
> #endif
>
> #ifdef CONFIG_ACPI_PROCFS_POWER
> -inline char *acpi_battery_units(struct acpi_battery *battery)
> +static char *acpi_battery_units(struct acpi_battery *battery)
> {
> return (battery->power_unit)?"mA":"mW";
> }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Sat, Mar 01, 2008 at 09:26:41PM +0300, Alexey Starikovskiy wrote:
> May I keep them inline?
The problem with such manual inlines is that we force gcc to always
inline them - and history has shown that functions grow without the
"inline" being removed.
And for static functions gcc should (at least in theory) know best when
to inline them.
> Thanks,
> Alex.
> Adrian Bunk wrote:
>> This patch makes the following functions static instead of global inline:
>> - acpi_battery_present()
>> - acpi_battery_units()
>>
>> Signed-off-by: Adrian Bunk <[email protected]>
>>
>> ---
>>
>> drivers/acpi/battery.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> 9cfc2b0a3162d8e2f23537faefb6937c97513f38 foobar
>> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
>> index f6215e8..d941c5a 100644
>> --- a/drivers/acpi/battery.c
>> +++ b/drivers/acpi/battery.c
>> @@ -116,7 +116,7 @@ struct acpi_battery {
>> #define to_acpi_battery(x) container_of(x, struct acpi_battery, bat);
>> -inline int acpi_battery_present(struct acpi_battery *battery)
>> +static int acpi_battery_present(struct acpi_battery *battery)
>> {
>> return battery->device->status.battery_present;
>> }
>> @@ -235,7 +235,7 @@ static enum power_supply_property energy_battery_props[] = {
>> #endif
>> #ifdef CONFIG_ACPI_PROCFS_POWER
>> -inline char *acpi_battery_units(struct acpi_battery *battery)
>> +static char *acpi_battery_units(struct acpi_battery *battery)
>> {
>> return (battery->power_unit)?"mA":"mW";
>> }
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
Adrian Bunk wrote:
> On Sat, Mar 01, 2008 at 09:26:41PM +0300, Alexey Starikovskiy wrote:
>
>> May I keep them inline?
>>
>
> The problem with such manual inlines is that we force gcc to always
> inline them - and history has shown that functions grow without the
> "inline" being removed.
>
>
> And for static functions gcc should (at least in theory) know best when
> to inline them.
>
>
Right. Acked-by: Alexey Starikovskiy <[email protected]>
As I understand, you have some automation tools for finding out these
non-static functions, etc; are they available somewhere?
Regards,
Alex.
>> Thanks,
>> Alex.
>> Adrian Bunk wrote:
>>
>>> This patch makes the following functions static instead of global inline:
>>> - acpi_battery_present()
>>> - acpi_battery_units()
>>>
>>> Signed-off-by: Adrian Bunk <[email protected]>
>>>
>>> ---
>>>
>>> drivers/acpi/battery.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> 9cfc2b0a3162d8e2f23537faefb6937c97513f38 foobar
>>> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
>>> index f6215e8..d941c5a 100644
>>> --- a/drivers/acpi/battery.c
>>> +++ b/drivers/acpi/battery.c
>>> @@ -116,7 +116,7 @@ struct acpi_battery {
>>> #define to_acpi_battery(x) container_of(x, struct acpi_battery, bat);
>>> -inline int acpi_battery_present(struct acpi_battery *battery)
>>> +static int acpi_battery_present(struct acpi_battery *battery)
>>> {
>>> return battery->device->status.battery_present;
>>> }
>>> @@ -235,7 +235,7 @@ static enum power_supply_property energy_battery_props[] = {
>>> #endif
>>> #ifdef CONFIG_ACPI_PROCFS_POWER
>>> -inline char *acpi_battery_units(struct acpi_battery *battery)
>>> +static char *acpi_battery_units(struct acpi_battery *battery)
>>> {
>>> return (battery->power_unit)?"mA":"mW";
>>> }
>>>
>
> cu
> Adrian
>
>
On Sat, Mar 01, 2008 at 09:42:59PM +0300, Alexey Starikovskiy wrote:
> Adrian Bunk wrote:
>> On Sat, Mar 01, 2008 at 09:26:41PM +0300, Alexey Starikovskiy wrote:
>>
>>> May I keep them inline?
>>>
>>
>> The problem with such manual inlines is that we force gcc to always
>> inline them - and history has shown that functions grow without the
>> "inline" being removed.
>>
>> And for static functions gcc should (at least in theory) know best
>> when to inline them.
>>
>>
> Right. Acked-by: Alexey Starikovskiy <[email protected]>
>
> As I understand, you have some automation tools for finding out these
> non-static functions, etc; are they available somewhere?
Type "make namespacecheck" after compiling the kernel.
> Regards,
> Alex.
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
* Adrian Bunk <[email protected]> wrote:
> On Sat, Mar 01, 2008 at 09:26:41PM +0300, Alexey Starikovskiy wrote:
> > May I keep them inline?
>
> The problem with such manual inlines is that we force gcc to always
> inline them - and history has shown that functions grow without the
> "inline" being removed.
what do you mean by "we force gcc to always inline them"? gcc is free to
decide whether to inline or to not inline. (and CONFIG_FORCED_INLINING
got removed from 2.6.25)
Ingo
On Mon, Mar 03, 2008 at 09:57:20AM +0100, Ingo Molnar wrote:
>
> * Adrian Bunk <[email protected]> wrote:
>
> > On Sat, Mar 01, 2008 at 09:26:41PM +0300, Alexey Starikovskiy wrote:
> > > May I keep them inline?
> >
> > The problem with such manual inlines is that we force gcc to always
> > inline them - and history has shown that functions grow without the
> > "inline" being removed.
>
> what do you mean by "we force gcc to always inline them"?
#define inline inline __attribute__((always_inline))
> gcc is free to decide whether to inline or to not inline.
Not with __attribute__((always_inline)).
> (and CONFIG_FORCED_INLINING got removed from 2.6.25)
CONFIG_FORCED_INLINING never had any effect.
> 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
* Adrian Bunk <[email protected]> wrote:
> On Mon, Mar 03, 2008 at 09:57:20AM +0100, Ingo Molnar wrote:
> >
> > * Adrian Bunk <[email protected]> wrote:
> >
> > > On Sat, Mar 01, 2008 at 09:26:41PM +0300, Alexey Starikovskiy wrote:
> > > > May I keep them inline?
> > >
> > > The problem with such manual inlines is that we force gcc to always
> > > inline them - and history has shown that functions grow without the
> > > "inline" being removed.
> >
> > what do you mean by "we force gcc to always inline them"?
>
> #define inline inline __attribute__((always_inline))
>
> > gcc is free to decide whether to inline or to not inline.
>
> Not with __attribute__((always_inline)).
but that wasnt used in the code you patched:
-inline int acpi_battery_present(struct acpi_battery *battery)
+static int acpi_battery_present(struct acpi_battery *battery)
> > (and CONFIG_FORCED_INLINING got removed from 2.6.25)
>
> CONFIG_FORCED_INLINING never had any effect.
my experience was that it had effects. Why do you say it 'never had any
effect'?
Ingo
On Mon, Mar 03, 2008 at 10:17:14AM +0100, Ingo Molnar wrote:
>
> * Adrian Bunk <[email protected]> wrote:
>
> > On Mon, Mar 03, 2008 at 09:57:20AM +0100, Ingo Molnar wrote:
> > >
> > > * Adrian Bunk <[email protected]> wrote:
> > >
> > > > On Sat, Mar 01, 2008 at 09:26:41PM +0300, Alexey Starikovskiy wrote:
> > > > > May I keep them inline?
> > > >
> > > > The problem with such manual inlines is that we force gcc to always
> > > > inline them - and history has shown that functions grow without the
> > > > "inline" being removed.
> > >
> > > what do you mean by "we force gcc to always inline them"?
> >
> > #define inline inline __attribute__((always_inline))
> >
> > > gcc is free to decide whether to inline or to not inline.
> >
> > Not with __attribute__((always_inline)).
>
> but that wasnt used in the code you patched:
>
> -inline int acpi_battery_present(struct acpi_battery *battery)
> +static int acpi_battery_present(struct acpi_battery *battery)
>From compiler-gcc.h:
#define inline inline __attribute__((always_inline))
So unless I am missing something obvious then each time we
say inline to a function we require gcc to inline the function.
It is my impression that today we only say inline if really needed
and otherwise let gcc decide. So in almost all cases inlise should
just be nuked?
Sam
On Mon, Mar 03, 2008 at 10:17:14AM +0100, Ingo Molnar wrote:
>
> * Adrian Bunk <[email protected]> wrote:
>
> > On Mon, Mar 03, 2008 at 09:57:20AM +0100, Ingo Molnar wrote:
> > >
> > > * Adrian Bunk <[email protected]> wrote:
> > >
> > > > On Sat, Mar 01, 2008 at 09:26:41PM +0300, Alexey Starikovskiy wrote:
> > > > > May I keep them inline?
> > > >
> > > > The problem with such manual inlines is that we force gcc to always
> > > > inline them - and history has shown that functions grow without the
> > > > "inline" being removed.
> > >
> > > what do you mean by "we force gcc to always inline them"?
> >
> > #define inline inline __attribute__((always_inline))
> >
> > > gcc is free to decide whether to inline or to not inline.
> >
> > Not with __attribute__((always_inline)).
>
> but that wasnt used in the code you patched:
>
> -inline int acpi_battery_present(struct acpi_battery *battery)
> +static int acpi_battery_present(struct acpi_battery *battery)
It was used, since the #define affects all inline's in the kernel.
> > > (and CONFIG_FORCED_INLINING got removed from 2.6.25)
> >
> > CONFIG_FORCED_INLINING never had any effect.
>
> my experience was that it had effects. Why do you say it 'never had any
> effect'?
I don't see how it could have possibly had any effect.
Which effects did you experience?
> 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
On Mon, Mar 03, 2008 at 10:31:03AM +0100, Sam Ravnborg wrote:
> On Mon, Mar 03, 2008 at 10:17:14AM +0100, Ingo Molnar wrote:
> >
> > * Adrian Bunk <[email protected]> wrote:
> >
> > > On Mon, Mar 03, 2008 at 09:57:20AM +0100, Ingo Molnar wrote:
> > > >
> > > > * Adrian Bunk <[email protected]> wrote:
> > > >
> > > > > On Sat, Mar 01, 2008 at 09:26:41PM +0300, Alexey Starikovskiy wrote:
> > > > > > May I keep them inline?
> > > > >
> > > > > The problem with such manual inlines is that we force gcc to always
> > > > > inline them - and history has shown that functions grow without the
> > > > > "inline" being removed.
> > > >
> > > > what do you mean by "we force gcc to always inline them"?
> > >
> > > #define inline inline __attribute__((always_inline))
> > >
> > > > gcc is free to decide whether to inline or to not inline.
> > >
> > > Not with __attribute__((always_inline)).
> >
> > but that wasnt used in the code you patched:
> >
> > -inline int acpi_battery_present(struct acpi_battery *battery)
> > +static int acpi_battery_present(struct acpi_battery *battery)
>
> >From compiler-gcc.h:
>
> #define inline inline __attribute__((always_inline))
>
> So unless I am missing something obvious then each time we
> say inline to a function we require gcc to inline the function.
Exactly.
> It is my impression that today we only say inline if really needed
> and otherwise let gcc decide. So in almost all cases inlise should
> just be nuked?
The rule is:
- all static functions in headers should be marked inline
- no functions in .c files should be marked inline
For the latter there are a _few_ exceptions in hotpaths where doing so
brings measurable advantages.
But generally (and especially long-term) it's best to let gcc decide
which static functions in .c files should be inlined.
> 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
* Sam Ravnborg <[email protected]> wrote:
> >From compiler-gcc.h:
> >
> > #define inline inline __attribute__((always_inline))
>
> So unless I am missing something obvious then each time we say inline
> to a function we require gcc to inline the function.
>
> It is my impression that today we only say inline if really needed and
> otherwise let gcc decide. So in almost all cases inlise should just be
> nuked?
no, what we should nuke is this always_inline definition. That was
always the intention of FORCED_INLINE, and the removal of FORCED_INLINE
was to _remove the forcing_, not to make it unconditional.
so Adrian, if you knew about this bug all along, you might as well have
reported it :-/
Ingo
On Mon, Mar 03, 2008 at 11:39:33AM +0100, Ingo Molnar wrote:
>
> * Sam Ravnborg <[email protected]> wrote:
>
> > >From compiler-gcc.h:
> > >
> > > #define inline inline __attribute__((always_inline))
> >
> > So unless I am missing something obvious then each time we say inline
> > to a function we require gcc to inline the function.
> >
> > It is my impression that today we only say inline if really needed and
> > otherwise let gcc decide. So in almost all cases inlise should just be
> > nuked?
>
> no, what we should nuke is this always_inline definition. That was
> always the intention of FORCED_INLINE, and the removal of FORCED_INLINE
> was to _remove the forcing_, not to make it unconditional.
It was always unconditional, and neither adding, toggling nor removing
of CONFIG_FORCED_INLINING changed this invariant.
And what we should do is to attack the excessive wrong usage of inlines
in .c files, not messing with a global #define in a way that the results
on 24 architectures with 7 different releases of gcc would be unpredictable.
> so Adrian, if you knew about this bug all along, you might as well have
> reported it :-/
http://lkml.org/lkml/2007/1/19/36
http://lkml.org/lkml/2007/4/9/363
are the result of a quick Google search of me stating this previously on
linux-kernel. It might have been more often, but I'm too lame too
search further.
> 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
* Adrian Bunk <[email protected]> wrote:
> are the result of a quick Google search of me stating this previously on
> linux-kernel. It might have been more often, but I'm too lame too
> search further.
>
> http://lkml.org/lkml/2007/1/19/36
that's a side-note, not a bugreport and not a patch to fix it.
> http://lkml.org/lkml/2007/4/9/363
this second one is this very thread that i replied to.
> > no, what we should nuke is this always_inline definition. That was
> > always the intention of FORCED_INLINE, and the removal of
> > FORCED_INLINE was to _remove the forcing_, not to make it
> > unconditional.
>
> It was always unconditional, and neither adding, toggling nor removing
> of CONFIG_FORCED_INLINING changed this invariant.
>
> And what we should do is to attack the excessive wrong usage of
> inlines in .c files, not messing with a global #define in a way that
> the results on 24 architectures with 7 different releases of gcc would
> be unpredictable.
i see, so you never properly reported and fixed it because you prefer a
1000 small crappy changes over one change. You could have significantly
contributed to truly making Linux smaller, but you decided not to do it.
and i disagree with your notion that flipping it around is risky in any
unacceptable or unmanageable way. It has far less risks on the compiler
than say CONFIG_CC_OPTIMIZE_FOR_SIZE. It has far less risks than
changing to a new compiler version. Why you think it's "unpredictable"
is a mystery to me.
It almost seems to me you were happy with having that bug in the kernel?
Please tell me that i'm wrong about that impression!
i'll reinstate this .config option and let it do the right thing. Forced
inlining was supposed to be _phased out_, not phased in.
Ingo
On Mon, Mar 03, 2008 at 12:45:34PM +0100, Ingo Molnar wrote:
>
> * Adrian Bunk <[email protected]> wrote:
>
> > are the result of a quick Google search of me stating this previously on
> > linux-kernel. It might have been more often, but I'm too lame too
> > search further.
> >
> > http://lkml.org/lkml/2007/1/19/36
>
> that's a side-note, not a bugreport and not a patch to fix it.
>
> > http://lkml.org/lkml/2007/4/9/363
>
> this second one is this very thread that i replied to.
>...
That's a lie.
> 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
* Adrian Bunk <[email protected]> wrote:
> > > http://lkml.org/lkml/2007/4/9/363
> >
> > this second one is this very thread that i replied to.
> >...
>
> That's a lie.
oops, i was simply wrong - it looked very much like this current thread
...
(It would be a lie had i been writing that intentinally. Thanks for
giving me the benefit of the doubt.)
Ingo
* Ingo Molnar <[email protected]> wrote:
> > > no, what we should nuke is this always_inline definition. That was
> > > always the intention of FORCED_INLINE, and the removal of
> > > FORCED_INLINE was to _remove the forcing_, not to make it
> > > unconditional.
> >
> > It was always unconditional, and neither adding, toggling nor
> > removing of CONFIG_FORCED_INLINING changed this invariant.
> >
> > And what we should do is to attack the excessive wrong usage of
> > inlines in .c files, not messing with a global #define in a way that
> > the results on 24 architectures with 7 different releases of gcc
> > would be unpredictable.
>
> i see, so you never properly reported and fixed it because you prefer
> a 1000 small crappy changes over one change. You could have
> significantly contributed to truly making Linux smaller, but you
> decided not to do it.
>
> and i disagree with your notion that flipping it around is risky in
> any unacceptable or unmanageable way. It has far less risks on the
> compiler than say CONFIG_CC_OPTIMIZE_FOR_SIZE. It has far less risks
> than changing to a new compiler version. Why you think it's
> "unpredictable" is a mystery to me.
>
> It almost seems to me you were happy with having that bug in the
> kernel? Please tell me that i'm wrong about that impression!
>
> i'll reinstate this .config option and let it do the right thing.
> Forced inlining was supposed to be _phased out_, not phased in.
i just implemented the trivial fix below and it gave me a massive, 2.3%
text size reduction (!) on a typical .config. That's more than 120K
shaved off the vmlinux.
Why, despite being aware of this bug, you never fixed this properly is a
mystery to me - this is more .text size savings than you have done so
far with all your uninlining patches of the past few years combined, and
it's probably more than you could have achieved in the next 5 years.
Ingo
---------------------->
Subject: x86: phase out forced inlining
From: Ingo Molnar <[email protected]>
Date: Mon Mar 03 12:38:52 CET 2008
allow gcc to optimize the kernel image's size by uninlining
functions that have been marked 'inline'. Previously gcc was
forced by Linux to always-inline these functions via a gcc
attribute:
#define inline inline __attribute__((always_inline))
Especially when the user has already selected
CONFIG_OPTIMIZE_FOR_SIZE=y this can make a huge difference in
kernel image size (using a standard Fedora .config):
text data bss dec hex filename
5613924 562708 3854336 10030968 990f78 vmlinux.before
5486689 562708 3854336 9903733 971e75 vmlinux.after
that's a 2.3% text size reduction (!).
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/Kconfig.debug | 13 +++++++++++++
arch/x86/configs/i386_defconfig | 1 +
arch/x86/configs/x86_64_defconfig | 1 +
include/linux/compiler-gcc.h | 12 +++++++++---
4 files changed, 24 insertions(+), 3 deletions(-)
Index: linux-x86.q/arch/x86/Kconfig.debug
===================================================================
--- linux-x86.q.orig/arch/x86/Kconfig.debug
+++ linux-x86.q/arch/x86/Kconfig.debug
@@ -336,3 +336,16 @@ config CPA_DEBUG
Do change_page_attr() self-tests every 30 seconds.
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-x86.q/arch/x86/configs/i386_defconfig
===================================================================
--- linux-x86.q.orig/arch/x86/configs/i386_defconfig
+++ linux-x86.q/arch/x86/configs/i386_defconfig
@@ -1421,6 +1421,7 @@ CONFIG_DEBUG_BUGVERBOSE=y
# CONFIG_DEBUG_VM is not set
# CONFIG_DEBUG_LIST is not set
# CONFIG_FRAME_POINTER is not set
+CONFIG_OPTIMIZE_INLINING=y
# CONFIG_RCU_TORTURE_TEST is not set
# CONFIG_LKDTM is not set
# CONFIG_FAULT_INJECTION is not set
Index: linux-x86.q/arch/x86/configs/x86_64_defconfig
===================================================================
--- linux-x86.q.orig/arch/x86/configs/x86_64_defconfig
+++ linux-x86.q/arch/x86/configs/x86_64_defconfig
@@ -1346,6 +1346,7 @@ CONFIG_DEBUG_BUGVERBOSE=y
# CONFIG_DEBUG_VM is not set
# CONFIG_DEBUG_LIST is not set
# CONFIG_FRAME_POINTER is not set
+CONFIG_OPTIMIZE_INLINING=y
# CONFIG_RCU_TORTURE_TEST is not set
# CONFIG_LKDTM is not set
# CONFIG_FAULT_INJECTION is not set
Index: linux-x86.q/include/linux/compiler-gcc.h
===================================================================
--- linux-x86.q.orig/include/linux/compiler-gcc.h
+++ linux-x86.q/include/linux/compiler-gcc.h
@@ -28,9 +28,15 @@
#define __must_be_array(a) \
BUILD_BUG_ON_ZERO(__builtin_types_compatible_p(typeof(a), typeof(&a[0])))
-#define inline inline __attribute__((always_inline))
-#define __inline__ __inline__ __attribute__((always_inline))
-#define __inline __inline __attribute__((always_inline))
+/*
+ * Force always-inline if the user requests it so via the .config:
+ */
+#ifndef CONFIG_OPTIMIZE_INLINING
+# define inline inline __attribute__((always_inline))
+# define __inline__ __inline__ __attribute__((always_inline))
+# define __inline __inline __attribute__((always_inline))
+#endif
+
#define __deprecated __attribute__((deprecated))
#define __packed __attribute__((packed))
#define __weak __attribute__((weak))
On Mon, Mar 03, 2008 at 01:10:15PM +0100, Ingo Molnar wrote:
>
> * Adrian Bunk <[email protected]> wrote:
>
> > > > http://lkml.org/lkml/2007/4/9/363
> > >
> > > this second one is this very thread that i replied to.
> > >...
> >
> > That's a lie.
>
> oops, i was simply wrong - it looked very much like this current thread
> ...
>
> (It would be a lie had i been writing that intentinally. Thanks for
> giving me the benefit of the doubt.)
Thanks for assuming only the best intentions from me.
I can only repeat that I did state several times on linux-kernel that it
never worked.
If you consider it my fault that noone reads my emails then you are
right that it's my fault...
> 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
* Adrian Bunk <[email protected]> wrote:
> I can only repeat that I did state several times on linux-kernel that
> it never worked.
>
> If you consider it my fault that noone reads my emails then you are
> right that it's my fault...
well, i'm trying to assume the best, so please explain the following
sequence of events to me:
1) as you said you knew about this bug - which bug causes more inlining
overhead than hundreds of your uninlining patches combined. The bug
was introduced ~2 years ago in -mm - before the feature hit mainline
in v2.6.16.
2) the fix was really trivial and the intention of the feature was well
understood - but the feature stayed as a NOP in the upstream kernel
for 2 years.
still, while you clearly had interest in this general area of the kernel
(for example you wrote hundreds of tiny uninlining patches that work
towards a similar goal), but strangely at the same time you neither
fixed, nor properly escallated this _far_ bigger bug that causes +2.3%
of text bloat on x86 [more than 120K of kernel text]. In fact:
- you created bugzillas for far smaller bugs in the past, but you never
created a bugzilla for this that i'm aware of.
- you never directly raised this issue with us: "look guys, this thing
really is broken - please reply to me with a fix".
- you never said "this is a regression that should be fixed" to any of
the regression lists.
in other words: for about two years you knew about a bug that should
have been fixed the day after it got introduced.
i obviously cannot know what your intentions were with this conduct, so
i'm eagerly awaiting your explanation for it. Thanks,
Ingo
On Mon, Mar 03, 2008 at 01:50:27PM +0100, Ingo Molnar wrote:
>
> * Adrian Bunk <[email protected]> wrote:
>
> > I can only repeat that I did state several times on linux-kernel that
> > it never worked.
> >
> > If you consider it my fault that noone reads my emails then you are
> > right that it's my fault...
>
> well, i'm trying to assume the best, so please explain the following
> sequence of events to me:
>
> 1) as you said you knew about this bug - which bug causes more inlining
> overhead than hundreds of your uninlining patches combined. The bug
> was introduced ~2 years ago in -mm - before the feature hit mainline
> in v2.6.16.
I don't remember having ever said this.
Your choices are:
[ ] prove your accusation that I said I
"knew about this bug before the feature hit mainline"
[ ] apologize
[ ] be the firest person ever in my killfile
>...
> still, while you clearly had interest in this general area of the kernel
> (for example you wrote hundreds of tiny uninlining patches that work
> towards a similar goal),
I'm not sure with whom you confuse me on this one.
Perhaps with Ilpo?
I have sending some bigger uninlining patches on my TODO list for quite
some time (since this is really the right thing to solve these issues),
but I've not yet gotten there.
> but strangely at the same time you neither
> fixed, nor properly escallated this _far_ bigger bug that causes +2.3%
> of text bloat on x86 [more than 120K of kernel text]. In fact:
>
> - you created bugzillas for far smaller bugs in the past, but you never
> created a bugzilla for this that i'm aware of.
I'm well aware of the fact that our Bugzilla is mostly a /dev/null for
the currently at about 1500 open bugs there.
That's why I tend [1] to only open bugs there for getting them into
Rafael's regression lists.
>...
> - you never said "this is a regression that should be fixed" to any of
> the regression lists.
This isn't a regression.
>...
> Ingo
cu
Adrian
[1] there are a few exceptions when I tried opening some bugs there
--
"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
Hi Ingo.
> Subject: x86: phase out forced inlining
Any particular reason you made the patch x86 specific?
It is preferred to gain this size decrease for other arch too
and having a single definiton of OPTIMIZE_FOR_SIZE
would help here.
We have this for other gcc options already.
> +config OPTIMIZE_INLINING
Other (not all) config options that deal with gcc behaviour are
named CC_*. But they mostly impact gcc options.
CC_OPTIMIZE_INLINING would match the naming of CC_OPTIMIZE_SIZE,
except in the latter OPTIMIZE refer to the -O option.
CC_DEFAULT_INLINE may give the right associations?
> + 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.
Would it be worth here to mention that stuff that really
needs inlining should use __always_inle and not inline?
> + */
> +#ifndef CONFIG_OPTIMIZE_INLINING
> +# define inline inline __attribute__((always_inline))
> +# define __inline__ __inline__ __attribute__((always_inline))
> +# define __inline __inline __attribute__((always_inline))
> +#endif
A quick google did not tell me the difference between inline, __inline, __inline__.
But it turned out the december 2005 thread where there was a lenghty discussion about
trusting gcc with respect to inlining.
It is not the subject of this patch but I just wondered why we need all these variants.
Sam
On Mon, 3 Mar 2008 13:13:35 +0100
Ingo Molnar <[email protected]> wrote:
>
> * Ingo Molnar <[email protected]> wrote:
>
> > > > no, what we should nuke is this always_inline definition. That
> > > > was always the intention of FORCED_INLINE, and the removal of
> > > > FORCED_INLINE was to _remove the forcing_, not to make it
> > > > unconditional.
> > >
> > > It was always unconditional, and neither adding, toggling nor
> > > removing of CONFIG_FORCED_INLINING changed this invariant.
> > >
> > > And what we should do is to attack the excessive wrong usage of
> > > inlines in .c files, not messing with a global #define in a way
> > > that the results on 24 architectures with 7 different releases of
> > > gcc would be unpredictable.
> >
> > i see, so you never properly reported and fixed it because you
> > prefer a 1000 small crappy changes over one change. You could have
> > significantly contributed to truly making Linux smaller, but you
> > decided not to do it.
> >
> > and i disagree with your notion that flipping it around is risky in
> > any unacceptable or unmanageable way. It has far less risks on the
> > compiler than say CONFIG_CC_OPTIMIZE_FOR_SIZE. It has far less
> > risks than changing to a new compiler version. Why you think it's
> > "unpredictable" is a mystery to me.
> >
> > It almost seems to me you were happy with having that bug in the
> > kernel? Please tell me that i'm wrong about that impression!
> >
> > i'll reinstate this .config option and let it do the right thing.
> > Forced inlining was supposed to be _phased out_, not phased in.
>
> i just implemented the trivial fix below and it gave me a massive,
> 2.3% text size reduction (!) on a typical .config. That's more than
> 120K shaved off the vmlinux.
>
> Why, despite being aware of this bug, you never fixed this properly
> is a mystery to me - this is more .text size savings than you have
> done so far with all your uninlining patches of the past few years
> combined, and it's probably more than you could have achieved in the
> next 5 years.
>
> Ingo
>
> ---------------------->
> Subject: x86: phase out forced inlining
> From: Ingo Molnar <[email protected]>
> Date: Mon Mar 03 12:38:52 CET 2008
>
> allow gcc to optimize the kernel image's size by uninlining
> functions that have been marked 'inline'. Previously gcc was
> forced by Linux to always-inline these functions via a gcc
> attribute:
>
> #define inline inline __attribute__((always_inline))
>
> Especially when the user has already selected
> CONFIG_OPTIMIZE_FOR_SIZE=y this can make a huge difference in
> kernel image size (using a standard Fedora .config):
>
> text data bss dec hex filename
> 5613924 562708 3854336 10030968 990f78 vmlinux.before
> 5486689 562708 3854336 9903733 971e75 vmlinux.after
>
> that's a 2.3% text size reduction (!).
>
> Signed-off-by: Ingo Molnar <[email protected]>
> ---
> arch/x86/Kconfig.debug | 13 +++++++++++++
> arch/x86/configs/i386_defconfig | 1 +
> arch/x86/configs/x86_64_defconfig | 1 +
> include/linux/compiler-gcc.h | 12 +++++++++---
> 4 files changed, 24 insertions(+), 3 deletions(-)
>
> Index: linux-x86.q/arch/x86/Kconfig.debug
> ===================================================================
> --- linux-x86.q.orig/arch/x86/Kconfig.debug
eh.. we ALREADY HAD THIS.
someone seems to have removed this wronly; not forcing inline should have been
the default after the removal (in 2.6.25-rc)!
So lets fix it that way, rather than putting the config option back under a different name.
On Mon, Mar 03, 2008 at 04:54:13PM +0200, Adrian Bunk wrote:
> On Mon, Mar 03, 2008 at 01:50:27PM +0100, Ingo Molnar wrote:
> >
> > * Adrian Bunk <[email protected]> wrote:
> >
> > > I can only repeat that I did state several times on linux-kernel that
> > > it never worked.
> > >
> > > If you consider it my fault that noone reads my emails then you are
> > > right that it's my fault...
> >
> > well, i'm trying to assume the best, so please explain the following
> > sequence of events to me:
> >
> > 1) as you said you knew about this bug - which bug causes more inlining
> > overhead than hundreds of your uninlining patches combined. The bug
> > was introduced ~2 years ago in -mm - before the feature hit mainline
> > in v2.6.16.
>
> I don't remember having ever said this.
>
> Your choices are:
> [ ] prove your accusation that I said I
> "knew about this bug before the feature hit mainline"
> [ ] apologize
> [ ] be the firest person ever in my killfile
>...
s/firest/first/
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
> eh.. we ALREADY HAD THIS.
> someone seems to have removed this wronly; not forcing inline should have been
> the default after the removal (in 2.6.25-rc)!
>
> So lets fix it that way, rather than putting the config option back under a different name.
The removal was my patch. I did notice that the config option had no
effect, I figured that was why it was on the removal schedule, as
it had been made the default.
My bad,
Harvey
On Mon, 3 Mar 2008 13:13:35 +0100 Ingo Molnar <[email protected]> wrote:
> +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.
urgh. This will cause whatever problem
4507a6a59cfc6997e532cd812a8bd244181e6205 fixed five years ago to resurface
for incautious gcc-3.x users.
I'd suggest that this
> +#ifndef CONFIG_OPTIMIZE_INLINING
become something along the lines of
> +#ifndef CONFIG_OPTIMIZE_INLINING && (__GNUC__ > 3)
It would be nice to be able to feed the gcc version into the Kconfig logic,
really..
* Andrew Morton <[email protected]> wrote:
> urgh. This will cause whatever problem
> 4507a6a59cfc6997e532cd812a8bd244181e6205 fixed five years ago to
> resurface for incautious gcc-3.x users.
hm, commit 4507a6a59cfc6997e532cd812a8bd244181e6205 does not exist:
fatal: bad object 4507a6a59cfc6997e532cd812a8bd244181e6205
but i suspect it must be something along the lines of the known problem
of really old gcc versions creating huge stackframes? Those pristine gcc
versions were practically unusable for distro kernels anyway (and were
patched by distros) - but i have no problem with restricting this
feature to gcc4x. gcc4x creates more compact -Os code too, so it's
recommended for smaller image sizes.
> I'd suggest that this
>
> > +#ifndef CONFIG_OPTIMIZE_INLINING
>
> become something along the lines of
>
> > +#ifndef CONFIG_OPTIMIZE_INLINING && (__GNUC__ > 3)
good point, fixed that.
> It would be nice to be able to feed the gcc version into the Kconfig
> logic, really..
yeah, instead of littering our include files with those quirks.
Ingo
On Tue, 4 Mar 2008 08:32:48 +0100 Ingo Molnar <[email protected]> wrote:
>
> * Andrew Morton <[email protected]> wrote:
>
> > urgh. This will cause whatever problem
> > 4507a6a59cfc6997e532cd812a8bd244181e6205 fixed five years ago to
> > resurface for incautious gcc-3.x users.
>
> hm, commit 4507a6a59cfc6997e532cd812a8bd244181e6205 does not exist:
>
> fatal: bad object 4507a6a59cfc6997e532cd812a8bd244181e6205
This was 2.5.x - you'll need to look in the historical-git tree.
Here it is:
: commit 4507a6a59cfc6997e532cd812a8bd244181e6205
: Author: akpm <akpm>
: Date: Tue Mar 11 07:42:00 2003 +0000
:
: [PATCH] work around gcc-3.x inlining bugs
:
: Force inlining even when gcc-3.x is too confused to do it for us.
:
: BKrev: 3e6d9348GA9aKzeN-bjzQzMMt85t8g
:
: diff --git a/include/linux/compiler.h b/include/linux/compiler.h
: index e92f472..a28d0d5 100644
: --- a/include/linux/compiler.h
: +++ b/include/linux/compiler.h
: @@ -1,6 +1,12 @@
: #ifndef __LINUX_COMPILER_H
: #define __LINUX_COMPILER_H
:
: +#if (__GNUC__ > 3) || (__GNUC__ == 3 && __GNUC_MINOR__ >= 1)
: +#define inline __inline__ __attribute__((always_inline))
: +#define __inline__ __inline__ __attribute__((always_inline))
: +#define __inline __inline__ __attribute__((always_inline))
: +#endif
: +
: /* Somewhere in the middle of the GCC 2.96 development cycle, we implemented
: a mechanism by which the user can annotate likely branch directions and
: expect the blocks to be reordered appropriately. Define __builtin_expect
:
I was very bad about changelogging that one. I do remember there was a bit
of to-and-fro before we decided to do it this way. Some googling would be
needed.
> but i suspect it must be something along the lines of the known problem
> of really old gcc versions creating huge stackframes?
iirc gcc was failing to inline functions which we'd marked `inline' and it
was generating poorer code as a result. It might also have been generating
an out-of-line copy for each compilation unit which called the inline (it
would have to do this?)
> Those pristine gcc
> versions were practically unusable for distro kernels anyway (and were
> patched by distros) - but i have no problem with restricting this
> feature to gcc4x. gcc4x creates more compact -Os code too, so it's
> recommended for smaller image sizes.
yup.
On Mon, Mar 03, 2008 at 10:42:31PM -0800, Andrew Morton wrote:
> On Mon, 3 Mar 2008 13:13:35 +0100 Ingo Molnar <[email protected]> wrote:
>
> > +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.
>
> urgh. This will cause whatever problem
> 4507a6a59cfc6997e532cd812a8bd244181e6205 fixed five years ago to resurface
> for incautious gcc-3.x users.
>
> I'd suggest that this
>
> > +#ifndef CONFIG_OPTIMIZE_INLINING
>
> become something along the lines of
>
> > +#ifndef CONFIG_OPTIMIZE_INLINING && (__GNUC__ > 3)
>
> It would be nice to be able to feed the gcc version into the Kconfig logic,
> really..
We can do this using a few environment options.
Somewhere in a Makefile:
export CC_MAJOR=`magic`
export CC_MINOR=`more magic`
And in a Kconfig file:
config CC_MAJOR
string
option env=CC_MAJOR
And then we can do:
config CC_DEFAULT_INLINE
bool "Force inline of functions annotated inline"
default y if CC_MAJOR = 04
Shall I try to cook up a patch for this?
[I may get access to my dev box tonight - cannot promise..]
Sam
On Tue, 4 Mar 2008 09:03:59 +0100 Sam Ravnborg <[email protected]> wrote:
> > It would be nice to be able to feed the gcc version into the Kconfig logic,
> > really..
> We can do this using a few environment options.
>
> Somewhere in a Makefile:
> export CC_MAJOR=`magic`
> export CC_MINOR=`more magic`
>
> And in a Kconfig file:
>
> config CC_MAJOR
> string
> option env=CC_MAJOR
>
>
> And then we can do:
> config CC_DEFAULT_INLINE
> bool "Force inline of functions annotated inline"
> default y if CC_MAJOR = 04
Sounds neat. I guess we should do HOST_CC too.
> Shall I try to cook up a patch for this?
> [I may get access to my dev box tonight - cannot promise..]
accuracy=1/speed. There's no rush on this ;)
Andrew Morton <[email protected]> writes:
>
> This was 2.5.x - you'll need to look in the historical-git tree.
>
> Here it is:
>
>
>
> : commit 4507a6a59cfc6997e532cd812a8bd244181e6205
> : Author: akpm <akpm>
> : Date: Tue Mar 11 07:42:00 2003 +0000
> :
> : [PATCH] work around gcc-3.x inlining bugs
> :
> : Force inlining even when gcc-3.x is too confused to do it for us.
I think these old inlining bugs were just caused by missing __always_inline
(e.g. in the vsyscall code which requires forced inlining or in copy_*_user)
AFAIK these all have __always_inline these days and if any are still missing these
are easy to change over as needed.
So Ingo's change is likely ok.
-Andi
* Adrian Bunk <[email protected]> wrote:
> > well, i'm trying to assume the best, so please explain the following
> > sequence of events to me:
> >
> > 1) as you said you knew about this bug - which bug causes more inlining
> > overhead than hundreds of your uninlining patches combined. The bug
> > was introduced ~2 years ago in -mm - before the feature hit mainline
> > in v2.6.16.
>
> I don't remember having ever said this.
>
> Your choices are:
> [ ] prove your accusation that I said I
> "knew about this bug before the feature hit mainline"
> [ ] apologize
> [ ] be the firest person ever in my killfile
Adrian, you must be misunderstanding something. Where exactly in the
above sentences do i assert that you "knew about this bug before the
feature hit mainline"? I dont say that and cannot say that - and it's
rather irrelevant. All i say is that you knew about this bug for a long
time.
> >...
> > still, while you clearly had interest in this general area of the kernel
> > (for example you wrote hundreds of tiny uninlining patches that work
> > towards a similar goal),
>
> I'm not sure with whom you confuse me on this one.
> Perhaps with Ilpo?
i mean you send tons of trivial patches along the lines of:
| Author: Adrian Bunk <[email protected]>
| Date: Fri Feb 22 21:58:37 2008 +0200
|
| x86: don't make swapper_pg_pmd global
to reduce size of the kernel. At 50 bytes of image savings a pop, the
127,000 bytes savings my patch gives would be equivalent to more than
2500 of such patches of yours.
In other words: you knew about a bug that would have the same kernel
image size reduction equivalent to 2500 of your own tiny patches. Still
you didnt feel the need to pursue the issue?
I'm not sure about you, but that sure looks weird to me, and i'm really
curious what your interpretation of it is. (which, AFAICS, you have not
offered so far. You have rejected my common-sense explanation of your
actions rather forcefully, so logically there must be some other
explanation.)
Ingo
On Tue, Mar 04, 2008 at 02:16:00PM +0100, Ingo Molnar wrote:
>
> * Adrian Bunk <[email protected]> wrote:
>
> > > well, i'm trying to assume the best, so please explain the following
> > > sequence of events to me:
> > >
> > > 1) as you said you knew about this bug - which bug causes more inlining
> > > overhead than hundreds of your uninlining patches combined. The bug
> > > was introduced ~2 years ago in -mm - before the feature hit mainline
> > > in v2.6.16.
> >
> > I don't remember having ever said this.
> >
> > Your choices are:
> > [ ] prove your accusation that I said I
> > "knew about this bug before the feature hit mainline"
> > [ ] apologize
> > [ ] be the firest person ever in my killfile
>
> Adrian, you must be misunderstanding something. Where exactly in the
> above sentences do i assert that you "knew about this bug before the
> feature hit mainline"? I dont say that and cannot say that -
Please explain your statement "before the feature hit mainline in
v2.6.16" in the above sentence of you in a reasonable way other than
that it should say I knew about it before the feature hit mainline.
> and it's
> rather irrelevant.
Perhaps for you.
For me it's not irrelevant what I'm publically being accused of.
And it's not the first time in the thread that you use things that are
objectively not true in accusations against me.
> All i say is that you knew about this bug for a long
> time.
>...
I found the bug.
I repeatedly told about this bug on linux-kernel.
I explained to you who claimed just yesterday "my experience was that
it had effects." why it couldn't have possibly worked (no matter why
your experience was differently).
And now you start big flames against me why I hadn't stated more boldly
that this patch never worked.
Not against Arjan who seems to have sent a patch that never worked.
Not against Harvey who independently spotted the same bug.
Not against the people who didn't care when I said on linux-kernel that
it didn't work (one of the threads even had CONFIG_FORCED_INLINING in
the subject).
Thanks a lot, Mr. Molnar.
> 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
* Adrian Bunk <[email protected]> wrote:
> On Tue, Mar 04, 2008 at 02:16:00PM +0100, Ingo Molnar wrote:
> >
> > * Adrian Bunk <[email protected]> wrote:
> >
> > > > well, i'm trying to assume the best, so please explain the following
> > > > sequence of events to me:
> > > >
> > > > 1) as you said you knew about this bug - which bug causes more inlining
> > > > overhead than hundreds of your uninlining patches combined. The bug
> > > > was introduced ~2 years ago in -mm - before the feature hit mainline
> > > > in v2.6.16.
> > >
> > > I don't remember having ever said this.
> > >
> > > Your choices are:
> > > [ ] prove your accusation that I said I
> > > "knew about this bug before the feature hit mainline"
> > > [ ] apologize
> > > [ ] be the firest person ever in my killfile
> >
> > Adrian, you must be misunderstanding something. Where exactly in the
> > above sentences do i assert that you "knew about this bug before the
> > feature hit mainline"? I dont say that and cannot say that -
>
> Please explain your statement "before the feature hit mainline in
> v2.6.16" in the above sentence of you in a reasonable way other than
> that it should say I knew about it before the feature hit mainline.
do you mean this paragraph:
| 1) as you said you knew about this bug - which bug causes more
| inlining overhead than hundreds of your uninlining patches combined.
| The bug was introduced ~2 years ago in -mm - before the feature hit
| mainline in v2.6.16.
sorry, but i know of no rule of grammar that could read your
interpretation into my two sentences.
(and that's not surprising at all, because i never intended to even
suggest that you knew about this breakage "before it went mainline" -
why would i even care about such a detail?)
Ingo
On Tue, 4 March 2008 15:22:48 +0100, Ingo Molnar wrote:
>
> do you mean this paragraph:
Could you two please refrain from sending any more of this? Who said
what when and where doesn't help anyone, not even either of your ego's
or hurt feelings.
A huge paperbag bug was found and I'm happy it's gone. It could and
should have been fixed long ago. As for personal motives, I just
couldn't care less. And this thread has devolved way beyond a
metadiscussion about motives.
Jörn
--
Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are,
by definition, not smart enough to debug it.
-- Brian W. Kernighan
* J?rn Engel <[email protected]> wrote:
> On Tue, 4 March 2008 15:22:48 +0100, Ingo Molnar wrote:
> >
> > do you mean this paragraph:
>
> Could you two please refrain from sending any more of this? Who said
> what when and where doesn't help anyone, not even either of your ego's
> or hurt feelings.
well, there's no hurt feelings on my side. I'm not sure about you, but i
tend to reply when i get accused of lying:
http://lkml.org/lkml/2008/3/3/119
at least up to a certain point. Which point we reached right now, so i'm
zapping this thread now ;-)
Ingo
* Sam Ravnborg <[email protected]> wrote:
> > Subject: x86: phase out forced inlining
>
> Any particular reason you made the patch x86 specific?
to keep it simple for now. Some of the other 24 architectures are
seriously under-tested and while we can make sure x86 works well, i dont
test the others. If it works out fine on x86 it can be generalized.
> > +config OPTIMIZE_INLINING
>
> Other (not all) config options that deal with gcc behaviour are named
> CC_*. But they mostly impact gcc options. CC_OPTIMIZE_INLINING would
> match the naming of CC_OPTIMIZE_SIZE, except in the latter OPTIMIZE
> refer to the -O option.
>
> CC_DEFAULT_INLINE may give the right associations?
i really wanted to name it 'optimize' - because that's what it does. We
just lost 2 years of uninlining advantage due to CONFIG_FORCED_INLINING
not working and nobody actually connecting the dots that the lack of
'forced inlining' should have resulted in a 'smaller image' and report
it as a bug.
> > + test gcc for this.
>
> Would it be worth here to mention that stuff that really needs
> inlining should use __always_inle and not inline?
i think people know that, but i'll add it.
> > + */
> > +#ifndef CONFIG_OPTIMIZE_INLINING
> > +# define inline inline __attribute__((always_inline))
> > +# define __inline__ __inline__ __attribute__((always_inline))
> > +# define __inline __inline __attribute__((always_inline))
> > +#endif
>
> A quick google did not tell me the difference between inline,
> __inline, __inline__. But it turned out the december 2005 thread where
> there was a lenghty discussion about trusting gcc with respect to
> inlining. It is not the subject of this patch but I just wondered why
> we need all these variants.
i dont know why they there are so many variants, but all of them seem to
be used throughout the kernel:
inline : 25648
__inline__ : 1380
__inline : 368
so obviously the patch has to cover them.
a few stats about inlines btw:
- in v2.6.24 there were 26452 inlines in the kernel in 8083114 lines of
code - or one inline per 305.6 lines of code.
- in v2.6.25-rc3 there are 27396 inlines in the kernel in 8387992 lines
of code - or one inline per 308.2 lines of code.
at that rate, all inlines will be removed in about 117.5 kernel cycles -
which, if we count with 90 day release cycles, will be finished in about
29 years.
if we only look at include/linux/ files [which have the largest inlining
effect], the rate of inline removal is in fact negative: in v2.6.24 we
had one inline per 59.1 lines, in 2.6.25-to-be we have one inline per
57.9 lines.
so i'm not holding my breath and i'm going for the much more immediate
benefit of CONFIG_OPTIMIZE_INLINING=y.
Ingo
On Tue, 2008-03-04 at 17:46 +0100, Ingo Molnar wrote:
> * Sam Ravnborg <[email protected]> wrote:
> i dont know why they there are so many variants, but all of them seem to
> be used throughout the kernel:
>
> inline : 25648
I'll assume this is the preferred way of saying it.
> __inline__ : 1380
Lots of them in include/asm-*...not sure if there is a reason for this.
> __inline : 368
Almost all of them in drivers/scsi
Cheers,
Harvey
Harvey Harrison wrote:
> On Tue, 2008-03-04 at 17:46 +0100, Ingo Molnar wrote:
>> * Sam Ravnborg <[email protected]> wrote:
>
>> i dont know why they there are so many variants, but all of them seem to
>> be used throughout the kernel:
>>
>> inline : 25648
>
> I'll assume this is the preferred way of saying it.
>
>> __inline__ : 1380
>
> Lots of them in include/asm-*...not sure if there is a reason for this.
>
Preferred form for code that's exported to userspace (since gcc
complains with -ansi -pedantic otherwise.)
-hpa
On Tue, 2008-03-04 at 10:09 -0800, H. Peter Anvin wrote:
> Harvey Harrison wrote:
> >
> >> __inline__ : 1380
> >
> > Lots of them in include/asm-*...not sure if there is a reason for this.
> >
>
> Preferred form for code that's exported to userspace (since gcc
> complains with -ansi -pedantic otherwise.)
>
Figured it would be something like that. Would it be reasonable to move
towards eliminating __inline?
Also, since the exported headers already go through unifdef, could we
move to using inline everywhere in the kernel and add a processing step
to make it __inline__ in the exported headers?
Harvey
On Tue, 2008-03-04 at 10:09 -0800, H. Peter Anvin wrote:
> Harvey Harrison wrote:
> > On Tue, 2008-03-04 at 17:46 +0100, Ingo Molnar wrote:
> >> * Sam Ravnborg <[email protected]> wrote:
> >
> >> i dont know why they there are so many variants, but all of them seem to
> >> be used throughout the kernel:
> >>
> >> inline : 25648
> >
> > I'll assume this is the preferred way of saying it.
> >
> >> __inline__ : 1380
> >
> > Lots of them in include/asm-*...not sure if there is a reason for this.
> >
>
> Preferred form for code that's exported to userspace (since gcc
> complains with -ansi -pedantic otherwise.)
>
> -hpa