Re: [PATCH 01/18] block/rnbd: fix mixed module-builtin object

[Andrew Davis]

On 11/21/22 11:59 PM, Masahiro Yamada wrote:
> On Tue, Nov 22, 2022 at 6:18 AM Andrew Davis <afd@xxxxxx> wrote:
> > On 11/19/22 5:04 PM, Alexander Lobakin wrote:
> > > From: Masahiro Yamada <masahiroy@xxxxxxxxxx>
> > >
> > > With CONFIG_BLK_DEV_RNBD_CLIENT=m and CONFIG_BLK_DEV_RNBD_SERVER=y
> > > (or vice versa), rnbd-common.o is linked to a module and also to
> > > vmlinux even though CFLAGS are different between builtins and modules.
> > >
> > > This is the same situation as fixed by commit 637a642f5ca5 ("zstd:
> > > Fixing mixed module-builtin objects").
> > >
> > > Turn rnbd_access_mode_str() into an inline function.
> >
> > Why inline? All you should need is "static" to keep these internal to
> > each compilation unit. Inline also bloats the object files when the
> > function is called from multiple places. Let the compiler decide when
> > to inline.
> >
> > Andrew
>
>
> Since it is a header file.
>
>
> In header files, "static inline" should be always used.
> Never "static".

My comment was more "why"?

> If a header is included from a C file and there is a function
> that is not used from that C file,
> "static" would emit -Wunused-function warning
> (-Wunused-function is enabled by -Wall, which is the case
> for the kernel build).

Inline still hints to the compiler to inline, causing unneeded
object size bloat. Using "inline" to signal something else (that
the function may be unused) when we already have a flag for that
(__maybe_unused) feels wrong.

Seems this was already debated way back in 2006.. So maybe not
worth revisiting today, but still a cleanup that could be good
to think more about later.

Andrew