Re: module unload deadlock

[Rusty Russell <rusty@rustcorp.com.au>]

In message <20040217172646.GT4478@dualathlon.random> you write:
> I've got bugreports for a deadlock due semaphore recursion in the module
> unload code of 2.6.
> 
> You considered the recursion issue for the ->init but not for the
> ->exit.

I considered it, but no cleanup called request_module which I
(reluctantly, as it made the locking non-trivial) dropped the lock
around init.

> the actual module doing request_modules in the cleanup handler is
> parport_pc, calling parport_enumerate (it calls it for another reason,
> and parport enumerate is told to load up a lowlevel driver if none was
> present, that's worthless for the unload routine but it's useful for all
> other calls of parport_enumerate). It's uncertain if other drivers
> triggers this deadlock.

Well, that's pretty stupid, but I think this should be fixed in the
module code for a practical reason: the number of oopses or hangs in
rmmod functions.  Bad enough that the module is useless: stopping
other module operations is very suboptimal.

> Comments?

Patch seems OK to me.  I'd prefer it to go via the -mm tree though to
shake things out in case I've missed something.

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


Name: Drop Lock Around Module Exit
Status: Experimental
From: Andrea Arcangeli <andrea@suse.de>

Turns out parport can do request_module from its exit function.  We
should drop lock anyway, because too many module exit functions oops
or hang anyway, rendering all module ops useless.

--- SLES/kernel/module.c.~1~	2004-02-12 17:24:42.000000000 +0100
+++ SLES/kernel/module.c	2004-02-17 18:08:05.519670280 +0100
@@ -732,7 +732,9 @@ sys_delete_module(const char __user *nam
 		wait_for_zero_refcount(mod);
 
 	/* Final destruction now noone is using it. */
+	up(&module_mutex);
 	mod->exit();
+	down(&module_mutex);
 	free_module(mod);
 
  out: