Messages in this thread Patch in this message | | | Date | Thu, 7 Feb 2019 11:54:31 +0100 | From | Jessica Yu <> | Subject | Re: [PATCH v2] include/linux/module.h: mark init/cleanup_module aliases as __init/exit |
| |
+++ Miguel Ojeda [06/02/19 22:19 +0100]: >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.
Grr, I guess it was not so simple after all. :)
>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).
My order of preference would be 2, 1, striving for 3 eventually.
Thanks Miguel!
Jessica
| |