Re: [PATCH] include/linux/module.h: mark init/cleanup_module aliases as __cold

[Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>]

On Mon, Feb 4, 2019 at 4:08 PM Jessica Yu <jeyu@kernel.org> wrote:
>
> IMHO I think annotating with __init is more straightforward, instead
> of cherry-picking attributes (we wouldn't know at first glance why the
> aliases are specifically annotated with __cold without looking at git
> history). Plus the actual module init function and alias declarations
> would be consistent. Just looking at the __init attributes:
>
>     #define __init              __section(.init.text) __cold  __latent_entropy __noinitretpoline
>
> __section(.init.text) - alias already has same section ndx as the
> target symbol so this doesn't have any effect.
>
> __latent_entropy - according to commit 0766f788eb7, if this attribute
> is used on a function then the plugin will utilize it for gathering
> entropy (apparently a local variable is created in every marked
> function, the value of which is modified randomly, and before function
> return it will write into the latent_entropy global variable). Module
> init functions are already annotated with this since they are
> annotated with __init, I don't think marking the alias would do any
> harm.
>
> __noinitretpoline - compiled away if the function is in a module and
> not built-in. The alias is not utilized if the module is built-in. So
> this wouldn't apply to the alias.

In that case, there is also the option suggested by Martin: using the
new "copy" attribute which copies all attributes, except those
blacklisted by GCC, at the moment:

    alias, always_inline, gnu_inline, ifunc, noinline, visibility,
    weak, weakref

Since we have the __init macro, there is not much gain in this
instance (but if you prefer the copy alternative, let me know).

> Unfortunately I don't have gcc9 set up on my machine so I can't
> actually test if it gets rid of all the warnings, so testing this
> would be appreciated :)

The warning triggers currently for a subset of attributes only:

    alloc_align, alloc_size, cold, const, hot, leaf, malloc,
    nonnull, noreturn, nothrow, pure, returns_nonnull,
    returns_twice

So the rest of the attributes do not make a difference w.r.t. the warnings.

I will change it to __init then and send the PR after a couple of days
in -next :)

Cheers,
Miguel