Re: module unload deadlock

[Andrea Arcangeli <andrea@suse.de>]

On Wed, Feb 18, 2004 at 04:46:59PM +0000, viro@parcelfarce.linux.theplanet.co.uk wrote:
> On Wed, Feb 18, 2004 at 04:40:41PM +0100, Andrea Arcangeli wrote:
> > On Wed, Feb 18, 2004 at 04:35:55AM +0000, viro@parcelfarce.linux.theplanet.co.uk wrote:
> > It's clear this could be fixed by making sure parport won't call
> > request_module from cleanup_module, the primary reason I fixed it in the
> > module code is that I don't know if other drivers are doing this, do
> > you? What parport did was legitimate, and it was working fine in the
> > past, sure the parport code could be made slightly more complex and
> > aware about the fact it doesn't worth to try loading the lowlevel module
> > in cleanup_exit, but it wasn't obviously wrong, the cleanup/init module
> > are slow paths, it didn't matter if parport tried to load a lowlevel
> > module there.
> 
> Sigh...
> 
> No, it wasn't legitimate.  As the matter of fact, _nothing_ outside of
> parport/share.c has any business looking at the list of ports.  IOW,
> parport_enumerate() should be removed regardless of the request_module()
> crap.
> 
> In particular, parport_pc should keep track of the ports it had created
> instead of messing with parport_enumerate().

The one you propose (of parport_pc keeping track of the ports by itself)
is a different model, currently it's the highlevel that keeps track of
the ports and each lowlevel registers the lowlevel ports in the
highlevel list of ports. It doesn't mean the current model is wrong. You
may not like it and you may find it less efficient, or less clean, or
whatever, but the current model is definitely legitimate (the parport
code has the troubles you found in the sharing code locking, but this
registration model you're complaining about now is legitimate instead).

But let's ignore parport, the only question is if you know if other
modules are doing the same thing or not. Calling request_module from
cleanup_module was allowed with the 2.4 module API, now it deadlocks.
The only single reason I changed the module code is to avoid other
modules to deadlock in rmmod.