[PATCH net] docs: netdev: Document guidance on inline functions

[PATCH net] docs: netdev: Document guidance on inline functions

[Simon Horman]

Document preference for non inline functions in .c files.
This has been the preference for as long as I can recall
and I was recently surprised to discover that it is undocumented.

Reported-by: Alexandre Ferrieux <alexandre.ferrieux@gmail.com>
Closes: https://lore.kernel.org/all/9662e6fe-cc91-4258-aba1-ab5b016a041a@orange.com/
Signed-off-by: Simon Horman <horms@kernel.org>
---
 Documentation/process/maintainer-netdev.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/process/maintainer-netdev.rst b/Documentation/process/maintainer-netdev.rst
index e497729525d5..1fbb8178b8cd 100644
--- a/Documentation/process/maintainer-netdev.rst
+++ b/Documentation/process/maintainer-netdev.rst
@@ -408,6 +408,17 @@ at a greater cost than the value of such clean-ups.
 
 Conversely, spelling and grammar fixes are not discouraged.
 
+Inline functions
+----------------
+
+The use of static inline functions in .c file is strongly discouraged
+unless there is a demonstrable reason for them, usually performance
+related. Rather, it is preferred to omit the inline keyword and allow the
+compiler to inline them as it sees fit.
+
+This is a stricter requirement than that of the general Linux Kernel
+:ref:`Coding Style<codingstyle>`
+
 Resending after review
 ~~~~~~~~~~~~~~~~~~~~~~
 

Re: [PATCH net] docs: netdev: Document guidance on inline functions

[Jonathan Corbet]

Simon Horman <horms@kernel.org> writes:

> Document preference for non inline functions in .c files.
> This has been the preference for as long as I can recall
> and I was recently surprised to discover that it is undocumented.
>
> Reported-by: Alexandre Ferrieux <alexandre.ferrieux@gmail.com>
> Closes: https://lore.kernel.org/all/9662e6fe-cc91-4258-aba1-ab5b016a041a@orange.com/
> Signed-off-by: Simon Horman <horms@kernel.org>
> ---
>  Documentation/process/maintainer-netdev.rst | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/Documentation/process/maintainer-netdev.rst b/Documentation/process/maintainer-netdev.rst
> index e497729525d5..1fbb8178b8cd 100644
> --- a/Documentation/process/maintainer-netdev.rst
> +++ b/Documentation/process/maintainer-netdev.rst
> @@ -408,6 +408,17 @@ at a greater cost than the value of such clean-ups.
>  
>  Conversely, spelling and grammar fixes are not discouraged.
>  
> +Inline functions
> +----------------
> +
> +The use of static inline functions in .c file is strongly discouraged
> +unless there is a demonstrable reason for them, usually performance
> +related. Rather, it is preferred to omit the inline keyword and allow the
> +compiler to inline them as it sees fit.
> +
> +This is a stricter requirement than that of the general Linux Kernel
> +:ref:`Coding Style<codingstyle>`

I have no objection to this change, but I do wonder if it does indeed
belong in the central coding-style document.  I don't think anybody
encourages use of "inline" these days...?

jon

Re: [PATCH net] docs: netdev: Document guidance on inline functions

[Mauro Carvalho Chehab]

Em Mon, 03 Feb 2025 08:00:56 -0700
Jonathan Corbet <corbet@lwn.net> escreveu:

> Simon Horman <horms@kernel.org> writes:
> 
> > Document preference for non inline functions in .c files.
> > This has been the preference for as long as I can recall
> > and I was recently surprised to discover that it is undocumented.
> >
> > Reported-by: Alexandre Ferrieux <alexandre.ferrieux@gmail.com>
> > Closes: https://lore.kernel.org/all/9662e6fe-cc91-4258-aba1-ab5b016a041a@orange.com/
> > Signed-off-by: Simon Horman <horms@kernel.org>
> > ---
> >  Documentation/process/maintainer-netdev.rst | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/Documentation/process/maintainer-netdev.rst b/Documentation/process/maintainer-netdev.rst
> > index e497729525d5..1fbb8178b8cd 100644
> > --- a/Documentation/process/maintainer-netdev.rst
> > +++ b/Documentation/process/maintainer-netdev.rst
> > @@ -408,6 +408,17 @@ at a greater cost than the value of such clean-ups.
> >  
> >  Conversely, spelling and grammar fixes are not discouraged.
> >  
> > +Inline functions
> > +----------------
> > +
> > +The use of static inline functions in .c file is strongly discouraged
> > +unless there is a demonstrable reason for them, usually performance
> > +related. Rather, it is preferred to omit the inline keyword and allow the
> > +compiler to inline them as it sees fit.

You should probably point to chapter (12) of Documentation/process/coding-style.rst
where it mentions that inline for function prototypes and as a way to
replace macros are OK.

> > +
> > +This is a stricter requirement than that of the general Linux Kernel
> > +:ref:`Coding Style<codingstyle>`  
> 
> I have no objection to this change, but I do wonder if it does indeed
> belong in the central coding-style document.  I don't think anybody
> encourages use of "inline" these days...?

Indeed IMO this belongs to the coding style. I would place it close
to chapter (12) at Documentation/process/coding-style.rst.

Regards,

Thanks,
Mauro

Re: [PATCH net] docs: netdev: Document guidance on inline functions

[Mauro Carvalho Chehab]

Em Mon, 3 Feb 2025 20:50:39 +0100
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:

> Em Mon, 03 Feb 2025 08:00:56 -0700
> Jonathan Corbet <corbet@lwn.net> escreveu:
> 
> > Simon Horman <horms@kernel.org> writes:
> > 
> > > Document preference for non inline functions in .c files.
> > > This has been the preference for as long as I can recall
> > > and I was recently surprised to discover that it is undocumented.
> > >
> > > Reported-by: Alexandre Ferrieux <alexandre.ferrieux@gmail.com>
> > > Closes: https://lore.kernel.org/all/9662e6fe-cc91-4258-aba1-ab5b016a041a@orange.com/
> > > Signed-off-by: Simon Horman <horms@kernel.org>
> > > ---
> > >  Documentation/process/maintainer-netdev.rst | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >
> > > diff --git a/Documentation/process/maintainer-netdev.rst b/Documentation/process/maintainer-netdev.rst
> > > index e497729525d5..1fbb8178b8cd 100644
> > > --- a/Documentation/process/maintainer-netdev.rst
> > > +++ b/Documentation/process/maintainer-netdev.rst
> > > @@ -408,6 +408,17 @@ at a greater cost than the value of such clean-ups.
> > >  
> > >  Conversely, spelling and grammar fixes are not discouraged.
> > >  
> > > +Inline functions
> > > +----------------
> > > +
> > > +The use of static inline functions in .c file is strongly discouraged
> > > +unless there is a demonstrable reason for them, usually performance
> > > +related. Rather, it is preferred to omit the inline keyword and allow the
> > > +compiler to inline them as it sees fit.
> 
> You should probably point to chapter (12) of Documentation/process/coding-style.rst
> where it mentions that inline for function prototypes and as a way to
> replace macros are OK.

Heh, I hit enter too quickly...

I mean:
	"inline for function prototypes and as a way to replace macros on
	 header files (*.h) are OK."

> 
> > > +
> > > +This is a stricter requirement than that of the general Linux Kernel
> > > +:ref:`Coding Style<codingstyle>`  
> > 
> > I have no objection to this change, but I do wonder if it does indeed
> > belong in the central coding-style document.  I don't think anybody
> > encourages use of "inline" these days...?
> 
> Indeed IMO this belongs to the coding style. I would place it close
> to chapter (12) at Documentation/process/coding-style.rst.
> 
> Regards,
> 
> Thanks,
> Mauro



Thanks,
Mauro

Re: [PATCH net] docs: netdev: Document guidance on inline functions

[Simon Horman]

On Mon, Feb 03, 2025 at 08:53:12PM +0100, Mauro Carvalho Chehab wrote:
> Em Mon, 3 Feb 2025 20:50:39 +0100
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> 
> > Em Mon, 03 Feb 2025 08:00:56 -0700
> > Jonathan Corbet <corbet@lwn.net> escreveu:
> > 
> > > Simon Horman <horms@kernel.org> writes:
> > > 
> > > > Document preference for non inline functions in .c files.
> > > > This has been the preference for as long as I can recall
> > > > and I was recently surprised to discover that it is undocumented.
> > > >
> > > > Reported-by: Alexandre Ferrieux <alexandre.ferrieux@gmail.com>
> > > > Closes: https://lore.kernel.org/all/9662e6fe-cc91-4258-aba1-ab5b016a041a@orange.com/
> > > > Signed-off-by: Simon Horman <horms@kernel.org>
> > > > ---
> > > >  Documentation/process/maintainer-netdev.rst | 11 +++++++++++
> > > >  1 file changed, 11 insertions(+)
> > > >
> > > > diff --git a/Documentation/process/maintainer-netdev.rst b/Documentation/process/maintainer-netdev.rst
> > > > index e497729525d5..1fbb8178b8cd 100644
> > > > --- a/Documentation/process/maintainer-netdev.rst
> > > > +++ b/Documentation/process/maintainer-netdev.rst
> > > > @@ -408,6 +408,17 @@ at a greater cost than the value of such clean-ups.
> > > >  
> > > >  Conversely, spelling and grammar fixes are not discouraged.
> > > >  
> > > > +Inline functions
> > > > +----------------
> > > > +
> > > > +The use of static inline functions in .c file is strongly discouraged
> > > > +unless there is a demonstrable reason for them, usually performance
> > > > +related. Rather, it is preferred to omit the inline keyword and allow the
> > > > +compiler to inline them as it sees fit.
> > 
> > You should probably point to chapter (12) of Documentation/process/coding-style.rst
> > where it mentions that inline for function prototypes and as a way to
> > replace macros are OK.
> 
> Heh, I hit enter too quickly...
> 
> I mean:
> 	"inline for function prototypes and as a way to replace macros on
> 	 header files (*.h) are OK."

Likewise, I responded to your previous message too quickly.

Yes, I agree something like that would be good.

> 
> > 
> > > > +
> > > > +This is a stricter requirement than that of the general Linux Kernel
> > > > +:ref:`Coding Style<codingstyle>`  
> > > 
> > > I have no objection to this change, but I do wonder if it does indeed
> > > belong in the central coding-style document.  I don't think anybody
> > > encourages use of "inline" these days...?
> > 
> > Indeed IMO this belongs to the coding style. I would place it close
> > to chapter (12) at Documentation/process/coding-style.rst.
> > 
> > Regards,
> > 
> > Thanks,
> > Mauro
> 
> 
> 
> Thanks,
> Mauro
> 

Re: [PATCH net] docs: netdev: Document guidance on inline functions

[Simon Horman]

On Mon, Feb 03, 2025 at 08:50:39PM +0100, Mauro Carvalho Chehab wrote:
> Em Mon, 03 Feb 2025 08:00:56 -0700
> Jonathan Corbet <corbet@lwn.net> escreveu:
> 
> > Simon Horman <horms@kernel.org> writes:
> > 
> > > Document preference for non inline functions in .c files.
> > > This has been the preference for as long as I can recall
> > > and I was recently surprised to discover that it is undocumented.
> > >
> > > Reported-by: Alexandre Ferrieux <alexandre.ferrieux@gmail.com>
> > > Closes: https://lore.kernel.org/all/9662e6fe-cc91-4258-aba1-ab5b016a041a@orange.com/
> > > Signed-off-by: Simon Horman <horms@kernel.org>
> > > ---
> > >  Documentation/process/maintainer-netdev.rst | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >
> > > diff --git a/Documentation/process/maintainer-netdev.rst b/Documentation/process/maintainer-netdev.rst
> > > index e497729525d5..1fbb8178b8cd 100644
> > > --- a/Documentation/process/maintainer-netdev.rst
> > > +++ b/Documentation/process/maintainer-netdev.rst
> > > @@ -408,6 +408,17 @@ at a greater cost than the value of such clean-ups.
> > >  
> > >  Conversely, spelling and grammar fixes are not discouraged.
> > >  
> > > +Inline functions
> > > +----------------
> > > +
> > > +The use of static inline functions in .c file is strongly discouraged

As suggested by Andrew Lunn elsewhere in this thread I will drop
"static" from the line above.

> > > +unless there is a demonstrable reason for them, usually performance
> > > +related. Rather, it is preferred to omit the inline keyword and allow the
> > > +compiler to inline them as it sees fit.
> 
> You should probably point to chapter (12) of Documentation/process/coding-style.rst
> where it mentions that inline for function prototypes and as a way to
>static  replace macros are OK.

Thanks, perhaps something like this would help:

  Using inline in .h files is fine and is encouraged in place of macros
  [reference section 12].

> 
> > > +
> > > +This is a stricter requirement than that of the general Linux Kernel
> > > +:ref:`Coding Style<codingstyle>`  
> > 
> > I have no objection to this change, but I do wonder if it does indeed
> > belong in the central coding-style document.  I don't think anybody
> > encourages use of "inline" these days...?
> 
> Indeed IMO this belongs to the coding style. I would place it close
> to chapter (12) at Documentation/process/coding-style.rst.

Sure, thanks to you and Jonathan for the positive feedback there.
I will prepare a revised patch that updates coding-style.rst instead.

Re: [PATCH net] docs: netdev: Document guidance on inline functions

[Andrew Lunn]

> Thanks, perhaps something like this would help:
> 
>   Using inline in .h files is fine and is encouraged in place of macros
>   [reference section 12].

The other major use of them in headers is for stub functions when an
API implementation has a Kconfig option. The question is, do we really
want to start creating such a list, and have people wanting to add to
it?

	Andrew

Re: [PATCH net] docs: netdev: Document guidance on inline functions

[Simon Horman]

On Tue, Feb 04, 2025 at 02:25:07PM +0100, Andrew Lunn wrote:
> > Thanks, perhaps something like this would help:
> > 
> >   Using inline in .h files is fine and is encouraged in place of macros
> >   [reference section 12].
> 
> The other major use of them in headers is for stub functions when an
> API implementation has a Kconfig option. The question is, do we really
> want to start creating such a list, and have people wanting to add to
> it?

Good point. Maybe it is sufficient to just make the distinction
between .c and .h files.

Re: [PATCH net] docs: netdev: Document guidance on inline functions

[David Laight]

... 
> > +Inline functions
> > +----------------
> > +
> > +The use of static inline functions in .c file is strongly discouraged
> > +unless there is a demonstrable reason for them, usually performance
> > +related. Rather, it is preferred to omit the inline keyword and allow the
> > +compiler to inline them as it sees fit.
> > +
> > +This is a stricter requirement than that of the general Linux Kernel
> > +:ref:`Coding Style<codingstyle>`  
> 
> I have no objection to this change, but I do wonder if it does indeed
> belong in the central coding-style document.  I don't think anybody
> encourages use of "inline" these days...?


Apart from the cases where the compiler really ought to inline something
but fails to do so because it doesn't notice just how much code collapses
out.
But in that case you need always_inline.

For instance get_sigset_argpack (fs/select.c) is marked inline but isn't
being inlined by gcc 12.2 (clang 18 is inlining it).

I've also seen places where #defines generate much better code than
inline functions because they get processed much earlier.

	David

Re: [PATCH net] docs: netdev: Document guidance on inline functions

[Andrew Lunn]

>  Conversely, spelling and grammar fixes are not discouraged.
>  
> +Inline functions
> +----------------
> +
> +The use of static inline functions in .c file is strongly discouraged

I don't think 'static' is relevant here. They probably are static, if
they are inline, and to avoid warnings about missing declarations. But
we just prefer not to have any sort of inline functions without good
justifications within a .c file.

A nit pick, so:

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net] docs: netdev: Document guidance on inline functions

[Simon Horman]

On Mon, Feb 03, 2025 at 04:10:24PM +0100, Andrew Lunn wrote:
> >  Conversely, spelling and grammar fixes are not discouraged.
> >  
> > +Inline functions
> > +----------------
> > +
> > +The use of static inline functions in .c file is strongly discouraged
> 
> I don't think 'static' is relevant here. They probably are static, if
> they are inline, and to avoid warnings about missing declarations. But
> we just prefer not to have any sort of inline functions without good
> justifications within a .c file.
> 
> A nit pick, so:
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Thanks Andrew,

I agree that static is not helpful here, I'll drop that in a v2.

Re: [PATCH net] docs: netdev: Document guidance on inline functions

[Randy Dunlap]

Hi Simon,

Another nit:

On 2/3/25 5:59 AM, Simon Horman wrote:
> Document preference for non inline functions in .c files.
> This has been the preference for as long as I can recall
> and I was recently surprised to discover that it is undocumented.
> 
> Reported-by: Alexandre Ferrieux <alexandre.ferrieux@gmail.com>
> Closes: https://lore.kernel.org/all/9662e6fe-cc91-4258-aba1-ab5b016a041a@orange.com/
> Signed-off-by: Simon Horman <horms@kernel.org>
> ---
>  Documentation/process/maintainer-netdev.rst | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/process/maintainer-netdev.rst b/Documentation/process/maintainer-netdev.rst
> index e497729525d5..1fbb8178b8cd 100644
> --- a/Documentation/process/maintainer-netdev.rst
> +++ b/Documentation/process/maintainer-netdev.rst
> @@ -408,6 +408,17 @@ at a greater cost than the value of such clean-ups.
>  
>  Conversely, spelling and grammar fixes are not discouraged.
>  
> +Inline functions
> +----------------
> +
> +The use of static inline functions in .c file is strongly discouraged
> +unless there is a demonstrable reason for them, usually performance
> +related. Rather, it is preferred to omit the inline keyword and allow the
> +compiler to inline them as it sees fit.
> +
> +This is a stricter requirement than that of the general Linux Kernel
> +:ref:`Coding Style<codingstyle>`

Is there an ending period (full stop) after that sentence?
Could/should there be?

Thanks.

> +
>  Resending after review
>  ~~~~~~~~~~~~~~~~~~~~~~
>  
> 
> 

-- 
~Randy

Re: [PATCH net] docs: netdev: Document guidance on inline functions

[Simon Horman]

On Mon, Feb 03, 2025 at 10:51:49AM -0800, Randy Dunlap wrote:
> Hi Simon,
> 
> Another nit:
> 
> On 2/3/25 5:59 AM, Simon Horman wrote:
> > Document preference for non inline functions in .c files.
> > This has been the preference for as long as I can recall
> > and I was recently surprised to discover that it is undocumented.
> > 
> > Reported-by: Alexandre Ferrieux <alexandre.ferrieux@gmail.com>
> > Closes: https://lore.kernel.org/all/9662e6fe-cc91-4258-aba1-ab5b016a041a@orange.com/
> > Signed-off-by: Simon Horman <horms@kernel.org>
> > ---
> >  Documentation/process/maintainer-netdev.rst | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/Documentation/process/maintainer-netdev.rst b/Documentation/process/maintainer-netdev.rst
> > index e497729525d5..1fbb8178b8cd 100644
> > --- a/Documentation/process/maintainer-netdev.rst
> > +++ b/Documentation/process/maintainer-netdev.rst
> > @@ -408,6 +408,17 @@ at a greater cost than the value of such clean-ups.
> >  
> >  Conversely, spelling and grammar fixes are not discouraged.
> >  
> > +Inline functions
> > +----------------
> > +
> > +The use of static inline functions in .c file is strongly discouraged
> > +unless there is a demonstrable reason for them, usually performance
> > +related. Rather, it is preferred to omit the inline keyword and allow the
> > +compiler to inline them as it sees fit.
> > +
> > +This is a stricter requirement than that of the general Linux Kernel
> > +:ref:`Coding Style<codingstyle>`
> 
> Is there an ending period (full stop) after that sentence?
> Could/should there be?

Thanks,

I think so. I will add one.