Can we remove pci_find_device() yet? [Kernel]

Prev: [PATCH] drivers/net/skfp/skfddi.c: use %pMF to show MAC address
Next: elf coredump: Add extended numbering support

From: Andi Kleen on 14 Jan 2010 06:10

From: Tilman Schmidt on 14 Jan 2010 08:00

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 2010-01-14 12:03 schrieb Andi Kleen:
> On Thu, Jan 14, 2010 at 11:02:14AM +0000, Alan Cox wrote:
[It would be better to enforce this constraint at runtime.]
>> 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.
>
> You just have to do it once, otherwise it'll fail after 4 billion times.

I guess we could live with that. pci_find_device() or its successor, the
hisax_find_pci_device() hack, is only called during device setup, once
for most drivers and at the very most 24 times for hfc_pci. Someone
would have to rmmod/insmod a HiSax module more than a hundred million
times to accumulate four billion calls.

- --
Tilman Schmidt E-Mail: tilman(a)imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Unge�ffnet mindestens haltbar bis: (siehe R�ckseite)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.12 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAktPEHMACgkQQ3+did9BuFuQ/gCgkhu6RJIwrpIDAJMWbO1mZgOd
BKwAnAh6kX5F/+dFPIrK/XLovEAv/R7O
=fflE
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

From: Tilman Schmidt on 20 Jan 2010 10:20

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(a)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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

First | Prev |
Pages: 1 2 3 4
Prev: [PATCH] drivers/net/skfp/skfddi.c: use %pMF to show MAC address
Next: elf coredump: Add extended numbering support