Re: Can we remove pci_find_device() yet?

[Tilman Schmidt <tilman@imap.cc>]

On Thu, Jan 14, 2010 at 11:02:14AM +0000, Alan Cox wrote:
> > e.g. set some global variable that forbids device removal
> > and warn in the kernel log. In theory this could be also
> > done per device, but I guess that would be more effort.
> 
> There is a simpler way to do that, which is to just leak a reference in
> the hisax_find_pci_device hack. The pci_dev won't be going anywhere then.

Something like this? (On top of my previous patch introducing
hisax_find_pci_device in the first place.)

Subject: [PATCH RFC] isdn/hisax: intentionally leak a reference in hisax_find_pci_device

Take a reference to the returned pci_dev which will never be released.
Until the HiSax drivers are fixed for proper PCI hotplug support,
this appears as the lesser evil compared to the danger of the
structure being freed without the caller knowing it.

Impact: replacing a bug by another, less consequential one
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/hisax/hisax.h |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/isdn/hisax/hisax.h b/drivers/isdn/hisax/hisax.h
index 832a878..e459502 100644
--- a/drivers/isdn/hisax/hisax.h
+++ b/drivers/isdn/hisax/hisax.h
@@ -1328,21 +1328,18 @@ void TeiFree(void);
 
 #include <linux/pci.h>
 
-/* adaptation wrapper for old usage
- * WARNING! This is unfit for use in a PCI hotplug environment,
- * as the returned PCI device can disappear at any moment in time.
+/* This function replaces the former function pci_find_device().
+ * Unlike the latter, it takes a reference on the returned pci_dev,
+ * resulting in a reference leak. This is intentional and serves to
+ * prevent an untimely release of the PCI device structure.
  * Callers should be converted to use pci_get_device() instead.
  */
 static inline struct pci_dev *hisax_find_pci_device(unsigned int vendor,
 						    unsigned int device,
 						    struct pci_dev *from)
 {
-	struct pci_dev *pdev;
-
 	pci_dev_get(from);
-	pdev = pci_get_subsys(vendor, device, PCI_ANY_ID, PCI_ANY_ID, from);
-	pci_dev_put(pdev);
-	return pdev;
+	return pci_get_subsys(vendor, device, PCI_ANY_ID, PCI_ANY_ID, from);
 }
 
 #endif
-- 
1.6.5.3.298.g39add