lkml.org 
[lkml]   [2003]   [Mar]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: PCI driver module unload race?
    On Sat, Mar 08, 2003 at 11:12:37AM -0800, Greg KH wrote:
    > On Sat, Mar 08, 2003 at 10:47:49AM +0000, Russell King wrote:
    > > Hi,
    > >
    > > What prevents the following scenario from happening? It's purely
    > > theoretical - I haven't seen this occuring.
    > >
    > > - Load PCI driver.
    > >
    > > - PCI driver registers using pci_module_init(), and adds itself to sysfs.
    > >
    > > - Hot-plugin a PCI device which uses this driver. sysfs matches the PCI
    > > driver, and calls the PCI drivers probe function.
    >
    > Ugh, yes you are correct, I can't believe I missed this before.
    >
    > How does this patch look?

    Bad...

    What are you trying to solve? After driver calls pci_unregister_driver,
    it is sure that there are no other users of this pci driver. So I do
    not understand why you are adding these additional protections (and one
    in remove looks suspicious to me: if you unplug device while module
    in unloading, you do not call remove?! who will call it then? you'll
    not get oopses in pci_unregister_driver? leaked memory?). Driver
    authors simple have to unregister from subsystems in correct order,
    no try_module_get() put anywhere is not going to solve this problem.

    There is simple no relation between modules and drivers - and anyone
    who is trying to force this is misguided. Driver starts its life
    inside call to *_register_*, and ceases in *_unregister_*, not
    somewhere after call to init_module, and before cleanup_module.
    And if module registers resources in wrong order, module has to
    cope with its stupidity, not kernel (and I have yet to see two
    kernel interfaces which cannot be properly ordered for both init
    and shutdown).
    Petr Vandrovec
    vandrove@vc.cvut.cz


    > thanks,
    >
    > greg k-h
    >
    >
    > ===== drivers/pci/pci-driver.c 1.17 vs edited =====
    > --- 1.17/drivers/pci/pci-driver.c Sat Nov 23 13:23:03 2002
    > +++ edited/drivers/pci/pci-driver.c Sat Mar 8 11:21:06 2003
    > @@ -48,6 +48,12 @@
    > if (!pci_dev->driver && drv->probe) {
    > const struct pci_device_id *id;
    >
    > + if (!try_module_get(drv->owner)) {
    > + dev_err(dev, "Can't get a module reference for %s\n",
    > + drv->name);
    > + error = -ENODEV;
    > + goto exit;
    > + }
    > id = pci_match_device(drv->id_table, pci_dev);
    > if (id)
    > error = drv->probe(pci_dev, id);
    > @@ -55,7 +61,9 @@
    > pci_dev->driver = drv;
    > error = 0;
    > }
    > + module_put(drv->owner);
    > }
    > +exit:
    > return error;
    > }
    >
    > @@ -66,7 +74,10 @@
    >
    > if (drv) {
    > if (drv->remove)
    > - drv->remove(pci_dev);
    > + if (try_module_get(drv->owner)) {
    > + drv->remove(pci_dev);
    > + module_put(drv->owner);
    > + }
    > pci_dev->driver = NULL;
    > }
    > return 0;
    > ===== include/linux/pci.h 1.36 vs edited =====
    > --- 1.36/include/linux/pci.h Tue Mar 4 21:09:58 2003
    > +++ edited/include/linux/pci.h Sat Mar 8 11:20:15 2003
    > @@ -493,6 +493,7 @@
    >
    > struct pci_driver {
    > struct list_head node;
    > + struct module *owner;
    > char *name;
    > const struct pci_device_id *id_table; /* NULL if wants all devices */
    > int (*probe) (struct pci_dev *dev, const struct pci_device_id *id); /* New device inserted */
    > -
    > 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/
    >
    -
    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/

    \
     
     \ /
      Last update: 2005-03-22 13:33    [W:0.051 / U:68.296 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site