Allow arbitrary number of init funcs in modules
Subsystems (8)
PATCH 3/3 Allow arbitrary number of init funcs in modules
This patch requires some explanation, see description fields below.
Thanks to Andrew for prodding, and DaveM for the hard questions.
Feedback is extremely welcome,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
Name: Allow Arbitrary Number of Init and Exit Functions
Author: Rusty Russell
Status: Tested on 2.5.73
Depends: Misc/unique_id.patch.gz
D: One longstanding complaint is that modules can only have one
D: module_init, and one module_exit (builtin code can have multiple
D: __initcall however). This means, for example, that it is not
D: possible to write a "module_proc_entry(name, readfn)" function
D: which can be used like so:
D:
D: module_init(myinitfn);
D: module_cleanup(myinitfn);
D: module_proc_entry("some/path/foo", read_foo);D:
D: The reason we don't allow multiple init functions in modules: if
D: one fails, we won't know what to do. The solution is to explicitly
D: pair them, hence module_init_exit(), with a priority arg. D:
D: If the module fails to load the exit function will be called
D: corresponding to each init function which was called, in backwards
D: order. All exit functions are called on module removal. D:
D: For non-modules, the exit arg is discarded, and the initcalls are
D: run in priority order at boot (order within priorities is still
D: controlled by linking, as previously). This means we can also get
D: rid of the initcall levels at link time and simply use priorities. D:
D: This patch also uses kallsyms to print function names when
D: "initcall_debug" is set.
arch/i386/vmlinux.lds.S +1 -7
@@ -69,13 +69,7 @@ SECTIONS __stop___param = .; __initcall_start = .; .initcall.init : { - *(.initcall1.init) - *(.initcall2.init) - *(.initcall3.init) - *(.initcall4.init) - *(.initcall5.init) - *(.initcall6.init) - *(.initcall7.init) + *(.initcall.init) } __initcall_end = .; __con_initcall_start = .;
include/linux/init.h +55 -39
134 lines not on this page · raw message
include/linux/module.h +4 -10
@@ -203,9 +203,6 @@ struct moduleunsigned int num_exentries;const struct exception_table_entry *extable; -/* Startup function. */ -int (*init)(void ); -/* If this is non-NULL, vfree after init() returns */ void *module_init;
@@ -224,6 +221,10 @@ struct module/* Am I GPL-compatible */ int license_gplok; +/* Pairs of init/exit functions. */ +struct module_init_exit *ie_pairs; +unsigned int num_ie_pairs; +#ifdef CONFIG_MODULE_UNLOAD /* Reference counts */ struct module_ref ref[NR_CPUS ];
@@ -233,9 +234,6 @@ struct module/* Who is waiting for us to be unloaded */ struct task_struct *waiter; - -/* Destruction function. */ -void (*exit)(void );#endif #ifdef CONFIG_KALLSYMS
@@ -449,10 +447,6 @@ extern struct module __this_module; struct module __this_module __attribute__((section(".gnu.linkonce.this_module"))) = { .name = __stringify(KBUILD_MODNAME), - .init = init_module, -#ifdef CONFIG_MODULE_UNLOAD - .exit = cleanup_module, -#endif };#endif /* KBUILD_MODNAME */ #endif /* MODULE */
init/main.c +55 -21
95 lines not on this page · raw message
kernel/module.c +67 -33
181 lines not on this page · raw message
Quoted 1 line
Feedback is extremely welcome,
OK...you asked for it. I found three separate bugs, two of them oopsed the system, and the other prevented module unloading. Did you try it with CONFIG_MODULE_UNLOAD? :) You also need to test for null init functions, because the module_init/module_exit mode creates them. Patch appended.
Also...some guy once posted (http://lwn.net/Articles/22763/):
I appeciate the series in modernizing modules, but just FYI, I don't think the old-style init_module/cleanup_module stuff will break any time soon: there are still a large number of drivers which use it, and there's not much point making such changes.
This patch breaks the old init_module/cleanup_module scheme; those functions no longer get called. Was that intentional?
jon
P.S. Beyond that, I think the patch makes sense :)
Jonathan Corbet
Executive editor, LWN.net
[email protected]
kernel/module.c +6 -3
@@ -617,9 +617,10 @@ {int i, balance =0 ; -for (i =0 ; i < num_pairs; i++) +for (i =0 ; i < num_pairs; i++) { balance += (pairs->init ?1 :0 ) - (pairs->exit ?1 :0 ); - + pairs++; + }return balance ==0 ; }
@@ -643,7 +644,7 @@ {struct module *mod;char name[MODULE_NAME_LEN ]; -unsigned int i; +int i;int ret, forced =0 ;if (!capable (CAP_SYS_MODULE ))
@@ -1725,6 +1726,8 @@/* Start the module */ for (i =0 ; i < mod->num_ie_pairs; i++) { +if (mod->ie_pairs[i].init ==NULL ) +continue ; ret = mod->ie_pairs[i].init ();if (ret !=0 ) { DEBUGP("%s: init/exit pair init=%p failed: %i\n",
In message <[email protected]> you write:
Quoted 6 lines
Quoted 1 line
Feedback is extremely welcome,
OK...you asked for it. I found three separate bugs, two of them oopsed the system, and the other prevented module unloading. Did you try it with
CONFIG_MODULE_UNLOAD? :) You also need to test for null init functions, because the module_init/module_exit mode creates them. Patch appended.
Wow, someone actually tested it! I do sometimes wonder.
It turns out that I tested an older version with modules, and only tested this rewrite with in-built code. Mea culpa.
Quoted 9 lines
Also...some guy once posted (http://lwn.net/Articles/22763/):
I appeciate the series in modernizing modules, but just FYI, I don't think the old-style init_module/cleanup_module stuff will break any time soon: there are still a large number of drivers which use it, and there's not much point making such changes.
This patch breaks the old init_module/cleanup_module scheme; those functions no longer get called. Was that intentional?
<SIGH> I'd forgotten about that; there are about 100 places where this technique is still used. The change would be trivial, but this isn't 2.5.lownum anymore. Workaround incorporated.
Quoted 1 line
P.S. Beyond that, I think the patch makes sense :)
Thanks...
Quoted 5 lines
kernel/module.c +0
-0@@ -617,9 +617,10 @@ {int i, balance =0 ;
kernel/module.c line 626
Quoted 10 lines
kernel/module.c +3
-2-for (i =0 ; i < num_pairs; i++) +for (i =0 ; i < num_pairs; i++) { balance += (pairs->init ?1 :0 ) - (pairs->exit ?1 :0 ); - + pairs++; + }return balance ==0 ; }
I prefer to use pairs[i].init. Rest applied untouched.
I'll test and release a new on soon...
Thanks!
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
In message <[email protected]> you write:
Quoted 3 lines
Quoted 1 line
Feedback is extremely welcome,
OK...you asked for it. I found three separate bugs, two of them oopsed the
OK, patch applied, ancient-style module init supported (after separate patch to clean out some cruft) and init-sorting routine fixed too. Tested on 2.5.73-bk1, test patch included.
For convenience, all patches in one long mail.
Feedback still welcome 8)
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
Name: Centralize token pasting and generation of unique IDs
Author: Rusty Russell
Status: Tested on 2.5.70-bk13
D: Add __cat(a,b) to implement token pasting to stringify.h. To
D: generate unique names, __unique_id(stem) is implemented (it'd be
D: nice to have a gcc extension to give a unique identifier). Change
D: module.h to use them.
include/linux/module.h +1 -3
@@ -55,10 +55,8 @@ search_extable(const struct exception_ta unsigned long value);#ifdef MODULE -#define ___module_cat (a,b) __mod_ ## a ## b -#define __module_cat (a,b) ___module_cat(a,b)#define __MODULE_INFO (tag, name, info) \ -static const char __module_cat(name,__LINE__)[] \ +static const char __unique_id(name)[] \ __attribute__((section(".modinfo"),unused)) = __stringify(tag) "=" info#define MODULE_GENERIC_TABLE (gtype,name) \
include/linux/stringify.h +7 -0
@@ -9,4 +9,11 @@#define __stringify_1 (x) #x#define __stringify (x) __stringify_1(x) +/* Paste two tokens together. */ +#define ___cat (a,b) a ## b +#define __cat (a,b) ___cat(a,b) + +/* Try to give a unique identifier: this comes close, iff used as static. */ +#define __unique_id (stem) \ + __cat(__cat(__uniq,stem),__cat(__LINE__,KBUILD_BASENAME))#endif /* !__LINUX_STRINGIFY_H */
Name: Delete redundant init_module and cleanup_module prototypes.
Author: Rusty Russell
Status: Trivial
D: A few places pre-declare "int module_init(void);" and "void
D: module_cleanup(void);". Other than being obsolete, this is
D: unneccessary (it's in init.h anyway).
D:
D: There are still about 100 places which still use the
D: obsolete-since-2.2 "a function named module_init() magically gets
D: called": this change frees us up implement that via a macro.
drivers/block/paride/pf.c +0 -3
@@ -222,9 +222,6 @@ MODULE_PARM(drive3, "1-7i");#define ATAPI_READ_10 0x28#define ATAPI_WRITE_10 0x2a -#ifdef MODULE -void cleanup_module (void ); -#endif static int pf_open (struct inode *inode,struct file *file);static void do_pf_request (request_queue_t * q); static int pf_ioctl(struct inode *inode, struct file *file,
drivers/char/istallion.c +0 -2
@@ -650,8 +650,6 @@ static unsigned int stli_baudrates[] = {*/ #ifdef MODULE -int init_module (void ); -void cleanup_module (void );static void stli_argbrds (void );static int stli_parsebrd (stlconf_t *confp,char **argp);
drivers/char/moxa.c +1 -4
@@ -216,10 +216,7 @@ static struct timer_list moxaEmptyTimer[static struct semaphore moxaBuffSem;int moxa_init (void ); -#ifdef MODULE -int init_module (void ); -void cleanup_module (void ); -#endif +/* * static functions: */
drivers/char/nwbutton.h +0 -4
@@ -32,10 +32,6 @@ int button_init (void);int button_add_callback (void (*callback) (void ),int count);int button_del_callback (void (*callback) (void ));static void button_consume_callbacks (int bpcount); -#ifdef MODULE -int init_module (void ); -void cleanup_module (void ); -#endif /* MODULE */ #else /* Not compiling the driver itself */
drivers/char/pcxx.c +7 -9
@@ -209,17 +209,9 @@ static void cleanup_board_resources(void#ifdef MODULE -/* -* pcxe_init() is our init_module(): -*/ -#define pcxe_init init_module - -void cleanup_module (void ); - -/*****************************************************************************/ -void cleanup_module () +static void pcxe_cleanup () {unsigned long flags;
@@ -240,6 +232,12 @@ void cleanup_module()kfree (digi_channels); restore_flags(flags); } + +/* +* pcxe_init() is our init_module(): +*/ +module_init (pcxe_init); +module_cleanup (pcxe_cleanup);#endif static inline struct channel *chan (registerstruct tty_struct *tty)
drivers/char/stallion.c +0 -2
@@ -472,8 +472,6 @@ static unsigned int stl_baudrates[] = {*/ #ifdef MODULE -int init_module (void ); -void cleanup_module (void );static void stl_argbrds (void );static int stl_parsebrd (stlconf_t *confp,char **argp);
drivers/net/wan/sdladrv.c +4 -9
@@ -160,10 +160,6 @@/****** Function Prototypes *************************************************/ -/* Module entry points. These are called by the OS and must be public. */ -int init_module (void ); -void cleanup_module (void ); -/* Hardware-specific functions */ static int sdla_detect (sdlahw_t * hw);static int sdla_autodpm (sdlahw_t * hw);
@@ -325,11 +321,7 @@ static int pci_slot_ar[MAX_S514_CARDS];* Context: process */ -#ifdef MODULE -int init_module (void) -#else int sdladrv_init (void ) -#endif {int i=0 ;
@@ -354,9 +346,12 @@ int sdladrv_init(void)* Module 'remove' entry point. * o release all remaining system resources */ -void cleanup_module (void ) +static void sdladrv_cleanup (void ) { } + +module_init (sdladrv_init); +module_cleanup (sdladrv_cleanup);#endif /******* Kernel APIs ********************************************************/
drivers/net/wan/sdlamain.c +3 -9
@@ -177,10 +177,6 @@ static void dbg_kfree(void * v, int lineextern void disable_irq (unsigned int );extern void enable_irq (unsigned int ); -/* Module entry points */ -int init_module (void ); -void cleanup_module (void ); -/* WAN link driver entry points */ static int setup (struct wan_device * wandev,wandev_conf_t * conf);static int shutdown (struct wan_device * wandev);
@@ -246,11 +242,7 @@ static int wanpipe_bh_critical=0;* Context: process */ -#ifdef MODULE -int init_module (void) -#else int wanpipe_init (void ) -#endif {int cnt, err =0 ;
@@ -313,7 +305,7 @@ int wanpipe_init(void)* o unregister all adapters from the WAN router * o release all remaining system resources */ -void cleanup_module (void ) +static void wanpipe_cleanup (void ) {int i;
@@ -329,6 +321,8 @@ void cleanup_module (void) printk(KERN_INFO "\nwanpipe: WANPIPE Modules Unloaded.\n"); } +module_init (wanpipe_init); +module_exit (wanpipe_cleanup);#endif /******* WAN Device Driver Entry Points *************************************/
Name: Eliminate Unused Functions
Author: Rusty Russell
Status: Tested on 2.5.70-bk16
D: GCC 3.3 has the ability to eliminate unused static functions. This includes
D: code like this:
D:
D: static int unusedfunc(void) { ... };
D: int otherfunc(void)
D: {
D: (void)unusedfunc;
D: ...
D:
D: This means that macros can suppress the "unused" warning on functions
D: without preventing the function elimination. This should allow us to
D: remove a number of #ifdefs around unused functions.
D:
D: Unfortunately, this elimination is only performed if
D: -finline-functions is used. In order to prevent GCC automatically
D: inlining anything, we also specify "--param max-inline-insns-auto=0". D:
D: Earlier compilers don't understand this parameter, so we test for
D: it at build time.
D:
D: Results: D: gcc 3.3 without patch: D: -rwxrwxr-x 1 rusty rusty 5115166 Jun 13 09:17 vmlinux D: gcc 3.3 with patch: D: -rwxrwxr-x 1 rusty rusty 5115166 Jun 13 09:58 vmlinux D: gcc 3.3 without patch (small unused function added): D: -rwxrwxr-x 1 rusty rusty 5115195 Jun 13 10:14 vmlinux D: gcc 3.3 with patch (small unused function added): D: -rwxrwxr-x 1 rusty rusty 5115166 Jun 13 10:15 vmlinux
Makefile +3 -1
@@ -213,10 +213,12 @@ CFLAGS_KERNEL = AFLAGS_KERNEL = NOSTDINC_FLAGS = -nostdinc -iwithprefix include +# Needs gcc 3.3 or above to understand max-inline-insns-auto. +INLINE_OPTS := $(shell $(CC) -o /non/existent/file -c --param max-inline-insns-auto=0 -xc /dev/null 2>&1 | grep /non/existent/file >/dev/null && echo -finline-functions --param max-inline-insns-auto=0) CPPFLAGS := -D__KERNEL__ -Iinclude CFLAGS := $(CPPFLAGS) -Wall -Wstrict-prototypes -Wno-trigraphs -O2 \ - -fno-strict-aliasing -fno-common + $(INLINE_OPTS) -fno-strict-aliasing -fno-common AFLAGS := -D__ASSEMBLY__ $(CPPFLAGS) export VERSION PATCHLEVEL SUBLEVEL EXTRAVERSION KERNELRELEASE ARCH \
Name: Test Arbitrary Number of Init and Exit Functions
Author: Rusty Russell
Status: Tested on 2.5.73-bk1
Depends: Module/init_exit.patch.gz
D: Test code for module_init_exit: adds example helpers for proc_net
D: and netfilter, and uses them in netfilter. Also adds module foo
D: which uses really old-style init.
include/linux/netfilter.h +21 -0
@@ -9,6 +9,8 @@#include <linux/if.h> #include <linux/wait.h> #include <linux/list.h> +#include <linux/stringify.h> +#include <linux/module.h> #endif /* Responses from hook functions. */
@@ -91,6 +93,21 @@ struct nf_infoint nf_register_hook (struct nf_hook_ops *reg);void nf_unregister_hook (struct nf_hook_ops *reg); +/* Automatically register/unregister */ +#define module_nf_hook (_prio, _hookfn, _pf, _hooknum, _hookprio) \ + static struct nf_hook_ops __cat(nf_hook, _hookfn) \ + = { .hook=_hookfn, .owner=THIS_MODULE, .pf=_pf, .hooknum=_hooknum, \ + .priority=_hookprio }; \ + static int __init __cat(nfh_reg, _hookfn)(void) \ + { \ + return nf_register_hook(&__cat(nf_hook, _hookfn)); \ + } \ + static void __init __cat(nfh_unreg, _hookfn)(void) \ + { \ + nf_unregister_hook(&__cat(nf_hook, _hookfn)); \ + } \ + module_init_exit(_prio, __cat(nfh_reg, _hookfn), __cat(nfh_unreg, _hookfn)) +/* Functions to register get/setsockopt ranges (non-inclusive). You need to check permissions yourself! */ int nf_register_sockopt (struct nf_sockopt_ops *reg);
@@ -163,6 +180,10 @@ extern void nf_invalidate_cache(int pf);#else /* !CONFIG_NETFILTER */ #define NF_HOOK (pf, hook, skb, indev, outdev, okfn) (okfn)(skb) + +/* Just check type and suppress unused message. */ +#define module_nf_hook (prio, hookfn, pf, hooknum, hookprio) \ +static inline nf_hookfn *__unique_id(hookfn) { return hookfn; }#endif /*CONFIG_NETFILTER*/ #endif /*__KERNEL__*/
include/linux/proc_fs.h +19 -0
@@ -4,6 +4,7 @@#include <linux/config.h> #include <linux/slab.h> #include <linux/fs.h> +#include <linux/init.h> #include <asm/atomic.h> /*
@@ -191,6 +192,20 @@ static inline void proc_net_remove(constextern void kclist_add (struct kcore_list *,void *,size_t );extern struct kcore_list *kclist_del (void *); +#define module_proc_net (prio, name, fn) \ +static int __cat(_proc_net_init,fn)(void) \ +{ \ + struct proc_dir_entry *proc = proc_net_create(name, 0, fn); \ + if (!proc) \ + return -EINVAL; \ + proc->owner = THIS_MODULE; \ + return 0; \ +} \ +static void __cat(_proc_net_exit,fn)(void) \ +{ \ + proc_net_remove(name); \ +} \ +module_init_exit(prio, __cat(_proc_net_init,fn), __cat(_proc_net_exit,fn))#else #define proc_root_driver NULL
@@ -236,6 +251,10 @@ static inline struct kcore_list * kclist return NULL; } +/* Check type and stop gcc from complaining about unused function, but +* allow gcc 3.3+ to discard it. */ +#define module_proc_net (prio, name, fn) \ +static inline get_info_t *__unique_id(test_fntype)(void) { return fn; }#endif /* CONFIG_PROC_FS */ struct proc_inode {
net/ipv4/Makefile +1 -0
@@ -23,3 +23,4 @@ obj-$(CONFIG_IP_PNP) += ipconfig.o obj-$(CONFIG_NETFILTER) += netfilter/ obj-y += xfrm4_policy.o xfrm4_state.o xfrm4_input.o xfrm4_tunnel.o +obj-m += foo.o
net/ipv4/foo.c +42 -0
42 lines not on this page · raw message
net/ipv4/netfilter/ip_conntrack_standalone.c +14 -102
134 lines not on this page · raw message
net/ipv4/netfilter/ip_tables.c +2 -19
@@ -1702,7 +1702,6 @@ static struct ipt_match icmp_matchstruct .checkentry = &icmp_checkentry, }; -#ifdef CONFIG_PROC_FS static inline int print_name (const struct ipt_table *t,off_t start_offset,char *buffer,int length,off_t *pos,unsigned int *count)
@@ -1737,13 +1736,13 @@ static int ipt_get_tables(char *buffer, *start=(char *)((unsigned long )count-offset);return pos; } -#endif /*CONFIG_PROC_FS*/ + +module_proc_net (-1 ,"ip_tables_names" , ipt_get_tables);static int __init init (void ) {int ret; -/* Noone else will be downing sem now, so we won't sleep */ down (&ipt_mutex);list_append (&ipt_target, &ipt_standard_target);list_append (&ipt_target, &ipt_error_target);
@@ -1759,19 +1758,6 @@ static int __init init(void)return ret; } -#ifdef CONFIG_PROC_FS - { -struct proc_dir_entry *proc; - - proc =proc_net_create ("ip_tables_names" ,0 , ipt_get_tables); -if (!proc) { -nf_unregister_sockopt (&ipt_sockopts); -return -ENOMEM ; - } - proc->owner =THIS_MODULE ; - } -#endif -printk ("ip_tables: (C) 2000-2002 Netfilter core team\n" );return 0 ; }
@@ -1779,9 +1765,6 @@ static int __init init(void) static void __exit fini(void) {nf_unregister_sockopt (&ipt_sockopts); -#ifdef CONFIG_PROC_FS -proc_net_remove ("ip_tables_names" ); -#endif }EXPORT_SYMBOL (ipt_register_table);
kernel/module.c line 623
Quoted 12 lines · [email protected] (Jonathan Corbet) said:
kernel/module.c +3
-2@@ -617,9 +617,10 @@ {int i, balance =0 ; -for (i =0 ; i < num_pairs; i++) +for (i =0 ; i < num_pairs; i++) { balance += (pairs->init ?1 :0 ) - (pairs->exit ?1 :0 ); - + pairs++; + }
Hummm... as you are essentially counting off pairs, isn't there a way of detecting end-of-pairs, i.e. along the line (last points to NULL):
for(pairs = start_of_pairs; *pairs; pairs++)
....
or just:
for(i = 0; i < num_pairs; i++) balance += (pairs[i].init ? 1 : 0) - (pairs[i].exit ? 1 : 0);
(as added bonus doesn't screw up variable pairs' value), or even (same as yours above but marginally clearer IMEHO):
for (i = 0; i < num_pairs; i++, pairs++)
.....Signature · 4 lines
Dr. Horst H. von Brand User #22616 counter.li.org Departamento de Informatica Fono: +56 32 654431 Universidad Tecnica Federico Santa Maria +56 32 654239 Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513
Hi,
Quoted 9 lines · Rusty Russell
D: One longstanding complaint is that modules can only have one
D:module_init, and onemodule_exit(builtin code can have multiple
D:__initcallhowever). This means, for example, that it is not
D: possible to write a "module_proc_entry(name, readfn)" function
D: which can be used like so:
D:D: module_init(myinitfn); D: module_cleanup(myinitfn); D: module_proc_entry("some/path/foo", read_foo);
What happens if a module is compiled into the kernel and one of the init functions fails?
bye, Roman
In message <Pine.LNX.4.44.0306241851550.11817-100000@serv> you write:
Quoted 14 lines
Hi,
Quoted 9 lines · Rusty Russell
D: One longstanding complaint is that modules can only have one
D:module_init, and onemodule_exit(builtin code can have multiple
D:__initcallhowever). This means, for example, that it is not
D: possible to write a "module_proc_entry(name, readfn)" function
D: which can be used like so:
D:D: module_init(myinitfn); D: module_cleanup(myinitfn); D: module_proc_entry("some/path/foo", read_foo);
Quoted 2 lines
What happens if a module is compiled into the kernel and one of the init functions fails?
We ignore the failure, as we do with initcalls at the moment. I wasn't really intending to deprecate the existing mechanisms: this is simple at least 8)
Hmm, were you thinking of grouping by KBUILD_BASENAME? Can you think of a case where that would be nicer to use?
Thanks!
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
Hi,
Quoted 9 lines · Rusty Russell
Quoted 2 lines
What happens if a module is compiled into the kernel and one of the init functions fails?
We ignore the failure, as we do with initcalls at the moment. I wasn't really intending to deprecate the existing mechanisms: this is simple at least 8)
Hmm, were you thinking of grouping by
KBUILD_BASENAME? Can you think of a case where that would be nicer to use?
It's not a case of nicer to use, it's about consistent behaviour independent of how the module is linked into the kernel. I'm really not convinced this feature is a good idea until the module initialization races are not at least basically solved.
bye, Roman