Re: modprobe + request_module() deadlock

Rusty Russell
2004-11-24T05:03:17Z

On Mon, 2004-11-22 at 17:52 +0100, Gerd Knorr wrote:
> > > I can fix that in the driver, by delaying the request_module() somehow
> > > until the saa7134 module initialization is finished. I don't think that
> > > this is a good idea through as it looks like I'm not the only one with
> > > that problem ...
> >
> > Delaying request_module() sounds ugly. Anyway, if you can
> > get it to work reliably...
>
> I think I can, havn't tried yet through.

I still don't really like it as a solution, but this patch (pending
anyway) should help.

Rusty.

Name: More Module Notifiers
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Status: Trivial

Put in the rest of the module notifiers, esp. one when a module is
being removed.

Index: linux-2.6.10-rc2-bk8-Module/kernel/module.c
===================================================================
--- linux-2.6.10-rc2-bk8-Module.orig/kernel/module.c 2004-11-16 15:30:10.000000000 +1100
+++ linux-2.6.10-rc2-bk8-Module/kernel/module.c 2004-11-24 15:28:10.745436776 +1100
@@ -82,6 +82,13 @@
}
EXPORT_SYMBOL(unregister_module_notifier);

+static void module_notify(struct module *mod, enum module_state state)
+{
+ down(&notify_mutex);
+ notifier_call_chain(&module_notify_list, state, mod);
+ up(&notify_mutex);
+}
+
/* We require a truly strong try_module_get() */
static inline int strong_try_module_get(struct module *mod)
{
@@ -579,6 +586,8 @@
if (ret != 0)
goto out;

+ module_notify(mod, MODULE_STATE_GOING);
+
/* Never wait if forced. */
if (!forced && module_refcount(mod) != 0)
wait_for_zero_refcount(mod);
@@ -589,6 +598,7 @@
mod->exit();
down(&module_mutex);
}
+ module_notify(mod, MODULE_STATE_GONE);
free_module(mod);

out:
@@ -1792,9 +1802,7 @@
/* Drop lock so they can recurse */
up(&module_mutex);

- down(&notify_mutex);
- notifier_call_chain(&module_notify_list, MODULE_STATE_COMING, mod);
- up(&notify_mutex);
+ module_notify(mod, MODULE_STATE_COMING);

/* Start the module */
if (mod->init != NULL)
@@ -1803,12 +1811,14 @@
/* Init routine failed: abort. Try to protect us from
buggy refcounters. */
mod->state = MODULE_STATE_GOING;
+ module_notify(mod, MODULE_STATE_GOING);
synchronize_kernel();
if (mod->unsafe)
printk(KERN_ERR "%s: module is now stuck!\n",
mod->name);
else {
module_put(mod);
+ module_notify(mod, MODULE_STATE_GONE);
down(&module_mutex);
free_module(mod);
up(&module_mutex);
@@ -1826,6 +1836,7 @@
mod->init_size = 0;
mod->init_text_size = 0;
up(&module_mutex);
+ module_notify(mod, MODULE_STATE_LIVE);

return 0;
}
Index: linux-2.6.10-rc2-bk8-Module/include/linux/module.h
===================================================================
--- linux-2.6.10-rc2-bk8-Module.orig/include/linux/module.h 2004-11-16 15:30:07.000000000 +1100
+++ linux-2.6.10-rc2-bk8-Module/include/linux/module.h 2004-11-24 15:28:10.767433432 +1100
@@ -220,6 +220,7 @@
MODULE_STATE_LIVE,
MODULE_STATE_COMING,
MODULE_STATE_GOING,
+ MODULE_STATE_GONE, /* Only for notifier: module about to be freed */
};

/* Similar stuff for section attributes. */

--
A bad analogy is like a leaky screwdriver -- Richard Braakman