Skip to contentSkip to conversation

Module loader preparation

PATCH In review Rusty Russell 3 participants 6 messages Oct 17, 2002 – Oct 18, 2002

Subsystems (4)
Networking Network drivers Char and misc drivers Parallel port
lore
Share & tools
lore

Rusty Russell

author
#0 lore raw

PATCH 1/1 Module loader preparation

This patch fixes a couple of places using the old "I can just call my function init_module() and it will be called at module init" and a couple of modules without module_init() declarations.

These uses are obsolete with the in-kernel module loader, because the module_init() is where we put the module name in the ".modulename" section (we could have a "no_init_func()" thing, but it's fairly rare and hardly intuitive).

Linus, please apply.
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Name: init_module removal
Author: Rusty Russell
Status: Trivial

D: Modules in 2.4 can simply name functions init_module() and
D: cleanup_module(), and they will be called at module load and
D: unload. The preferred way is to use module_init(funcname) and
D: module_exit(funcname), and this is the only way once the
D: in-kernel-module loader is included.
D:
D: This is not all of them, just some examples (the ones I needed to
D: successfully compile).

drivers/net/ethertap.c +6 -2
diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.35/drivers/net/ethertap.c working-2.5.35-modbase-try-i386/drivers/net/ethertap.c
@@ -346,7 +346,7 @@ static struct net_device dev_ethertap =
 	0, 0, 0, NULL, ethertap_probe
 };
 
-int init_module(void)
+static int init(void)
 {
 	dev_ethertap.base_addr=unit+NETLINK_TAPBASE;
 	sprintf(dev_ethertap.name,"tap%d",unit);

@@ -360,7 +360,7 @@ int init_module(void)
 	return 0;
 }
 
-void cleanup_module(void)
+static void cleanup(void)
 {
 	tap_map[dev_ethertap.base_addr]=NULL;
 	unregister_netdev(&dev_ethertap);

@@ -372,6 +372,10 @@ void cleanup_module(void)
 	kfree(dev_ethertap.priv);
 	dev_ethertap.priv = NULL;	/* gets re-allocated by ethertap_probe */
 }
+
+/* FIXME: Remove Space.c hardcoded crap --RR */
+module_init(init);
+module_exit(cleanup);
 
 #endif /* MODULE */
 MODULE_LICENSE("GPL");
drivers/net/slhc.c +5 -2
diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.35/drivers/net/slhc.c working-2.5.35-modbase-try-i386/drivers/net/slhc.c
@@ -735,17 +735,20 @@ EXPORT_SYMBOL(slhc_toss);
 
 #ifdef MODULE
 
-int init_module(void)
+static int init(void)
 {
 	printk(KERN_INFO "CSLIP: code copyright 1989 Regents of the University of California\n");
 	return 0;
 }
 
-void cleanup_module(void)
+static void fini(void)
 {
 	return;
 }
 
+/* FIXME: Remove hard-coded initializers for builtin case */
+module_init(init);
+module_exit(fini);
 #endif /* MODULE */
 #else /* CONFIG_INET */
drivers/parport/init.c +5 -2
diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.35/drivers/parport/init.c working-2.5.35-modbase-try-i386/drivers/parport/init.c
@@ -120,7 +120,7 @@ __setup ("parport=", parport_setup);
 #endif
 
 #ifdef MODULE
-int init_module(void)
+static int init(void)
 {
 #ifdef CONFIG_SYSCTL
 	parport_default_proc_register ();

@@ -128,12 +128,15 @@ int init_module(void)
 	return 0;
 }
 
-void cleanup_module(void)
+static void fini(void)
 {
 #ifdef CONFIG_SYSCTL
 	parport_default_proc_unregister ();
 #endif
 }
+
+module_init(init);
+module_exit(fini);
 
 #else
drivers/parport/parport_pc.c +6 -2
diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.35/drivers/parport/parport_pc.c working-2.5.35-modbase-try-i386/drivers/parport/parport_pc.c
@@ -3069,7 +3069,7 @@ MODULE_PARM_DESC(verbose_probing, "Log c
 MODULE_PARM(verbose_probing, "i");
 #endif
 
-int init_module(void)
+static int init(void)
 {	
 	/* Work out how many ports we have, then get parport_share to parse
 	   the irq values. */

@@ -3118,7 +3118,7 @@ int init_module(void)
 	return ret;
 }
 
-void cleanup_module(void)
+static void fini(void)
 {
 	/* We ought to keep track of which ports are actually ours. */
 	struct parport *p = parport_enumerate(), *tmp;

@@ -3134,4 +3134,8 @@ void cleanup_module(void)
 		p = tmp;
 	}
 }
+
+/* FIXME: Merge builtin and modular initializers --RR */
+module_init(init);
+module_exit(fini);
 #endif
net/netlink/netlink_dev.c +6 -2
diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.35/net/netlink/netlink_dev.c working-2.5.35-modbase-try-i386/net/netlink/netlink_dev.c
@@ -208,16 +208,20 @@ int __init init_netlink(void)
 
 MODULE_LICENSE("GPL");
 
-int init_module(void)
+static int init(void)
 {
 	printk(KERN_INFO "Network Kernel/User communications module 0.04\n");
 	return init_netlink();
 }
 
-void cleanup_module(void)
+static void fini(void)
 {
 	devfs_unregister (devfs_handle);
 	unregister_chrdev(NETLINK_MAJOR, "netlink");
 }
+
+/* FIXME: Remove harded init call in socket.c */
+module_init(init);
+module_exit(fini);
 
 #endif
drivers/char/ftape/zftape/zftape-init.c +8 -0
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .18828-linux-2.5.35/drivers/char/ftape/zftape/zftape-init.c .18828-linux-2.5.35.updated/drivers/char/ftape/zftape/zftape-init.c
@@ -393,20 +393,26 @@ KERN_INFO
 
 
 #ifdef MODULE
+
+#if 0 /* FIXME --RR */
 /* Called by modules package before trying to unload the module
  */
 static int can_unload(void)
 {
 	return (GET_USE_COUNT(THIS_MODULE)||zft_dirty()||test_bit(0,&busy_flag))?-EBUSY:0;
 }
+#endif
+
 /* Called by modules package when installing the driver
  */
 int init_module(void)
 {
+#if 0 /*FIXME --RR*/
 	if (!mod_member_present(&__this_module, can_unload)) {
 		return -EBUSY;
 	}
 	__this_module.can_unload = can_unload;
+#endif
 	return zft_init();
 }
 

@@ -444,3 +450,5 @@ void cleanup_module(void)
 }
 
 #endif /* MODULE */
+
+module_init(init_module);

Name: Every module needs module_init
Author: Rusty Russell
Status: Trivial

D: Every module needs a module_init now (that's when we insert the
D: module name), so insert a trivial one where needed. Most files
D: already have them, and it's a good sanity check that you're dealing
D: with a real module.

lib/zlib_deflate/deflate_syms.c +6 -0
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .17783-2.5.35-modbase-try-i386.pre/lib/zlib_deflate/deflate_syms.c .17783-2.5.35-modbase-try-i386/lib/zlib_deflate/deflate_syms.c
@@ -19,3 +19,9 @@ EXPORT_SYMBOL(zlib_deflateReset);
 EXPORT_SYMBOL(zlib_deflateCopy);
 EXPORT_SYMBOL(zlib_deflateParams);
 MODULE_LICENSE("GPL");
+
+static int init(void)
+{
+	return 0;
+}
+module_init(init);
lib/zlib_inflate/inflate_syms.c +6 -0
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .17783-2.5.35-modbase-try-i386.pre/lib/zlib_inflate/inflate_syms.c .17783-2.5.35-modbase-try-i386/lib/zlib_inflate/inflate_syms.c
@@ -20,3 +20,9 @@ EXPORT_SYMBOL(zlib_inflateReset);
 EXPORT_SYMBOL(zlib_inflateSyncPoint);
 EXPORT_SYMBOL(zlib_inflateIncomp); 
 MODULE_LICENSE("GPL");
+
+static int init(void)
+{
+	return 0;
+}
+module_init(init);
#1 lore raw
Quoted 8 lines · Rusty Russell

This patch fixes a couple of places using the old "I can just call my function init_module() and it will be called at module init" and a couple of modules without module_init() declarations.

These uses are obsolete with the in-kernel module loader, because the module_init() is where we put the module name in the ".modulename" section (we could have a "no_init_func()" thing, but it's fairly rare and hardly intuitive).

I don't think requiring a init func is a good idea. Please fix your module loader to generate a stub if no module_init() is present instead.

init_module() sounds like a good idea to me, though.

author
#2 lore raw

In message <[email protected]> you write:

Quoted 3 lines

I don't think requiring a init func is a good idea. Please fix your module loader to generate a stub if no module_init() is present instead.

But where do you generate the module name (a module now knows its own name, in the .modulename section)? We could do an extra step in the module build where we generate this and link it in, bit given that we're talking about a handful of trivial modules, I don't think it's worth it.

Quoted 1 line

init_module() sounds like a good idea to me, though.

Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

#3 lore raw
In reply to Rusty Russell
Quoted 8 lines · Rusty Russell

This patch fixes a couple of places using the old "I can just call my function init_module() and it will be called at module init" and a couple of modules without module_init() declarations.

These uses are obsolete with the in-kernel module loader, because the module_init() is where we put the module name in the ".modulename" section (we could have a "no_init_func()" thing, but it's fairly rare and hardly intuitive).

Converting things to module_init() is certainly a good thing, but having to provide fake init functions for modules which don't need them doesn't look so nice.

Did you consider just generating the info you need unconditionally in include/linux/module.h and then removing duplicates for multi-part modules using ld's link-once (I didn't try that yet, but it seems doable and would also remove duplicated .modinfo.kernel_version strings and the like)

--Kai

author
#4 lore raw

In message <[email protected]> yo u write:

Quoted 3 lines

Converting things to module_init() is certainly a good thing, but having to provide fake init functions for modules which don't need them doesn't look so nice.

Since there are only a couple, I didn't get exotic.

Quoted 4 lines

Did you consider just generating the info you need unconditionally in include/linux/module.h and then removing duplicates for multi-part modules using ld's link-once (I didn't try that yet, but it seems doable and would also remove duplicated .modinfo.kernel_version strings and the like)

Didn't think of it, to be honest, and I can't find any reference to link-once glancing through the ld info page.

You'll still have problems with objects linked into two modules (ip_conntrack_core being the typical one), but we could ban these and just #include the .c file rather than linking.

Really, the number of modules which do this is so small, the code to add init function to them is less than the change in the build system to get tricky.

Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

#5 lore raw
Quoted 11 lines · Rusty Russell
Quoted 4 lines

Did you consider just generating the info you need unconditionally in include/linux/module.h and then removing duplicates for multi-part modules using ld's link-once (I didn't try that yet, but it seems doable and would also remove duplicated .modinfo.kernel_version strings and the like)

Didn't think of it, to be honest, and I can't find any reference to link-once glancing through the ld info page.

You'll still have problems with objects linked into two modules (ip_conntrack_core being the typical one), but we could ban these and just #include the .c file rather than linking.

Right, they'd be a problem (I'm not sure if having this kind of code sharing is a good idea, generally, but that's another issue).

Quoted 3 lines

Really, the number of modules which do this is so small, the code to add init function to them is less than the change in the build system to get tricky.

I tend to agree, in particular taking into account the problem you mentioned above...

--Kai