Messages in this thread | | | Date | Thu, 21 Aug 2008 13:40:57 -0700 | From | Greg KH <> | Subject | Re: refcount leak in pci_get_device()? |
| |
On Thu, Aug 21, 2008 at 02:19:18PM -0600, Alex Chiang wrote: > Hi Greg, > > While playing around with my slot symlink stuff, I noticed that > the following sequence is problematic: > > 1. clean boot > 2. modprobe acpiphp > 3. echo 0 > /sys/bus/pci/slots/N/power > 4. ??? > > After step 3, we *should* be seeing pci_release_dev() getting > called, but we never do because the refcount on the device is > still quite high (5 or 6, on my ia64 system). > > I'm still trying to track this down, but I did notice, via code > inspection, at least one suspicious area: > > #define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL) > > That eventually calls pci_get_dev_by_id(), which increases the > refcount on the device, but never decrements it. > > Looks like that change in behavior happened here: > > PCI: clean up search.c a lot > 95247b57ed844511a212265b45cf9a919753aea1 > > pci_get_device() used to decrement the refcount, but no longer > does.
No, pci_get_device() never decremented the refcount, and that didn't change in the above git commit.
The description of pci_get_device() says that a reference is grabbed: Iterates through the list of known PCI devices. If a PCI device is found with a matching @vendor and @device, the reference count to the device is incremented and a pointer to its device structure is returned. Otherwise, %NULL is returned. A new search is initiated by passing %NULL as the @from argument. Otherwise if @from is not %NULL, searches continue from next device on the global list. The reference count for @from is always decremented if it is not %NULL.
All of the pci_find* functions should not have grabbed a reference to the device, as that was the "old" behavior. All of the pci_get* functions do grab a reference.
Did I somehow mess up and one of the pci_find* functions now improperly increment a reference? Hopefully we shouldn't be using those functions anymore as they aren't hotplug safe...
thanks,
greg k-h
| |