Messages in this thread | | | From | Miguel Ojeda <> | Date | Wed, 6 Feb 2019 22:19:37 +0100 | Subject | Re: [PATCH v2] include/linux/module.h: mark init/cleanup_module aliases as __init/exit |
| |
On Wed, Feb 6, 2019 at 6:56 PM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > diff --git a/include/linux/module.h b/include/linux/module.h > index 8fa38d3e7538..1b5e370f1bc0 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -129,13 +129,13 @@ extern void cleanup_module(void); > #define module_init(initfn) \ > static inline initcall_t __maybe_unused __inittest(void) \ > { return initfn; } \ > - int init_module(void) __attribute__((alias(#initfn))); > + int init_module(void) __init __attribute__((alias(#initfn))); > > /* This is only required if you want to be unloadable. */ > #define module_exit(exitfn) \ > static inline exitcall_t __maybe_unused __exittest(void) \ > { return exitfn; } \ > - void cleanup_module(void) __attribute__((alias(#exitfn))); > + void cleanup_module(void) __exit __attribute__((alias(#exitfn)));
It turns out that there are some modules without __init/__exit marked functions, which GCC complains about, since now the alias is in a different section than the target:
* In some cases, this is due to a missing __init/__exit marking (e.g. drivers/connector/connector.c). These should be fixed in any case. * In other cases, the cleanup function is not marked as such because it is called from another place in the TU, like the init function (e.g. arch/x86/kvm/vmx.c). We would need to create an actual cleanup function (marked as __exit) that simply calls the current exit static function.
So we have a few alternatives:
1) Going only with __cold. 2) Using the new __copy attribute (because then they are copied only in the cases they are actually used). 3) Fix and go for __init/__exit. While this requires some tweaking as explained above, it would be good if we can achieve it since then we are enforcing proper __init/__exit markings for all modules (whereas __copy wouldn't spot).
I think it is worth achieving 3), but that will take a bit more of time. In that case, I suggest we push 1) or 2) for the moment (so that the warning is fixed) and then work on fixing all instances. As soon as that is done, we can push 3).
What do you think?
Cheers, Miguel
| |