* [PATCH 3/3] Allow arbitrary number of init funcs in modules
@ 2003-06-23 9:19 Rusty Russell
2003-06-23 19:11 ` Jonathan Corbet
2003-06-24 17:01 ` Roman Zippel
0 siblings, 2 replies; 8+ messages in thread
From: Rusty Russell @ 2003-06-23 9:19 UTC (permalink / raw)
To: torvalds, akpm, davem; +Cc: linux-kernel, mochel
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.
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .15772-linux-2.5.73/arch/i386/vmlinux.lds.S .15772-linux-2.5.73.updated/arch/i386/vmlinux.lds.S
--- .15772-linux-2.5.73/arch/i386/vmlinux.lds.S 2003-06-15 11:29:47.000000000 +1000
+++ .15772-linux-2.5.73.updated/arch/i386/vmlinux.lds.S 2003-06-23 18:10:41.000000000 +1000
@@ -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 = .;
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .15772-linux-2.5.73/include/linux/init.h .15772-linux-2.5.73.updated/include/linux/init.h
--- .15772-linux-2.5.73/include/linux/init.h 2003-06-15 11:30:08.000000000 +1000
+++ .15772-linux-2.5.73.updated/include/linux/init.h 2003-06-23 18:17:22.000000000 +1000
@@ -2,6 +2,7 @@
#define _LINUX_INIT_H
#include <linux/config.h>
+#include <linux/stringify.h>
/* These macros are used to mark some functions or
* initialized data (doesn't apply to uninitialized data)
@@ -65,34 +66,48 @@ typedef void (*exitcall_t)(void);
extern initcall_t __con_initcall_start, __con_initcall_end;
extern initcall_t __security_initcall_start, __security_initcall_end;
+
+struct module_init_exit
+{
+ int prio;
+ initcall_t init;
+ exitcall_t exit;
+};
+
+struct kernel_init
+{
+ int prio;
+ initcall_t init;
+};
#endif
-
+
#ifndef MODULE
#ifndef __ASSEMBLY__
+/* Suppress unused warning on exitfn, and test type. */
+#define __exitcall(fn) \
+static inline exitcall_t __unique_id(exit_test)(void) { return fn; }
-/* initcalls are now grouped by functionality into separate
- * subsections. Ordering inside the subsections is determined
- * by link order.
- * For backwards compatibility, initcall() puts the call in
- * the device init subsection.
- */
-
-#define __define_initcall(level,fn) \
- static initcall_t __initcall_##fn __attribute__ ((unused,__section__ (".initcall" level ".init"))) = fn
-
-#define core_initcall(fn) __define_initcall("1",fn)
-#define postcore_initcall(fn) __define_initcall("2",fn)
-#define arch_initcall(fn) __define_initcall("3",fn)
-#define subsys_initcall(fn) __define_initcall("4",fn)
-#define fs_initcall(fn) __define_initcall("5",fn)
-#define device_initcall(fn) __define_initcall("6",fn)
-#define late_initcall(fn) __define_initcall("7",fn)
+/* Pair of calls: one called at boot, one at exit (ie. never, when not
+ a module). Unlike module_init, you can have any number of these.
+ prio controls ordering: negative runs before module_init, positive after.
+ Within a priority, link order rules.
+*/
+#define module_init_exit(prio, initfn, exitfn) \
+ __exitcall(exitfn); \
+ static struct kernel_init __initcall_##initfn \
+ __attribute__((unused,__section__ (".initcall.init"))) \
+ = { prio, initfn }
-#define __initcall(fn) device_initcall(fn)
+#define core_initcall(fn) module_init_exit(-5000, fn, NULL)
+#define postcore_initcall(fn) module_init_exit(-4000, fn, NULL)
+#define arch_initcall(fn) module_init_exit(-3000, fn, NULL)
+#define subsys_initcall(fn) module_init_exit(-2000, fn, NULL)
+#define fs_initcall(fn) module_init_exit(-1000, fn, NULL)
+#define device_initcall(fn) module_init_exit(0, fn, NULL)
+#define late_initcall(fn) module_init_exit(1000, fn, NULL)
-#define __exitcall(fn) \
- static exitcall_t __exitcall_##fn __exit_call = fn
+#define __initcall(fn) device_initcall(fn)
#define console_initcall(fn) \
static initcall_t __initcall_##fn __attribute__ ((unused,__section__ (".con_initcall.init")))=fn
@@ -112,8 +127,6 @@ struct obs_kernel_param {
__attribute__((unused,__section__ (".init.setup"))) \
= { __setup_str_##fn, fn }
-#endif /* __ASSEMBLY__ */
-
/**
* module_init() - driver initialization entry point
* @x: function to be run at kernel boot time or module insertion
@@ -135,6 +148,7 @@ struct obs_kernel_param {
* There can only be one per module.
*/
#define module_exit(x) __exitcall(x);
+#endif /* __ASSEMBLY__ */
#else /* MODULE */
@@ -149,25 +163,27 @@ struct obs_kernel_param {
#define security_initcall(fn) module_init(fn)
-/* These macros create a dummy inline: gcc 2.9x does not count alias
- as usage, hence the `unused function' warning when __init functions
- are declared static. We use the dummy __*_module_inline functions
- both to kill the warning and check the type of the init/cleanup
- function. */
-
-/* Each module must use one module_init(), or one no_module_init */
-#define module_init(initfn) \
- static inline initcall_t __inittest(void) \
- { return initfn; } \
- int init_module(void) __attribute__((alias(#initfn)));
+/* Each module can have one module_init(). */
+#define module_init(initfn) \
+ static struct module_init_exit module_init \
+ __attribute__ ((unused,__section__ ("__initexit"))) \
+ = { 0, initfn, NULL };
-/* This is only required if you want to be unloadable. */
-#define module_exit(exitfn) \
- static inline exitcall_t __exittest(void) \
- { return exitfn; } \
- void cleanup_module(void) __attribute__((alias(#exitfn)));
+/* If you have a module_init, and want to be unloadable, you need this too. */
+#define module_exit(exitfn) \
+ static struct module_init_exit module_exit \
+ __attribute__ ((unused,__section__ ("__initexit"))) \
+ = { 0, NULL, exitfn };
#define __setup(str,func) /* nothing */
+
+/* Pair of calls: one called at init, one at exit. Unlike
+ module_init/module_exit, you can have any number of these: ordering is
+ controlled by priority, in ascending order: module_init is 0. */
+#define module_init_exit(prio, initfn, exitfn) \
+ static struct module_init_exit __unique_id(mie) \
+ __attribute__ ((unused,__section__ ("__initexit"))) \
+ = { prio, initfn, exitfn }
#endif
/* Data marked not to be saved by software_suspend() */
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .15772-linux-2.5.73/include/linux/module.h .15772-linux-2.5.73.updated/include/linux/module.h
--- .15772-linux-2.5.73/include/linux/module.h 2003-06-23 17:29:39.000000000 +1000
+++ .15772-linux-2.5.73.updated/include/linux/module.h 2003-06-23 17:29:42.000000000 +1000
@@ -203,9 +203,6 @@ struct module
unsigned 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 */
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .15772-linux-2.5.73/init/main.c .15772-linux-2.5.73.updated/init/main.c
--- .15772-linux-2.5.73/init/main.c 2003-06-17 16:57:33.000000000 +1000
+++ .15772-linux-2.5.73.updated/init/main.c 2003-06-23 18:51:34.000000000 +1000
@@ -38,6 +38,7 @@
#include <linux/moduleparam.h>
#include <linux/writeback.h>
#include <linux/cpu.h>
+#include <linux/kallsyms.h>
#include <asm/io.h>
#include <asm/bugs.h>
@@ -478,34 +479,67 @@ __setup("initcall_debug", initcall_debug
struct task_struct *child_reaper = &init_task;
-extern initcall_t __initcall_start, __initcall_end;
-
-static void __init do_initcalls(void)
+static void do_initcall(initcall_t call)
{
- initcall_t *call;
+ char *msg = NULL;
+ char buf[128];
+ char *m;
+ unsigned long d;
int count = preempt_count();
- for (call = &__initcall_start; call < &__initcall_end; call++) {
- char *msg;
+ if (initcall_debug)
+ printk("calling initcall 0x%p %s\n", *call,
+ kallsyms_lookup((long)call, &d, &d, &m, buf) ?: "");
+ call();
- if (initcall_debug)
- printk("calling initcall 0x%p\n", *call);
+ if (preempt_count() != count) {
+ msg = "preemption imbalance";
+ preempt_count() = count;
+ }
+ if (irqs_disabled()) {
+ msg = "disabled interrupts";
+ local_irq_enable();
+ }
+ if (msg) {
+ printk("error in initcall at 0x%p %s: "
+ "returned with %s\n", *call,
+ kallsyms_lookup((long)call, &d, &d, &m, buf) ?: "",
+ msg);
+ }
+}
- (*call)();
+/* Defined by linker magic. */
+extern struct kernel_init __initcall_start[], __initcall_end[];
- msg = NULL;
- if (preempt_count() != count) {
- msg = "preemption imbalance";
- preempt_count() = count;
- }
- if (irqs_disabled()) {
- msg = "disabled interrupts";
- local_irq_enable();
- }
- if (msg) {
- printk("error in initcall at 0x%p: "
- "returned with %s\n", *call, msg);
+/* Find first initfunc minimally >= this prio level. */
+static struct kernel_init *find_next_prio(int min_prio_todo)
+{
+ struct kernel_init *i, *best;
+
+ best = NULL;
+ for (i = __initcall_start; i < __initcall_end; i++) {
+ if (i->prio >= min_prio_todo
+ && (!best || i->prio < best->prio))
+ best = i;
+ }
+ return best;
+}
+
+static void __init do_initcalls(void)
+{
+ struct kernel_init *next, *i;
+ int min_prio_todo = INT_MIN;
+
+ /* This is O(num calls * num levels), which is OK. */
+ while ((next = find_next_prio(min_prio_todo)) != NULL) {
+ if (initcall_debug)
+ printk("Doing priority %i initcalls:\n", next->prio);
+ /* Now call all at that priority. */
+ for (i = next; i < __initcall_end; i++) {
+ if (i->prio == next->prio)
+ do_initcall(i->init);
}
+ min_prio_todo = next->prio+1;
}
/* Make sure there is no pending stuff from the initcall sequence */
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .15772-linux-2.5.73/kernel/module.c .15772-linux-2.5.73.updated/kernel/module.c
--- .15772-linux-2.5.73/kernel/module.c 2003-06-15 11:30:11.000000000 +1000
+++ .15772-linux-2.5.73.updated/kernel/module.c 2003-06-23 17:29:42.000000000 +1000
@@ -91,13 +91,6 @@ static inline int strong_try_module_get(
return try_module_get(mod);
}
-/* Stub function for modules which don't have an initfn */
-int init_module(void)
-{
- return 0;
-}
-EXPORT_SYMBOL(init_module);
-
/* Find a module section: 0 means not found. */
static unsigned int find_sec(Elf_Ehdr *hdr,
Elf_Shdr *sechdrs,
@@ -619,11 +612,16 @@ static inline int try_force(unsigned int
}
#endif /* CONFIG_MODULE_FORCE_UNLOAD */
-/* Stub function for modules which don't have an exitfn */
-void cleanup_module(void)
+/* Need as many exits as inits to unload. */
+static int can_unload(unsigned int num_pairs, struct module_init_exit *pairs)
{
+ int i, balance = 0;
+
+ for (i = 0; i < num_pairs; i++)
+ balance += (pairs->init ? 1 : 0) - (pairs->exit ? 1 : 0);
+
+ return balance == 0;
}
-EXPORT_SYMBOL(cleanup_module);
static void wait_for_zero_refcount(struct module *mod)
{
@@ -645,6 +643,7 @@ sys_delete_module(const char __user *nam
{
struct module *mod;
char name[MODULE_NAME_LEN];
+ unsigned int i;
int ret, forced = 0;
if (!capable(CAP_SYS_MODULE))
@@ -678,9 +677,8 @@ sys_delete_module(const char __user *nam
goto out;
}
- /* If it has an init func, it must have an exit func to unload */
- if ((mod->init != init_module && mod->exit == cleanup_module)
- || mod->unsafe) {
+ /* Init and exit functions must balance to unload. */
+ if (!can_unload(mod->num_ie_pairs, mod->ie_pairs) || mod->unsafe) {
forced = try_force(flags);
if (!forced) {
/* This module can't be removed */
@@ -713,8 +711,10 @@ sys_delete_module(const char __user *nam
if (!forced && module_refcount(mod) != 0)
wait_for_zero_refcount(mod);
- /* Final destruction now noone is using it. */
- mod->exit();
+ /* Final destruction now noone is using it, reverse priority order. */
+ for (i = mod->num_ie_pairs - 1; i >= 0; i--)
+ if (mod->ie_pairs[i].exit)
+ mod->ie_pairs[i].exit();
free_module(mod);
out:
@@ -741,7 +741,7 @@ static void print_unload_info(struct seq
seq_printf(m, "[unsafe],");
}
- if (mod->init != init_module && mod->exit == cleanup_module) {
+ if (!can_unload(mod->num_ie_pairs, mod->ie_pairs)) {
printed_something = 1;
seq_printf(m, "[permanent],");
}
@@ -1351,6 +1351,23 @@ static void add_kallsyms(struct module *
}
#endif
+/* Order init_exit pairs in ascending priority. num is small. */
+static void sort_ie_pairs(struct module_init_exit *ie_pairs, unsigned int num)
+{
+ unsigned int i, j;
+ struct module_init_exit tmp;
+
+ for (i = 0; i < num; i++) {
+ for (j = i; j+1 < num; j++) {
+ if (ie_pairs[j].prio > ie_pairs[j+1].prio) {
+ tmp = ie_pairs[j];
+ ie_pairs[j] = ie_pairs[j+1];
+ ie_pairs[j+1] = tmp;
+ }
+ }
+ }
+}
+
/* Allocate and load the module: note that size of section 0 is always
zero, and we rely on this for optional sections. */
static struct module *load_module(void __user *umod,
@@ -1362,7 +1379,7 @@ static struct module *load_module(void _
char *secstrings, *args, *modmagic, *strtab = NULL;
unsigned int i, symindex = 0, strindex = 0, setupindex, exindex,
exportindex, modindex, obsparmindex, infoindex, gplindex,
- crcindex, gplcrcindex, versindex, pcpuindex;
+ crcindex, gplcrcindex, versindex, pcpuindex, iepairindex;
long arglen;
struct module *mod;
long err = 0;
@@ -1437,6 +1454,7 @@ static struct module *load_module(void _
obsparmindex = find_sec(hdr, sechdrs, secstrings, "__obsparm");
versindex = find_sec(hdr, sechdrs, secstrings, "__versions");
infoindex = find_sec(hdr, sechdrs, secstrings, ".modinfo");
+ iepairindex = find_sec(hdr, sechdrs, secstrings, "__initexit");
pcpuindex = find_pcpusec(hdr, sechdrs, secstrings);
/* Don't keep modinfo section */
@@ -1587,6 +1605,12 @@ static struct module *load_module(void _
mod->num_exentries = sechdrs[exindex].sh_size / sizeof(*mod->extable);
mod->extable = (void *)sechdrs[exindex].sh_addr;
+ /* Set up init/exit pair table, and sort into prio order. */
+ mod->num_ie_pairs = sechdrs[iepairindex].sh_size
+ / sizeof(struct module_init_exit);
+ mod->ie_pairs = (void *)sechdrs[iepairindex].sh_addr;
+ sort_ie_pairs(mod->ie_pairs, mod->num_ie_pairs);
+
/* Now do relocations. */
for (i = 1; i < hdr->e_shnum; i++) {
const char *strtab = (char *)sechdrs[strindex].sh_addr;
@@ -1661,7 +1685,7 @@ sys_init_module(void __user *umod,
const char __user *uargs)
{
struct module *mod;
- int ret;
+ int i, ret;
/* Must have permission */
if (!capable(CAP_SYS_MODULE))
@@ -1700,22 +1724,13 @@ sys_init_module(void __user *umod,
up(¬ify_mutex);
/* Start the module */
- ret = mod->init();
- if (ret < 0) {
- /* Init routine failed: abort. Try to protect us from
- buggy refcounters. */
- mod->state = MODULE_STATE_GOING;
- synchronize_kernel();
- if (mod->unsafe)
- printk(KERN_ERR "%s: module is now stuck!\n",
- mod->name);
- else {
- module_put(mod);
- down(&module_mutex);
- free_module(mod);
- up(&module_mutex);
+ for (i = 0; i < mod->num_ie_pairs; i++) {
+ ret = mod->ie_pairs[i].init();
+ if (ret != 0) {
+ DEBUGP("%s: init/exit pair init=%p failed: %i\n",
+ mod->name, mod->ie_pairs[i].init, ret);
+ goto unwind_iepairs;
}
- return ret;
}
/* Now it's a first class citizen! */
@@ -1729,6 +1744,25 @@ sys_init_module(void __user *umod,
up(&module_mutex);
return 0;
+
+unwind_iepairs:
+ /* Init routine failed: abort. Try to protect us from
+ buggy refcounters. */
+ mod->state = MODULE_STATE_GOING;
+ module_put(mod);
+ synchronize_kernel();
+ if (module_refcount(mod)) {
+ printk(KERN_ERR "%s: module is now stuck!\n", mod->name);
+ return ret;
+ }
+
+ down(&module_mutex);
+ while (--i >= 0)
+ if (mod->ie_pairs[i].exit)
+ mod->ie_pairs[i].exit();
+ free_module(mod);
+ up(&module_mutex);
+ return ret;
}
static inline int within(unsigned long addr, void *start, unsigned long size)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] Allow arbitrary number of init funcs in modules
2003-06-23 9:19 [PATCH 3/3] Allow arbitrary number of init funcs in modules Rusty Russell
@ 2003-06-23 19:11 ` Jonathan Corbet
2003-06-24 2:29 ` Rusty Russell
` (2 more replies)
2003-06-24 17:01 ` Roman Zippel
1 sibling, 3 replies; 8+ messages in thread
From: Jonathan Corbet @ 2003-06-23 19:11 UTC (permalink / raw)
To: Rusty Russell; +Cc: linux-kernel
> 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
corbet@lwn.net
--- 2.5.73-rr/kernel/module.c Tue Jun 24 02:58:32 2003
+++ 2.5.73/kernel/module.c Tue Jun 24 03:00:36 2003
@@ -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",
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] Allow arbitrary number of init funcs in modules
2003-06-23 19:11 ` Jonathan Corbet
@ 2003-06-24 2:29 ` Rusty Russell
2003-06-24 6:57 ` Rusty Russell
2003-06-24 20:06 ` Horst von Brand
2 siblings, 0 replies; 8+ messages in thread
From: Rusty Russell @ 2003-06-24 2:29 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: linux-kernel
In message <20030623191141.31814.qmail@eklektix.com> you write:
> > 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.
> 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.
> P.S. Beyond that, I think the patch makes sense :)
Thanks...
> --- 2.5.73-rr/kernel/module.c Tue Jun 24 02:58:32 2003
> +++ 2.5.73/kernel/module.c Tue Jun 24 03:00:36 2003
> @@ -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;
> }
>
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] Allow arbitrary number of init funcs in modules
2003-06-23 19:11 ` Jonathan Corbet
2003-06-24 2:29 ` Rusty Russell
@ 2003-06-24 6:57 ` Rusty Russell
2003-06-24 20:06 ` Horst von Brand
2 siblings, 0 replies; 8+ messages in thread
From: Rusty Russell @ 2003-06-24 6:57 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: linux-kernel
In message <20030623191141.31814.qmail@eklektix.com> you write:
> > 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.
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .26569-linux-2.5.70-bk16/include/linux/module.h .26569-linux-2.5.70-bk16.updated/include/linux/module.h
--- .26569-linux-2.5.70-bk16/include/linux/module.h 2003-06-12 09:58:02.000000000 +1000
+++ .26569-linux-2.5.70-bk16.updated/include/linux/module.h 2003-06-12 16:19:16.000000000 +1000
@@ -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) \
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .26569-linux-2.5.70-bk16/include/linux/stringify.h .26569-linux-2.5.70-bk16.updated/include/linux/stringify.h
--- .26569-linux-2.5.70-bk16/include/linux/stringify.h 2003-01-02 12:25:36.000000000 +1100
+++ .26569-linux-2.5.70-bk16.updated/include/linux/stringify.h 2003-06-12 16:32:17.000000000 +1000
@@ -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.
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .11903-linux-2.5.73-bk1/drivers/block/paride/pf.c .11903-linux-2.5.73-bk1.updated/drivers/block/paride/pf.c
--- .11903-linux-2.5.73-bk1/drivers/block/paride/pf.c 2003-05-05 12:36:58.000000000 +1000
+++ .11903-linux-2.5.73-bk1.updated/drivers/block/paride/pf.c 2003-06-24 11:43:58.000000000 +1000
@@ -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,
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .11903-linux-2.5.73-bk1/drivers/char/istallion.c .11903-linux-2.5.73-bk1.updated/drivers/char/istallion.c
--- .11903-linux-2.5.73-bk1/drivers/char/istallion.c 2003-06-15 11:29:51.000000000 +1000
+++ .11903-linux-2.5.73-bk1.updated/drivers/char/istallion.c 2003-06-24 11:43:58.000000000 +1000
@@ -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);
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .11903-linux-2.5.73-bk1/drivers/char/moxa.c .11903-linux-2.5.73-bk1.updated/drivers/char/moxa.c
--- .11903-linux-2.5.73-bk1/drivers/char/moxa.c 2003-06-23 10:52:46.000000000 +1000
+++ .11903-linux-2.5.73-bk1.updated/drivers/char/moxa.c 2003-06-24 11:43:58.000000000 +1000
@@ -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:
*/
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .11903-linux-2.5.73-bk1/drivers/char/nwbutton.h .11903-linux-2.5.73-bk1.updated/drivers/char/nwbutton.h
--- .11903-linux-2.5.73-bk1/drivers/char/nwbutton.h 2003-05-27 15:02:08.000000000 +1000
+++ .11903-linux-2.5.73-bk1.updated/drivers/char/nwbutton.h 2003-06-24 11:43:58.000000000 +1000
@@ -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 */
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .11903-linux-2.5.73-bk1/drivers/char/pcxx.c .11903-linux-2.5.73-bk1.updated/drivers/char/pcxx.c
--- .11903-linux-2.5.73-bk1/drivers/char/pcxx.c 2003-06-15 11:29:51.000000000 +1000
+++ .11903-linux-2.5.73-bk1.updated/drivers/char/pcxx.c 2003-06-24 11:50:28.000000000 +1000
@@ -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(register struct tty_struct *tty)
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .11903-linux-2.5.73-bk1/drivers/char/stallion.c .11903-linux-2.5.73-bk1.updated/drivers/char/stallion.c
--- .11903-linux-2.5.73-bk1/drivers/char/stallion.c 2003-06-15 11:29:52.000000000 +1000
+++ .11903-linux-2.5.73-bk1.updated/drivers/char/stallion.c 2003-06-24 11:43:58.000000000 +1000
@@ -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);
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .11903-linux-2.5.73-bk1/drivers/net/wan/sdladrv.c .11903-linux-2.5.73-bk1.updated/drivers/net/wan/sdladrv.c
--- .11903-linux-2.5.73-bk1/drivers/net/wan/sdladrv.c 2003-06-15 11:29:56.000000000 +1000
+++ .11903-linux-2.5.73-bk1.updated/drivers/net/wan/sdladrv.c 2003-06-24 11:43:58.000000000 +1000
@@ -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 ********************************************************/
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .11903-linux-2.5.73-bk1/drivers/net/wan/sdlamain.c .11903-linux-2.5.73-bk1.updated/drivers/net/wan/sdlamain.c
--- .11903-linux-2.5.73-bk1/drivers/net/wan/sdlamain.c 2003-05-27 15:02:12.000000000 +1000
+++ .11903-linux-2.5.73-bk1.updated/drivers/net/wan/sdlamain.c 2003-06-24 11:43:58.000000000 +1000
@@ -177,10 +177,6 @@ static void dbg_kfree(void * v, int line
extern 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
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal working-2.5.70-bk16-check_region/Makefile working-2.5.70-bk16-check_region-inline/Makefile
--- working-2.5.70-bk16-check_region/Makefile 2003-06-12 09:57:39.000000000 +1000
+++ working-2.5.70-bk16-check_region-inline/Makefile 2003-06-12 21:34:40.000000000 +1000
@@ -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.
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .15095-linux-2.5.73-bk1/include/linux/netfilter.h .15095-linux-2.5.73-bk1.updated/include/linux/netfilter.h
--- .15095-linux-2.5.73-bk1/include/linux/netfilter.h 2003-05-05 12:37:12.000000000 +1000
+++ .15095-linux-2.5.73-bk1.updated/include/linux/netfilter.h 2003-06-24 12:19:07.000000000 +1000
@@ -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_info
int 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__*/
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .15095-linux-2.5.73-bk1/include/linux/proc_fs.h .15095-linux-2.5.73-bk1.updated/include/linux/proc_fs.h
--- .15095-linux-2.5.73-bk1/include/linux/proc_fs.h 2003-06-15 11:30:09.000000000 +1000
+++ .15095-linux-2.5.73-bk1.updated/include/linux/proc_fs.h 2003-06-24 12:19:07.000000000 +1000
@@ -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(const
extern 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 {
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .15095-linux-2.5.73-bk1/net/ipv4/Makefile .15095-linux-2.5.73-bk1.updated/net/ipv4/Makefile
--- .15095-linux-2.5.73-bk1/net/ipv4/Makefile 2003-06-15 11:30:12.000000000 +1000
+++ .15095-linux-2.5.73-bk1.updated/net/ipv4/Makefile 2003-06-24 12:19:07.000000000 +1000
@@ -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
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .15095-linux-2.5.73-bk1/net/ipv4/foo.c .15095-linux-2.5.73-bk1.updated/net/ipv4/foo.c
--- .15095-linux-2.5.73-bk1/net/ipv4/foo.c 1970-01-01 10:00:00.000000000 +1000
+++ .15095-linux-2.5.73-bk1.updated/net/ipv4/foo.c 2003-06-24 16:45:38.000000000 +1000
@@ -0,0 +1,42 @@
+#include <linux/types.h>
+#include <linux/ip.h>
+#include <linux/netfilter.h>
+#include <linux/netfilter_ipv4.h>
+#include <linux/init.h>
+#include <linux/moduleparam.h>
+
+static int fail_hook;
+module_param(fail_hook, int, 0);
+
+#define HOOK(n) \
+static int init##n(void) \
+{ \
+ printk("foo: init %u\n", n); \
+ if (n == fail_hook) \
+ return -EINVAL; \
+ return 0; \
+} \
+static void fini##n(void) \
+{ \
+ printk("foo: fini %u\n", n); \
+} \
+module_init_exit(-n, init##n, fini##n)
+
+HOOK(5);
+HOOK(1);
+HOOK(2);
+HOOK(3);
+HOOK(4);
+HOOK(6);
+
+/* Old-style... */
+int init_module(void)
+{
+ printk("foo: init module\n");
+ return 0;
+}
+
+void cleanup_module(void)
+{
+ printk("foo: cleanup module\n");
+}
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .15095-linux-2.5.73-bk1/net/ipv4/netfilter/ip_conntrack_standalone.c .15095-linux-2.5.73-bk1.updated/net/ipv4/netfilter/ip_conntrack_standalone.c
--- .15095-linux-2.5.73-bk1/net/ipv4/netfilter/ip_conntrack_standalone.c 2003-05-05 12:37:14.000000000 +1000
+++ .15095-linux-2.5.73-bk1.updated/net/ipv4/netfilter/ip_conntrack_standalone.c 2003-06-24 12:19:07.000000000 +1000
@@ -223,95 +223,6 @@ static unsigned int ip_conntrack_local(u
return ip_conntrack_in(hooknum, pskb, in, out, okfn);
}
-/* Connection tracking may drop packets, but never alters them, so
- make it the first hook. */
-static struct nf_hook_ops ip_conntrack_in_ops = {
- .hook = ip_conntrack_in,
- .owner = THIS_MODULE,
- .pf = PF_INET,
- .hooknum = NF_IP_PRE_ROUTING,
- .priority = NF_IP_PRI_CONNTRACK,
-};
-
-static struct nf_hook_ops ip_conntrack_local_out_ops = {
- .hook = ip_conntrack_local,
- .owner = THIS_MODULE,
- .pf = PF_INET,
- .hooknum = NF_IP_LOCAL_OUT,
- .priority = NF_IP_PRI_CONNTRACK,
-};
-
-/* Refragmenter; last chance. */
-static struct nf_hook_ops ip_conntrack_out_ops = {
- .hook = ip_refrag,
- .owner = THIS_MODULE,
- .pf = PF_INET,
- .hooknum = NF_IP_POST_ROUTING,
- .priority = NF_IP_PRI_LAST,
-};
-
-static struct nf_hook_ops ip_conntrack_local_in_ops = {
- .hook = ip_confirm,
- .owner = THIS_MODULE,
- .pf = PF_INET,
- .hooknum = NF_IP_LOCAL_IN,
- .priority = NF_IP_PRI_LAST-1,
-};
-
-static int init_or_cleanup(int init)
-{
- struct proc_dir_entry *proc;
- int ret = 0;
-
- if (!init) goto cleanup;
-
- ret = ip_conntrack_init();
- if (ret < 0)
- goto cleanup_nothing;
-
- proc = proc_net_create("ip_conntrack",0,list_conntracks);
- if (!proc) goto cleanup_init;
- proc->owner = THIS_MODULE;
-
- ret = nf_register_hook(&ip_conntrack_in_ops);
- if (ret < 0) {
- printk("ip_conntrack: can't register pre-routing hook.\n");
- goto cleanup_proc;
- }
- ret = nf_register_hook(&ip_conntrack_local_out_ops);
- if (ret < 0) {
- printk("ip_conntrack: can't register local out hook.\n");
- goto cleanup_inops;
- }
- ret = nf_register_hook(&ip_conntrack_out_ops);
- if (ret < 0) {
- printk("ip_conntrack: can't register post-routing hook.\n");
- goto cleanup_inandlocalops;
- }
- ret = nf_register_hook(&ip_conntrack_local_in_ops);
- if (ret < 0) {
- printk("ip_conntrack: can't register local in hook.\n");
- goto cleanup_inoutandlocalops;
- }
-
- return ret;
-
- cleanup:
- nf_unregister_hook(&ip_conntrack_local_in_ops);
- cleanup_inoutandlocalops:
- nf_unregister_hook(&ip_conntrack_out_ops);
- cleanup_inandlocalops:
- nf_unregister_hook(&ip_conntrack_local_out_ops);
- cleanup_inops:
- nf_unregister_hook(&ip_conntrack_in_ops);
- cleanup_proc:
- proc_net_remove("ip_conntrack");
- cleanup_init:
- ip_conntrack_cleanup();
- cleanup_nothing:
- return ret;
-}
-
/* FIXME: Allow NULL functions and sub in pointers to generic for
them. --RR */
int ip_conntrack_protocol_register(struct ip_conntrack_protocol *proto)
@@ -351,25 +262,26 @@ void ip_conntrack_protocol_unregister(st
ip_ct_selective_cleanup(kill_proto, &proto->proto);
}
-static int __init init(void)
-{
- return init_or_cleanup(1);
-}
-
-static void __exit fini(void)
-{
- init_or_cleanup(0);
-}
-
-module_init(init);
-module_exit(fini);
-
/* Some modules need us, but don't depend directly on any symbol.
They should call this. */
void need_ip_conntrack(void)
{
}
+module_init_exit(-1, ip_conntrack_init, ip_conntrack_cleanup);
+module_proc_net(0, "ip_conntrack", list_conntracks);
+
+/* Connection tracking may drop packets, but never alters them, so
+ make it the first hook. */
+module_nf_hook(0, ip_conntrack_in, PF_INET, NF_IP_PRE_ROUTING,
+ NF_IP_PRI_CONNTRACK);
+module_nf_hook(0, ip_conntrack_local, PF_INET, NF_IP_LOCAL_OUT,
+ NF_IP_PRI_CONNTRACK);
+module_nf_hook(0, ip_confirm, PF_INET, NF_IP_LOCAL_IN, NF_IP_PRI_LAST-1);
+
+/* Refragmenter; last chance. */
+module_nf_hook(0, ip_refrag, PF_INET, NF_IP_POST_ROUTING, NF_IP_PRI_LAST);
+
EXPORT_SYMBOL(ip_conntrack_protocol_register);
EXPORT_SYMBOL(ip_conntrack_protocol_unregister);
EXPORT_SYMBOL(invert_tuplepr);
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .15095-linux-2.5.73-bk1/net/ipv4/netfilter/ip_tables.c .15095-linux-2.5.73-bk1.updated/net/ipv4/netfilter/ip_tables.c
--- .15095-linux-2.5.73-bk1/net/ipv4/netfilter/ip_tables.c 2003-05-27 15:02:26.000000000 +1000
+++ .15095-linux-2.5.73-bk1.updated/net/ipv4/netfilter/ip_tables.c 2003-06-24 12:19:07.000000000 +1000
@@ -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);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] Allow arbitrary number of init funcs in modules
2003-06-23 9:19 [PATCH 3/3] Allow arbitrary number of init funcs in modules Rusty Russell
2003-06-23 19:11 ` Jonathan Corbet
@ 2003-06-24 17:01 ` Roman Zippel
2003-06-25 3:10 ` Rusty Russell
1 sibling, 1 reply; 8+ messages in thread
From: Roman Zippel @ 2003-06-24 17:01 UTC (permalink / raw)
To: Rusty Russell; +Cc: torvalds, akpm, davem, linux-kernel, mochel
Hi,
On Mon, 23 Jun 2003, Rusty Russell wrote:
> 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);
What happens if a module is compiled into the kernel and one of the init
functions fails?
bye, Roman
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] Allow arbitrary number of init funcs in modules
2003-06-23 19:11 ` Jonathan Corbet
2003-06-24 2:29 ` Rusty Russell
2003-06-24 6:57 ` Rusty Russell
@ 2003-06-24 20:06 ` Horst von Brand
2 siblings, 0 replies; 8+ messages in thread
From: Horst von Brand @ 2003-06-24 20:06 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: Linux Kernel Mailing List
corbet@lwn.net (Jonathan Corbet) said:
> --- 2.5.73-rr/kernel/module.c Tue Jun 24 02:58:32 2003
> +++ 2.5.73/kernel/module.c Tue Jun 24 03:00:36 2003
> @@ -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++)
.....
--
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] Allow arbitrary number of init funcs in modules
2003-06-24 17:01 ` Roman Zippel
@ 2003-06-25 3:10 ` Rusty Russell
2003-06-25 10:30 ` Roman Zippel
0 siblings, 1 reply; 8+ messages in thread
From: Rusty Russell @ 2003-06-25 3:10 UTC (permalink / raw)
To: Roman Zippel; +Cc: torvalds, akpm, davem, linux-kernel, mochel
In message <Pine.LNX.4.44.0306241851550.11817-100000@serv> you write:
> Hi,
>
> On Mon, 23 Jun 2003, Rusty Russell wrote:
>
> > 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);
>
> 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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] Allow arbitrary number of init funcs in modules
2003-06-25 3:10 ` Rusty Russell
@ 2003-06-25 10:30 ` Roman Zippel
0 siblings, 0 replies; 8+ messages in thread
From: Roman Zippel @ 2003-06-25 10:30 UTC (permalink / raw)
To: Rusty Russell; +Cc: Linus Torvalds, akpm, davem, linux-kernel, mochel
Hi,
On Wed, 25 Jun 2003, Rusty Russell wrote:
> > 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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2003-06-25 10:16 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-06-23 9:19 [PATCH 3/3] Allow arbitrary number of init funcs in modules Rusty Russell
2003-06-23 19:11 ` Jonathan Corbet
2003-06-24 2:29 ` Rusty Russell
2003-06-24 6:57 ` Rusty Russell
2003-06-24 20:06 ` Horst von Brand
2003-06-24 17:01 ` Roman Zippel
2003-06-25 3:10 ` Rusty Russell
2003-06-25 10:30 ` Roman Zippel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).