PCI device list locking - take 3
RFC 1/1 PCI device list locking - take 3
Ok, I think I got it all covered this time :)
Here's the latest version of the pci list locking patch. I've taken Chris's comments and addressed them by making sure we don't walk off the end of a deleted device in the pci_find_* and pci_get_* functions.
This version has survived a pretty hard beating on a pci hotplug system, as proof of concept...
Comments?
thanks,
greg k-h
drivers/pci/bus.c +6 -0
@@ -93,7 +93,11 @@continue ;device_add (&dev->dev); + +spin_lock (&pci_bus_lock);list_add_tail (&dev->global_list, &pci_devices); +spin_unlock (&pci_bus_lock); +pci_proc_attach_device (dev);pci_create_sysfs_dev_files (dev);
@@ -108,7 +112,9 @@* it and then scan for unattached PCI devices. */ if (dev->subordinate &&list_empty (&dev->subordinate->node)) { +spin_lock (&pci_bus_lock);list_add_tail (&dev->subordinate->node, &dev->bus->children); +spin_unlock (&pci_bus_lock);pci_bus_add_devices (dev->subordinate); } }
drivers/pci/hotplug.c +23 -11
@@ -173,6 +173,24 @@ }EXPORT_SYMBOL (pci_visit_dev); +static void pci_destroy_dev (struct pci_dev *dev) +{ +device_unregister (&dev->dev); + +/* Remove the device from the device lists, and prevent any further +* list accesses from this device */ +spin_lock (&pci_bus_lock); +list_del (&dev->bus_list); +list_del (&dev->global_list); + dev->bus_list.next = dev->bus_list.prev =NULL ; + dev->global_list.next = dev->global_list.prev =NULL ; +spin_unlock (&pci_bus_lock); + +pci_free_resources (dev); +pci_proc_detach_device (dev); +pci_put_dev (dev); +} +/** * pci_remove_device_safe - remove an unused hotplug device * @dev: the device to remove
@@ -186,11 +204,7 @@ {if (pci_dev_driver (dev))return -EBUSY ; -device_unregister (&dev->dev); -list_del (&dev->bus_list); -list_del (&dev->global_list); -pci_free_resources (dev); -pci_proc_detach_device (dev); +pci_destroy_dev (dev);return 0 ; }EXPORT_SYMBOL (pci_remove_device_safe);
@@ -237,17 +251,15 @@pci_remove_behind_bridge (dev);pci_proc_detach_bus (b); +spin_lock (&pci_bus_lock);list_del (&b->node); +spin_unlock (&pci_bus_lock); +kfree (b); dev->subordinate =NULL ; } -device_unregister (&dev->dev); -list_del (&dev->bus_list); -list_del (&dev->global_list); -pci_free_resources (dev); -pci_proc_detach_device (dev); -pci_put_dev (dev); +pci_destroy_dev (dev); }/**
drivers/pci/pci.h +3 -0
@@ -59,3 +59,6 @@extern int pci_visit_dev (struct pci_visit *fn,struct pci_dev_wrapped *wrapped_dev,struct pci_bus_wrapped *wrapped_parent); + +/* Lock for read/write access to pci device and bus lists */ +extern spinlock_t pci_bus_lock;
drivers/pci/proc.c +25 -19
89 lines not on this page · raw message
drivers/pci/search.c +135 -16
222 lines not on this page · raw message
include/linux/pci.h +11 -0
@@ -566,6 +566,10 @@int pci_find_capability (struct pci_dev *dev,int cap);struct pci_bus *pci_find_next_bus (const struct pci_bus *from); +struct pci_dev *pci_get_device (unsigned int vendor,unsigned int device,struct pci_dev *from); +struct pci_dev *pci_get_subsys (unsigned int vendor,unsigned int device, +unsigned int ss_vendor,unsigned int ss_device, +struct pci_dev *from);int pci_bus_read_config_byte (struct pci_bus *bus,unsigned int devfn,int where,u8 *val);int pci_bus_read_config_word (struct pci_bus *bus,unsigned int devfn,int where,u16 *val);int pci_bus_read_config_dword (struct pci_bus *bus,unsigned int devfn,int where,u32 *val);
@@ -686,6 +690,13 @@static inline struct pci_dev *pci_find_subsys (unsigned int vendor,unsigned int device,unsigned int ss_vendor,unsigned int ss_device,const struct pci_dev *from) +{return NULL ; } + +static inline struct pci_dev *pci_get_device (unsigned int vendor,unsigned int device,struct pci_dev *from) +{return NULL ; } + +static inline struct pci_dev *pci_get_subsys (unsigned int vendor,unsigned int device, +unsigned int ss_vendor,unsigned int ss_device,struct pci_dev *from) {return NULL ; }static inline void pci_set_master (struct pci_dev *dev) { }
Quoted 3 lines · * Greg KH ([email protected]) wrote:
Here's the latest version of the pci list locking patch. I've taken Chris's comments and addressed them by making sure we don't walk off the end of a deleted device in the pci_find_* and pci_get_* functions.
Looks good. Perhaps the pci_proc_detach should be earlier (i.e. before list removal) to reflect the order in which things are added. I'm not sure of the dependencies, but seems a good practice anyway.
thanks,
-chris
Signature · 1 line
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
Quoted 8 lines · Chris Wright
Quoted 3 lines · * Greg KH ([email protected]) wrote:
Here's the latest version of the pci list locking patch. I've taken Chris's comments and addressed them by making sure we don't walk off the end of a deleted device in the pci_find_* and pci_get_* functions.
Looks good. Perhaps the
pci_proc_detachshould be earlier (i.e. before list removal) to reflect the order in which things are added. I'm not sure of the dependencies, but seems a good practice anyway.
Thanks, I'll go fix that one just to be safe.
greg k-h