[PATCH] Stop making "inline" imply forced inlining.

[PATCH] Stop making "inline" imply forced inlining.

[Robert P. J. Day]

  Remove the macros that define simple "inlining" to mean forced
inlining, since you can (and *should*) get that effect with the
CONFIG_FORCED_INLINING kernel config variable instead.

Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>

---

  this change was compile tested on x86 with "make allyesconfig",
followed by turning off forced inlining.

  now the alpha folks can simplify their compiler.h file. :-)


diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 6e1c44a..5a90bd9 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -23,9 +23,6 @@
     (typeof(ptr)) (__ptr + (off)); })


-#define inline		inline		__attribute__((always_inline))
-#define __inline__	__inline__	__attribute__((always_inline))
-#define __inline	__inline	__attribute__((always_inline))
 #define __deprecated			__attribute__((deprecated))
 #define  noinline			__attribute__((noinline))
 #define __attribute_pure__		__attribute__((pure))

Re: [PATCH] Stop making "inline" imply forced inlining.

[Adrian Bunk]

On Fri, Jan 19, 2007 at 11:56:30AM -0500, Robert P. J. Day wrote:
> 
>   Remove the macros that define simple "inlining" to mean forced
> inlining, since you can (and *should*) get that effect with the
> CONFIG_FORCED_INLINING kernel config variable instead.

NAK.

I don't see any place in the kernel where we need a non-forced inline.

We have tons of inline's in C files that should simply be removed - 
let's do this instead.

> Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>
> 
> ---
> 
>   this change was compile tested on x86 with "make allyesconfig",
> followed by turning off forced inlining.
> 
>   now the alpha folks can simplify their compiler.h file. :-)
> 
> 
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 6e1c44a..5a90bd9 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -23,9 +23,6 @@
>      (typeof(ptr)) (__ptr + (off)); })
> 
> 
> -#define inline		inline		__attribute__((always_inline))
> -#define __inline__	__inline__	__attribute__((always_inline))
> -#define __inline	__inline	__attribute__((always_inline))
>  #define __deprecated			__attribute__((deprecated))
>  #define  noinline			__attribute__((noinline))
>  #define __attribute_pure__		__attribute__((pure))

Oh, and your patch would have been broken for gcc < 4.

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] Stop making "inline" imply forced inlining.

[Robert P. J. Day]

On Fri, 19 Jan 2007, Adrian Bunk wrote:

> On Fri, Jan 19, 2007 at 11:56:30AM -0500, Robert P. J. Day wrote:
> >
> >   Remove the macros that define simple "inlining" to mean forced
> > inlining, since you can (and *should*) get that effect with the
> > CONFIG_FORCED_INLINING kernel config variable instead.
>
> NAK.
>
> I don't see any place in the kernel where we need a non-forced
> inline.

that's not the point.  the point is that, as it stands now, the build
is *broken* in three ways.

first, it's broken because declaring something simply as "inline"
*forces* it to be inlined, which flies in the face of historical
convention and is more than a little misleading.

second, it's broken because both the use of
"__attribute__((always_inline))" all over the place and the
CONFIG_FORCED_INLINING kernel config option imply that you indeed have
a choice, when you clearly *don't*.  quite simply, you can play with
that kernel config option or splash the "always_inline" attributes
around all you want and, unbeknownst to you, none of that is making
the *slightest* bit of difference.  that is the very *definition* of a
"broken" build.

and, finally, you claim that you "don't see any place in the kernel
where we need a non-forced inline."  i have already posted an alpha
header file that claims (rightly or wrongly) to need that freedom.
Q.E.D.

> We have tons of inline's in C files that should simply be removed -
> let's do this instead.

that may be a better idea, but it doesn't address the current
brokenness.

i'm willing to believe that this patch has zero chance of going
anywhere.  but if you want to reject it, at least be honest about it.
don't say, "there's no problem here."  instead, say, "yes, the build
is broken but we don't feel like doing anything about it."

rday

Re: [PATCH] Stop making "inline" imply forced inlining.

[Adrian Bunk]

On Fri, Jan 19, 2007 at 12:39:37PM -0500, Robert P. J. Day wrote:
> On Fri, 19 Jan 2007, Adrian Bunk wrote:
> 
> > On Fri, Jan 19, 2007 at 11:56:30AM -0500, Robert P. J. Day wrote:
> > >
> > >   Remove the macros that define simple "inlining" to mean forced
> > > inlining, since you can (and *should*) get that effect with the
> > > CONFIG_FORCED_INLINING kernel config variable instead.
> >
> > NAK.
> >
> > I don't see any place in the kernel where we need a non-forced
> > inline.
> 
> that's not the point.  the point is that, as it stands now, the build
> is *broken* in three ways.
> 
> first, it's broken because declaring something simply as "inline"
> *forces* it to be inlined, which flies in the face of historical
> convention and is more than a little misleading.

In the kernel it's what you should expect since it's defined this way 
for some time.

> second, it's broken because both the use of
> "__attribute__((always_inline))" all over the place and the
> CONFIG_FORCED_INLINING kernel config option imply that you indeed have
> a choice, when you clearly *don't*.  quite simply, you can play with
> that kernel config option or splash the "always_inline" attributes
> around all you want and, unbeknownst to you, none of that is making
> the *slightest* bit of difference.  that is the very *definition* of a
> "broken" build.

It's the definition of a broken option.

The solution is to remove CONFIG_FORCED_INLINING.

> and, finally, you claim that you "don't see any place in the kernel
> where we need a non-forced inline."  i have already posted an alpha
> header file that claims (rightly or wrongly) to need that freedom.
> Q.E.D.

Not Q.E.D. due to "rightly or wrongly".

It could be because Alpha uses "extern inline" and with it's old 
semantice I'd understand that always_inline might be a problem - but is 
there actually any place where Alpha uses "extern inline" with this
semantics and not in a way that it could be replaced with "static inline"?

> > We have tons of inline's in C files that should simply be removed -
> > let's do this instead.
> 
> that may be a better idea, but it doesn't address the current
> brokenness.
> 
> i'm willing to believe that this patch has zero chance of going
> anywhere.  but if you want to reject it, at least be honest about it.
> don't say, "there's no problem here."  instead, say, "yes, the build
> is broken but we don't feel like doing anything about it."

Can you give at least one concrete example of actually broken code?

The only implication I know it has caused is increased code size, but 
that's different from being "broken".

And that's not "but we don't feel like doing anything about it" - it's
"we should remove all the inline's from C files".

> rday

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