[RFC][PATCH] tree-wide: Remove __inline__ and __inline usage

[RFC][PATCH] tree-wide: Remove __inline__ and __inline usage

[Peter Zijlstra]

Hi Linus,

A proposed GCC asm extention:

  https://gcc.gnu.org/ml/gcc-patches/2018-10/msg01931.html

allows adding the "inline" keyword to 'asm ("")' statements. The
problem is that we're possibly redefining "inline" to
"inline __attribute__((__always_inline__))" which makes the proposed:

  asm volatile inline ("")

not compile.

However, since we've been depricating the use of the alternative
inline keywords: "__inline__" and "__inline", and there are only a
'few' uses left of them in the tree:

  $ git grep -e "\<__inline__\>" | wc -l
  487
  $ git grep -e "\<__inline\>" | wc -l
  56
  $ git grep -e "\<inline\>" | wc -l
  69957

let's finish them off now.

Therefore I'm proposing to run:

  git grep -l "\<__inline\(\|__\)\>" | while read file
  do
	sed -i -e 's/\<__inline\(\|__\)\>/inline/g' $file
  done

On your current tree, and apply the below fixup patch on top of that
result.

This would then allow us to do something like (+- GCC feature tests):

  #define asm_volatile(stmt...) asm volatile __inline__(stmt)

once that GCC patch lands.

Cc: Nadav Amit <namit@vmware.com>
Cc: Joe Perches <joe@perches.com>
Cc: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: Segher Boessenkool <segher@kernel.crashing.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/compiler_types.h |    3 ---
 scripts/checkpatch.pl          |   12 ++++++------
 scripts/genksyms/keywords.c    |    2 --
 scripts/kernel-doc             |    2 --
 4 files changed, 6 insertions(+), 13 deletions(-)

--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -202,9 +202,6 @@ struct ftrace_likely_data {
 	__maybe_unused notrace
 #endif
 
-#define inline inline
-#define inline   inline
-
 /*
  * Rather then using noinline to prevent stack consumption, use
  * noinline_for_stack instead.  For documentation reasons.
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -389,7 +389,7 @@ our $Attribute	= qr{
 			__weak
 		  }x;
 our $Modifier;
-our $Inline	= qr{inline|__always_inline|noinline|inline|inline};
+our $Inline	= qr{inline|__always_inline|noinline};
 our $Member	= qr{->$Ident|\.$Ident|\[[^]]*\]};
 our $Lval	= qr{$Ident(?:$Member)*};
 
@@ -5771,13 +5771,13 @@ sub process {
 			      "inline keyword should sit between storage class and type\n" . $herecurr);
 		}
 
-# Check for inline and inline, prefer inline
+# Check for __inline__ and __inline, prefer inline
 		if ($realfile !~ m@\binclude/uapi/@ &&
-		    $line =~ /\b(inline|inline)\b/) {
-			if (WARN("INLINE",
-				 "plain inline is preferred over $1\n" . $herecurr) &&
+		    $line =~ /\b(__inline__|__inline)\b/) {
+			if (ERROR("INLINE",
+				  "plain inline is preferred over $1\n" . $herecurr) &&
 			    $fix) {
-				$fixed[$fixlinenr] =~ s/\b(inline|inline)\b/inline/;
+				$fixed[$fixlinenr] =~ s/\b(__inline__|__inline)\b/inline/;
 
 			}
 		}
--- a/scripts/genksyms/keywords.c
+++ b/scripts/genksyms/keywords.c
@@ -14,8 +14,6 @@ static struct resword {
 	{ "__const", CONST_KEYW },
 	{ "__const__", CONST_KEYW },
 	{ "__extension__", EXTENSION_KEYW },
-	{ "inline", INLINE_KEYW },
-	{ "inline", INLINE_KEYW },
 	{ "__signed", SIGNED_KEYW },
 	{ "__signed__", SIGNED_KEYW },
 	{ "__typeof", TYPEOF_KEYW },
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1569,8 +1569,6 @@ sub dump_function($$) {
     $prototype =~ s/^extern +//;
     $prototype =~ s/^asmlinkage +//;
     $prototype =~ s/^inline +//;
-    $prototype =~ s/^inline +//;
-    $prototype =~ s/^inline +//;
     $prototype =~ s/^__always_inline +//;
     $prototype =~ s/^noinline +//;
     $prototype =~ s/__init +//;

Re: [RFC][PATCH] tree-wide: Remove __inline__ and __inline usage

[Miguel Ojeda]

On Tue, Nov 6, 2018 at 11:02 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> allows adding the "inline" keyword to 'asm ("")' statements. The
> problem is that we're possibly redefining "inline" to
> "inline __attribute__((__always_inline__))" which makes the proposed:
>
>   asm volatile inline ("")
>
> (...)
>
> -#define inline inline
> -#define inline   inline
> -

It seems somehow your patch got underscores removed.

By the way, we have been re#defining the inline keyword since (at
least) 2003, and already in 2008 Ingo was commenting in a commit to
add the #ifdef to avoid it for x86. Is it still a good idea nowadays
that the minimum compiler is quite modern compare to a decade ago? Is
it still needed on non-x86 arches (they don't have it in the
defconfig)? Couldn't functions be marked as __always_inline if really
needed (as many are)?

By the way (x2): CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING is only ever
referenced in that #ifdef. May it be removed?

Cheers,
Miguel

Re: [RFC][PATCH] tree-wide: Remove __inline__ and __inline usage

[Peter Zijlstra]

On Tue, Nov 06, 2018 at 03:09:37PM +0100, Miguel Ojeda wrote:
> On Tue, Nov 6, 2018 at 11:02 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > allows adding the "inline" keyword to 'asm ("")' statements. The
> > problem is that we're possibly redefining "inline" to
> > "inline __attribute__((__always_inline__))" which makes the proposed:
> >
> >   asm volatile inline ("")
> >
> > (...)
> >
> > -#define inline inline
> > -#define inline   inline
> > -
> 
> It seems somehow your patch got underscores removed.

If you actually read what I wrote:

> > Therefore I'm proposing to run:
> > 
> >   git grep -l "\<__inline\(\|__\)\>" | while read file
> >   do
> > 	sed -i -e 's/\<__inline\(\|__\)\>/inline/g' $file
> >   done
> > 
> > On your current tree, and apply the below fixup patch on top of that
> > result.

It makes sense.

> By the way, we have been re#defining the inline keyword since (at
> least) 2003, and already in 2008 Ingo was commenting in a commit to
> add the #ifdef to avoid it for x86. Is it still a good idea nowadays
> that the minimum compiler is quite modern compare to a decade ago? Is
> it still needed on non-x86 arches (they don't have it in the
> defconfig)? Couldn't functions be marked as __always_inline if really
> needed (as many are)?
> 
> By the way (x2): CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING is only ever
> referenced in that #ifdef. May it be removed?

Dunno, but that is a far more difficult patch. The proposed one is an
obvious identify.

Re: [RFC][PATCH] tree-wide: Remove __inline__ and __inline usage

[Miguel Ojeda]

On Tue, Nov 6, 2018 at 3:46 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> If you actually read what I wrote:

You got me, I did read very quickly :)

> Dunno, but that is a far more difficult patch. The proposed one is an
> obvious identify.

I would say they are orthogonal, even if both would solve the problem.
I also wondered why we have the funny __inline[__] stuff around while
doing the compiler attributes series, so I thought bringing it up.

Cheers,
Miguel

Re: [RFC][PATCH] tree-wide: Remove __inline__ and __inline usage

[Linus Torvalds]

On Tue, Nov 6, 2018 at 2:02 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> Therefore I'm proposing to run:
>
>   git grep -l "\<__inline\(\|__\)\>" | while read file
>   do
>         sed -i -e 's/\<__inline\(\|__\)\>/inline/g' $file
>   done
>
> On your current tree, and apply the below fixup patch on top of that
> result.

So I started doing this, and in fact fixed up a few more issues by
hand on top of your patch, but then realized hat it's somewhat
dangerous and possibly broken.

For the uapi header files in particular, __inline__ may actually be
required. Depending on use, and compiler settings, "inline" can be a
word reserved for the user, and shouldn't be used by system headers.

Now, several uapi headers obviously *do* use "inline", and I think in
this day and age that's fine, but I don't actually want to break
possible valid uses.

So I'd argue that we don't actually want to get rid of "__inline__" at
all, because we may need it.

But we *could* get rid of these two lines in include/linux/compiler_types.h

  #define __inline__ inline
  #define __inline   inline

and just say that "inline" for the kernel means "always_inline", but
if you use __inline__ or __inline then you get the "raw" compiler
inlining.

Then people can decide to get rid of __inline__ on a case-by-case basis.

                 Linus

Re: [RFC][PATCH] tree-wide: Remove __inline__ and __inline usage

[Peter Zijlstra]

On Tue, Nov 06, 2018 at 11:18:59AM -0800, Linus Torvalds wrote:
> On Tue, Nov 6, 2018 at 2:02 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > Therefore I'm proposing to run:
> >
> >   git grep -l "\<__inline\(\|__\)\>" | while read file
> >   do
> >         sed -i -e 's/\<__inline\(\|__\)\>/inline/g' $file
> >   done
> >
> > On your current tree, and apply the below fixup patch on top of that
> > result.
> 
> So I started doing this, and in fact fixed up a few more issues by
> hand on top of your patch, but then realized hat it's somewhat
> dangerous and possibly broken.
> 
> For the uapi header files in particular, __inline__ may actually be
> required. Depending on use, and compiler settings, "inline" can be a
> word reserved for the user, and shouldn't be used by system headers.

*groan*, indeed. Now obvious those headers need to compile without our
override, so we could simply exclude uapi from the transformation.

(and __inline is mostly in staging/ and a few stray places, we really
should get rid of that one I feel, there's so few of them)

> But we *could* get rid of these two lines in include/linux/compiler_types.h
> 
>   #define __inline__ inline
>   #define __inline   inline
> 
> and just say that "inline" for the kernel means "always_inline", but
> if you use __inline__ or __inline then you get the "raw" compiler
> inlining.
> 
> Then people can decide to get rid of __inline__ on a case-by-case basis.

Right, that gets us what we need; but makes a fair bunch of kernel code
compile differently.

It probably doesn't matter, and a fair amount of the __inline__ usage is
in fairly crusty code which will likely never get fixed up.

And that is probably still a safer option than removing the #define
inline entirely.

Do you want me to do that patch, or have you already just done it?

Re: [RFC][PATCH] tree-wide: Remove __inline__ and __inline usage

[Linus Torvalds]

On Tue, Nov 6, 2018 at 11:42 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> Do you want me to do that patch, or have you already just done it?

I'd rather see it go through something like -tip than doing it myself
directly, and get at least some of the automated testing before
unleashing it on an unsuspecting world.

I don't think it will affect much, but there *could* be situations
where there are some crusty __inline__ users that actually want
__always_inline behavior.

And we *may* actually have cases where we want the "let compiler make
a judgement call" behavior, so with this change people will have that
as an option. But yes, in the short term, it has the possibility of
regressions due to missed inlining.

                Linus

Re: [RFC][PATCH] tree-wide: Remove __inline__ and __inline usage

[Peter Zijlstra]

On Tue, Nov 06, 2018 at 11:51:35AM -0800, Linus Torvalds wrote:
> On Tue, Nov 6, 2018 at 11:42 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > Do you want me to do that patch, or have you already just done it?
> 
> I'd rather see it go through something like -tip than doing it myself
> directly, and get at least some of the automated testing before
> unleashing it on an unsuspecting world.
> 
> I don't think it will affect much, but there *could* be situations
> where there are some crusty __inline__ users that actually want
> __always_inline behavior.
> 
> And we *may* actually have cases where we want the "let compiler make
> a judgement call" behavior, so with this change people will have that
> as an option. But yes, in the short term, it has the possibility of
> regressions due to missed inlining.

Fair enough; I'll do the patch and let it soak a bit in tip.

Thanks!