Re: modprobe + request_module() deadlock

[Gerd Knorr <kraxel@bytesex.org>]

On Wed, Nov 24, 2004 at 04:02:31PM +1100, Rusty Russell wrote:
> 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.

Untested proof-of-concept code (don't have a saa7134 card in my machine
at the moment), but that way it could work I think.  Tried to keep it
generic.  Basically it keeps a list of pending module loads and the
dependencies.  Then it hooks into the module state notifier chain and
calls request_module() once the depending module went to LIVE state.

Comments?

  Gerd

Index: saa7134-core.c
===================================================================
RCS file: /home/cvsroot/video4linux/saa7134-core.c,v
retrieving revision 1.20
diff -u -p -r1.20 saa7134-core.c
--- saa7134-core.c	23 Nov 2004 17:29:09 -0000	1.20
+++ saa7134-core.c	25 Nov 2004 15:54:23 -0000
@@ -233,6 +233,75 @@ static void dump_statusregs(struct saa71
 }
 #endif
 
+/* ----------------------------------------------------------- */
+
+struct pending_module {
+	struct module    *dep;
+	char             *name;
+	struct list_head next;
+};
+
+static int pending_call(struct notifier_block *self, unsigned long state,
+			void *module);
+
+static LIST_HEAD(pending_modules);
+static struct notifier_block pending_notifier = {
+	.notifier_call = pending_call,
+};
+
+static int pending_call(struct notifier_block *self, unsigned long state,
+			void *module)
+{
+	struct list_head *item;
+	struct pending_module *mod = NULL;
+
+	list_for_each(item,&pending_modules) {
+		mod = list_entry(item, struct pending_module, next);
+		if (mod->dep == module)
+			break;
+		mod = NULL;
+	}
+	if (NULL == mod)
+		return NOTIFY_DONE;
+
+	switch (state) {
+	case MODULE_STATE_LIVE:
+		request_module(mod->name);
+		/* fall through */
+	case MODULE_STATE_GOING:
+		list_del(&mod->next);
+		kfree(mod);
+		if (list_empty(&pending_modules))
+			unregister_module_notifier(&pending_notifier);
+		break;
+	}
+	return NOTIFY_DONE;
+}
+
+static void request_module_depend(struct module *dep, char *name)
+{
+	struct pending_module *mod;
+
+	switch (dep->state) {
+	case MODULE_STATE_COMING:
+		mod = kmalloc(sizeof(mod),GFP_KERNEL);
+		if (NULL == mod)
+			return;
+		mod->dep  = dep;
+		mod->name = name;
+		if (list_empty(&pending_modules))
+			register_module_notifier(&pending_notifier);
+		list_add(&mod->next,&pending_modules);
+		break;
+	case MODULE_STATE_LIVE:
+		request_module(name);
+		break;
+	default:
+		/* nothing */;
+		break;
+	}
+}
+
 /* ------------------------------------------------------------------ */
 
 /* nr of (saa7134-)pages for the given buffer size */
@@ -954,12 +1023,12 @@ static int __devinit saa7134_initdev(str
 		request_module("tuner");
 	if (dev->tda9887_conf)
 		request_module("tda9887");
-  	if (card_is_empress(dev)) {
-		request_module("saa7134-empress");
+  	if (1 /* card_is_empress(dev) */) {
+		request_module_depend(THIS_MODULE,"saa7134-empress");
 		request_module("saa6752hs");
 	}
-  	if (card_is_dvb(dev))
-		request_module("saa7134-dvb");
+  	if (1 /* card_is_dvb(dev) */)
+		request_module_depend(THIS_MODULE,"saa7134-dvb");
 
 	v4l2_prio_init(&dev->prio);