On Wed, 5 Jun 2002, Patrick Mochel wrote:
> No, I haven't given up on it, and I admit it's fishy. However, I did it
> to ensure that no one can access the driver since it is going away as
> soon as we return to pci_unregister_driver. Though, I'll also admit that
> making the next potential user hit the BUG() in get_driver() is not the
> nicest thing to do...
There's two phases:
o remove_driver():
Make sure that now one else can find (and inc the refcount) your struct
anymore. That can and should be done immediately. Drop your reference.
o At some point later, everybody else will have dropped their references
and the destructor can be called.
Alternatively:
o remove_driver():
Make sure that now one else can find (and inc the refcount) your struct
anymore. That can and should be done immediately. Drop your reference.
Wait for everybody else to drop their reference. Then return to the
caller
o At some point later, everybody else will have dropped their references
and the remove_driver() should be woken up and you get to the return.
I claim the latter is more appropriate, since driver_register() and
driver_remove() (btw, I'd rather have the latter called
driver_unregister()) will always be called by the same user, normally
the driver module. That's different from some object which isn't
explicitly unregistered somewhere, but will be cleaned up e.g. due to
memory pressure some time later. You can always achieve the second
if you have the first, but I see no point in having driver authors deal
with it.
> There is a destructor: struct device_driver::release(). It's the last
> thing called from __remove_driver().
>
> No matter what, the module will get unloaded immediately. What would be
> nice is if we could delay it until we said it was ok (from the driver's
> release callback). We could even define a common release callback for all
> drivers:
That's not true. module_exit() is called from rmmod(8) and thus in process
context. It's perfectly fine to sleep in module_exit() until we're safe to
be deleted.
> void driver_release(struct device_driver * drv)
> {
> struct module * module = drv->module;
>
> /* do unload of module */
> }
>
> We wouldn't need a completion; we would just have to wait until the
> refcount hit 0 naturally. But, I'm just making this stuff up. Is this at
> all possible?
Now this is bad, what if the last reference went away from irq context?
Unloading a module from irq context doesn't sound like a good idea to me.
I'd suggest something like
pci_driver_release(pdrv)
{
complete(&pdrv->complete);
}
pci_register_driver(pdrv)
{
...
pdrv->driver.release = pci_driver_release;
pdrv->complete = NULL;
}
pci_unregister_driver(pdrv)
{
DECLARE_COMPLETION(complete);
pdrv->complete = &complete;
put_driver(&drv->driver);
wait_for_completion(&complete);
}
And make clear that pci_unregister_driver() can be called from process
context only.
Actually, as said above, I'd rather do the same thing in remove_driver(),
i.e. make that sleep as well - unless you can come up with an example
where anybody would want to call remove_driver() and cannot sleep.
Just replace ->release with the struct completion *.
--Kai
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Fri Jun 07 2002 - 22:00:26 EST