From mboxrd@z Thu Jan 1 00:00:00 1970 From: john stultz Date: Mon, 04 Mar 2002 23:54:11 +0000 Subject: [PATCH] PCI device list locking MIME-Version: 1 Content-Type: multipart/mixed; boundary="=-nSahOyL6b0zgBFqTdzY+" Message-Id: List-Id: To: linux-hotplug@vger.kernel.org --=-nSahOyL6b0zgBFqTdzY+ Content-Type: multipart/alternative; boundary="=-BRzxMr6OmpSZ3jy2yW7x" --=-BRzxMr6OmpSZ3jy2yW7x Content-Type: text/plain Content-Transfer-Encoding: 7bit At Greg K-H's request, I've been looking into locking the pci_devices list to protect it from being altered by hotplug while being traversed. So far I've limited my work to just the core pci code, only adding the pcidevlist_lock spinlock and using it to wrap all uses of pci_for_each_dev() or pci_devices. I'm planning to go on and lock all uses of pci_for_each_dev() in the driver and arch specific files, as well as the bus device lists (and later the bus lists), but I wanted to send this out for comments first. I don't really have any test cases for sanity checking, but it boots on an 8 way and running 50 "cat /proc/pci"s in parallel doesn't choke, so let me know if I've mistakenly broken anything. Thanks -john (please CC me as I'm not sub'ed to the lists) I apologize if this is a dup, evo crashed on send and I'm not sure if it made it out first. --=-BRzxMr6OmpSZ3jy2yW7x Content-Type: text/html; charset=utf-8 At Greg K-H's request, I've been looking into locking the pci_devices list to protect it from being altered by hotplug while being traversed. So far I've limited my work to just the core pci code, only adding the pcidevlist_lock spinlock and using it to wrap all uses of pci_for_each_dev() or pci_devices. I'm planning to go on and lock all uses of pci_for_each_dev() in the driver and arch specific files, as well as the bus device lists (and later the bus lists), but I wanted to send this out for comments first. I don't really have any test cases for sanity checking, but it boots on an 8 way and running 50 "cat /proc/pci"s in parallel doesn't choke, so let me know if I've mistakenly broken anything.

Thanks
-john

(please CC me as I'm not sub'ed to the lists)

I apologize if this is a dup, evo crashed on send and I'm not sure if it made it out first. --=-BRzxMr6OmpSZ3jy2yW7x-- --=-nSahOyL6b0zgBFqTdzY+ Content-Disposition: attachment; filename=pcidevlist_lock.patch Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=ISO-8859-1 Index: linux24/drivers/pci/pci.c diff -u linux24/drivers/pci/pci.c:1.1.1.1 linux24/drivers/pci/pci.c:1.1.1.1= .28.16 --- linux24/drivers/pci/pci.c:1.1.1.1 Tue Dec 18 15:49:16 2001 +++ linux24/drivers/pci/pci.c Mon Mar 4 15:38:39 2002 @@ -37,6 +37,7 @@ =20 LIST_HEAD(pci_root_buses); LIST_HEAD(pci_devices); +spinlock_t pcidevlist_lock =3D SPIN_LOCK_UNLOCKED; =20 /** * pci_find_slot - locate PCI device from a given PCI slot @@ -54,11 +55,14 @@ pci_find_slot(unsigned int bus, unsigned int devfn) { struct pci_dev *dev; - + spin_lock(&pcidevlist_lock); pci_for_each_dev(dev) { - if (dev->bus->number =3D=3D bus && dev->devfn =3D=3D devfn) + if (dev->bus->number =3D=3D bus && dev->devfn =3D=3D devfn){ + spin_unlock(&pcidevlist_lock); return dev; + } } + spin_unlock(&pcidevlist_lock); return NULL; } =20 @@ -81,17 +85,23 @@ unsigned int ss_vendor, unsigned int ss_device, const struct pci_dev *from) { - struct list_head *n =3D from ? from->global_list.next : pci_devices.next; + struct list_head *n; +=09 + spin_lock(&pcidevlist_lock); + n =3D from ? from->global_list.next : pci_devices.next; =20 while (n !=3D &pci_devices) { struct pci_dev *dev =3D pci_dev_g(n); if ((vendor =3D=3D PCI_ANY_ID || dev->vendor =3D=3D vendor) && (device =3D=3D PCI_ANY_ID || dev->device =3D=3D device) && (ss_vendor =3D=3D PCI_ANY_ID || dev->subsystem_vendor =3D=3D ss_vend= or) && - (ss_device =3D=3D PCI_ANY_ID || dev->subsystem_device =3D=3D ss_devi= ce)) + (ss_device =3D=3D PCI_ANY_ID || dev->subsystem_device =3D=3D ss_devi= ce)){ + spin_unlock(&pcidevlist_lock); return dev; + } n =3D n->next; } + spin_unlock(&pcidevlist_lock); return NULL; } =20 @@ -130,14 +140,19 @@ struct pci_dev * pci_find_class(unsigned int class, const struct pci_dev *from) { - struct list_head *n =3D from ? from->global_list.next : pci_devices.next; + struct list_head *n; =20 + spin_lock(&pcidevlist_lock); + n =3D from ? from->global_list.next : pci_devices.next; while (n !=3D &pci_devices) { struct pci_dev *dev =3D pci_dev_g(n); - if (dev->class =3D=3D class) + if (dev->class =3D=3D class){ + spin_unlock(&pcidevlist_lock); return dev; + } n =3D n->next; } + spin_unlock(&pcidevlist_lock); return NULL; } =20 @@ -605,10 +620,12 @@ int count =3D 0; =20 list_add_tail(&drv->node, &pci_drivers); + spin_lock(&pcidevlist_lock); pci_for_each_dev(dev) { if (!pci_dev_driver(dev)) count +=3D pci_announce_device(drv, dev); } + spin_unlock(&pcidevlist_lock); return count; } =20 @@ -628,6 +645,7 @@ struct pci_dev *dev; =20 list_del(&drv->node); + spin_lock(&pcidevlist_lock); pci_for_each_dev(dev) { if (dev->driver =3D=3D drv) { if (drv->remove) @@ -635,6 +653,7 @@ dev->driver =3D NULL; } } + spin_unlock(&pcidevlist_lock); } =20 #ifdef CONFIG_HOTPLUG @@ -700,7 +719,7 @@ if (drv->remove && pci_announce_device(drv, dev)) break; } - +=09 /* notify userspace of new hotplug device */ run_sbin_hotplug(dev, TRUE); } @@ -716,7 +735,9 @@ pci_insert_device(struct pci_dev *dev, struct pci_bus *bus) { list_add_tail(&dev->bus_list, &bus->devices); + spin_lock(&pcidevlist_lock); list_add_tail(&dev->global_list, &pci_devices); + spin_unlock(&pcidevlist_lock); #ifdef CONFIG_PROC_FS pci_proc_attach_device(dev); #endif @@ -751,7 +772,9 @@ dev->driver =3D NULL; } list_del(&dev->bus_list); + spin_lock(&pcidevlist_lock);=20 list_del(&dev->global_list); + spin_unlock(&pcidevlist_lock); pci_free_resources(dev); #ifdef CONFIG_PROC_FS pci_proc_detach_device(dev); @@ -1327,7 +1350,9 @@ * Link the device to both the global PCI device chain and * the per-bus list of devices. */ + spin_lock(&pcidevlist_lock); list_add_tail(&dev->global_list, &pci_devices); + spin_unlock(&pcidevlist_lock); list_add_tail(&dev->bus_list, &bus->devices); =20 /* Fix up broken headers */ @@ -1368,7 +1393,6 @@ if (dev->hdr_type =3D=3D PCI_HEADER_TYPE_BRIDGE || dev->hdr_type =3D=3D= PCI_HEADER_TYPE_CARDBUS) max =3D pci_scan_bridge(bus, dev, max, pass); } - /* * We've scanned the bus and so we know all about what's on * the other side of any bridges that may be on this bus plus @@ -1935,9 +1959,11 @@ =20 pcibios_init(); =20 + spin_lock(&pcidevlist_lock); pci_for_each_dev(dev) { pci_fixup_device(PCI_FIXUP_FINAL, dev); } + spin_unlock(&pcidevlist_lock); =20 #ifdef CONFIG_PM pm_register(PM_PCI_DEV, 0, pci_pm_callback); Index: linux24/drivers/pci/proc.c diff -u linux24/drivers/pci/proc.c:1.1.1.1 linux24/drivers/pci/proc.c:1.1.1= .1.28.2 --- linux24/drivers/pci/proc.c:1.1.1.1 Tue Dec 18 15:49:16 2001 +++ linux24/drivers/pci/proc.c Fri Mar 1 12:51:09 2002 @@ -306,17 +306,26 @@ /* iterator */ static void *pci_seq_start(struct seq_file *m, loff_t *pos) { - struct list_head *p =3D &pci_devices; - loff_t n =3D *pos; - - /* XXX: surely we need some locking for traversing the list? */ + struct list_head *p; + loff_t n; + spin_lock(&pcidevlist_lock); + p =3D &pci_devices; + n =3D *pos; while (n--) { p =3D p->next; - if (p =3D=3D &pci_devices) + if (p =3D=3D &pci_devices){ + spin_unlock(&pcidevlist_lock); return NULL; + } } + spin_unlock(&pcidevlist_lock); return p; } + +/* +XXX: pci_seq_next() doesn't use pcidevlist_lock, make sure=20 +you hold it before calling. (Currently unused?) +*/ static void *pci_seq_next(struct seq_file *m, void *v, loff_t *pos) { struct list_head *p =3D v; @@ -559,9 +568,11 @@ entry =3D create_proc_entry("devices", 0, proc_bus_pci_dir); if (entry) entry->proc_fops =3D &proc_bus_pci_dev_operations; + spin_lock(&pcidevlist_lock); pci_for_each_dev(dev) { pci_proc_attach_device(dev); } + spin_unlock(&pcidevlist_lock); entry =3D create_proc_entry("pci", 0, NULL); if (entry) entry->proc_fops =3D &proc_pci_operations; Index: linux24/drivers/pci/setup-bus.c diff -u linux24/drivers/pci/setup-bus.c:1.1.1.1 linux24/drivers/pci/setup-b= us.c:1.1.1.1.28.3 --- linux24/drivers/pci/setup-bus.c:1.1.1.1 Tue Dec 18 15:49:16 2001 +++ linux24/drivers/pci/setup-bus.c Mon Mar 4 15:33:30 2002 @@ -222,9 +222,11 @@ ranges.found_vga =3D 0; pbus_assign_resources(b, &ranges); } + spin_lock(&pcidevlist_lock); pci_for_each_dev(dev) { pdev_enable_device(dev); } + spin_unlock(&pcidevlist_lock); } =20 /* Check whether the bridge supports I/O forwarding. Index: linux24/drivers/pci/setup-irq.c diff -u linux24/drivers/pci/setup-irq.c:1.1.1.1 linux24/drivers/pci/setup-i= rq.c:1.1.1.1.28.1 --- linux24/drivers/pci/setup-irq.c:1.1.1.1 Tue Dec 18 15:49:16 2001 +++ linux24/drivers/pci/setup-irq.c Fri Mar 1 14:27:55 2002 @@ -65,7 +65,9 @@ int (*map_irq)(struct pci_dev *, u8, u8)) { struct pci_dev *dev; + spin_lock(&pcidevlist_lock); pci_for_each_dev(dev) { pdev_fixup_irq(dev, swizzle, map_irq); } + spin_unlock(&pcidevlist_lock); } Index: linux24/drivers/pnp/isapnp.c diff -u linux24/drivers/pnp/isapnp.c:1.1.1.2 linux24/drivers/pnp/isapnp.c:1= .1.1.2.2.3 --- linux24/drivers/pnp/isapnp.c:1.1.1.2 Wed Feb 13 14:53:31 2002 +++ linux24/drivers/pnp/isapnp.c Mon Mar 4 15:33:30 2002 @@ -1685,10 +1685,14 @@ } #ifdef CONFIG_PCI if (!isapnp_skip_pci_scan) { + spin_lock(&pcidevlist_lock); pci_for_each_dev(dev) { - if (dev->irq =3D=3D irq) + if (dev->irq =3D=3D irq){ + spin_unlock(&pcidevlist_lock); return 1; + } } + spin_unlock(&pcidevlist_lock); } #endif if (request_irq(irq, isapnp_test_handler, SA_INTERRUPT, "isapnp", NULL)) Index: linux24/include/linux/pci.h diff -u linux24/include/linux/pci.h:1.1.1.1 linux24/include/linux/pci.h:1.1= .1.1.28.7 --- linux24/include/linux/pci.h:1.1.1.1 Tue Dec 18 15:49:00 2001 +++ linux24/include/linux/pci.h Wed Feb 27 15:11:05 2002 @@ -16,7 +16,7 @@ =20 #ifndef LINUX_PCI_H #define LINUX_PCI_H - +#include /* * Under PCI, each device has 256 bytes of configuration address space, * of which the first 64 bytes are standardized as follows: @@ -435,6 +435,7 @@ =20 extern struct list_head pci_root_buses; /* list of all known PCI buses */ extern struct list_head pci_devices; /* list of all devices */ +extern spinlock_t pcidevlist_lock; =20 /* * Error values that may be returned by PCI functions. --=-nSahOyL6b0zgBFqTdzY+-- _______________________________________________ Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net Linux-hotplug-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel