From mboxrd@z Thu Jan 1 00:00:00 1970 From: john stultz Date: Tue, 05 Mar 2002 01:05:58 +0000 Subject: Re: [PATCH] PCI device list locking MIME-Version: 1 Content-Type: multipart/mixed; boundary="=-kxRs9jYcnQnz6CtZyS9w" Message-Id: List-Id: References: In-Reply-To: To: linux-hotplug@vger.kernel.org --=-kxRs9jYcnQnz6CtZyS9w Content-Type: text/plain Content-Transfer-Encoding: 7bit On Mon, 2002-03-04 at 16:17, Greg KH wrote: > In short, I don't think you can use a spinlock, but rather a semaphore > is probably necessary. Sage advice indeed! Take 2: with semaphores Again, this is just fixes to the core pci code. Various drivers and arch specific code still traverse the pci_devices list without the pcidevlist_sem. If there are no complaints with this code (probably just wishful thinking) I'll go on and clean up those unprotected traversals. thanks, -john --=-kxRs9jYcnQnz6CtZyS9w 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.20 --- linux24/drivers/pci/pci.c:1.1.1.1 Tue Dec 18 15:49:16 2001 +++ linux24/drivers/pci/pci.c Mon Mar 4 17:03:27 2002 @@ -37,6 +37,7 @@ =20 LIST_HEAD(pci_root_buses); LIST_HEAD(pci_devices); +DECLARE_MUTEX(pcidevlist_sem); =20 /** * pci_find_slot - locate PCI device from a given PCI slot @@ -55,10 +56,14 @@ { struct pci_dev *dev; =20 + down(&pcidevlist_sem); 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){ + up(&pcidevlist_sem); return dev; + } } + up(&pcidevlist_sem); return NULL; } =20 @@ -81,17 +86,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 + down(&pcidevlist_sem); + 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)){ + up(&pcidevlist_sem); return dev; + } n =3D n->next; } + up(&pcidevlist_sem); return NULL; } =20 @@ -130,14 +141,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 + down(&pcidevlist_sem); + 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){ + up(&pcidevlist_sem); return dev; + } n =3D n->next; } + up(&pcidevlist_sem); return NULL; } =20 @@ -605,10 +621,12 @@ int count =3D 0; =20 list_add_tail(&drv->node, &pci_drivers); + down(&pcidevlist_sem); pci_for_each_dev(dev) { if (!pci_dev_driver(dev)) count +=3D pci_announce_device(drv, dev); } + up(&pcidevlist_sem); return count; } =20 @@ -628,6 +646,7 @@ struct pci_dev *dev; =20 list_del(&drv->node); + down(&pcidevlist_sem); pci_for_each_dev(dev) { if (dev->driver =3D=3D drv) { if (drv->remove) @@ -635,6 +654,7 @@ dev->driver =3D NULL; } } + up(&pcidevlist_sem); } =20 #ifdef CONFIG_HOTPLUG @@ -716,7 +736,9 @@ pci_insert_device(struct pci_dev *dev, struct pci_bus *bus) { list_add_tail(&dev->bus_list, &bus->devices); + down(&pcidevlist_sem); list_add_tail(&dev->global_list, &pci_devices); + up(&pcidevlist_sem); #ifdef CONFIG_PROC_FS pci_proc_attach_device(dev); #endif @@ -751,7 +773,9 @@ dev->driver =3D NULL; } list_del(&dev->bus_list); + down(&pcidevlist_sem);=20 list_del(&dev->global_list); + up(&pcidevlist_sem); pci_free_resources(dev); #ifdef CONFIG_PROC_FS pci_proc_detach_device(dev); @@ -1327,7 +1351,9 @@ * Link the device to both the global PCI device chain and * the per-bus list of devices. */ + down(&pcidevlist_sem); list_add_tail(&dev->global_list, &pci_devices); + up(&pcidevlist_sem); list_add_tail(&dev->bus_list, &bus->devices); =20 /* Fix up broken headers */ @@ -1935,9 +1961,11 @@ =20 pcibios_init(); =20 + down(&pcidevlist_sem); pci_for_each_dev(dev) { pci_fixup_device(PCI_FIXUP_FINAL, dev); } + up(&pcidevlist_sem); =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.3 --- linux24/drivers/pci/proc.c:1.1.1.1 Tue Dec 18 15:49:16 2001 +++ linux24/drivers/pci/proc.c Mon Mar 4 16:55:22 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; + down(&pcidevlist_sem); + 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){ + up(&pcidevlist_sem); return NULL; + } } + up(&pcidevlist_sem); return p; } + +/* +XXX: pci_seq_next() doesn't use pcidevlist_sem, make sure=20 +you down 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; + down(&pcidevlist_sem); pci_for_each_dev(dev) { pci_proc_attach_device(dev); } + up(&pcidevlist_sem); 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.4 --- 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 16:55:22 2002 @@ -222,9 +222,11 @@ ranges.found_vga =3D 0; pbus_assign_resources(b, &ranges); } + down(&pcidevlist_sem); pci_for_each_dev(dev) { pdev_enable_device(dev); } + up(&pcidevlist_sem); } =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.2 --- linux24/drivers/pci/setup-irq.c:1.1.1.1 Tue Dec 18 15:49:16 2001 +++ linux24/drivers/pci/setup-irq.c Mon Mar 4 16:55:22 2002 @@ -65,7 +65,9 @@ int (*map_irq)(struct pci_dev *, u8, u8)) { struct pci_dev *dev; + down(&pcidevlist_sem); pci_for_each_dev(dev) { pdev_fixup_irq(dev, swizzle, map_irq); } + up(&pcidevlist_sem); } 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.4 --- linux24/drivers/pnp/isapnp.c:1.1.1.2 Wed Feb 13 14:53:31 2002 +++ linux24/drivers/pnp/isapnp.c Mon Mar 4 16:55:22 2002 @@ -1685,10 +1685,14 @@ } #ifdef CONFIG_PCI if (!isapnp_skip_pci_scan) { + down(&pcidevlist_sem); pci_for_each_dev(dev) { - if (dev->irq =3D=3D irq) + if (dev->irq =3D=3D irq){ + up(&pcidevlist_sem); return 1; + } } + up(&pcidevlist_sem); } #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.9 --- linux24/include/linux/pci.h:1.1.1.1 Tue Dec 18 15:49:00 2001 +++ linux24/include/linux/pci.h Mon Mar 4 16:57:00 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 struct semaphore pcidevlist_sem; =20 /* * Error values that may be returned by PCI functions. --=-kxRs9jYcnQnz6CtZyS9w-- _______________________________________________ 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