Skip to contentSkip to conversation

pci_find_device to

PATCH Greg KH 2 participants 2 messages Sep 21, 2004 Kernel janitors

lore
Share & tools
lore

Greg KH

author
#0 lore raw
Quoted 33 lines · Scott Feldman

Use pci_get_device instead of pci_find_device as pci device returned from pci_find_device can disappear at any momemt in time.

Signed-off-by Scott Feldman <[email protected]>

diff -Naurp linux-2.6.9-rc2-mm1/drivers/net/tg3.c linux-2.6.9-rc2-mm1-dsf/drivers/net/tg3.c

drivers/net/tg3.c +4 -10
@@ -7325,6 +7325,7 @@ static int __devinit tg3_get_invariants(
 	u32 pci_state_reg, grc_misc_cfg;
 	u32 val;
 	u16 pci_cmd;
+	struct pci_dev *pdev;
 	int err;
 
 #ifdef CONFIG_SPARC64

@@ -7338,17 +7339,19 @@ static int __devinit tg3_get_invariants(
 	 * every mailbox register write to force the writes to be
 	 * posted to the chip in order.
 	 */
-	if (pci_find_device(PCI_VENDOR_ID_INTEL,
-			    PCI_DEVICE_ID_INTEL_82801AA_8, NULL) ||
-	    pci_find_device(PCI_VENDOR_ID_INTEL,
-			    PCI_DEVICE_ID_INTEL_82801AB_8, NULL) ||
-	    pci_find_device(PCI_VENDOR_ID_INTEL,
-			    PCI_DEVICE_ID_INTEL_82801BA_11, NULL) ||
-	    pci_find_device(PCI_VENDOR_ID_INTEL,
-			    PCI_DEVICE_ID_INTEL_82801BA_6, NULL) ||
-	    pci_find_device(PCI_VENDOR_ID_AMD,
-			    PCI_DEVICE_ID_AMD_FE_GATE_700C, NULL))
+	if ((pdev = pci_get_device(PCI_VENDOR_ID_INTEL,
+			    PCI_DEVICE_ID_INTEL_82801AA_8, NULL)) ||
+	    (pdev = pci_get_device(PCI_VENDOR_ID_INTEL,

drivers/net/tg3.c static int __devinit tg3_get_invariants( · line 7354

Quoted 10 lines
drivers/net/tg3.c +9 -0
+			    PCI_DEVICE_ID_INTEL_82801AB_8, NULL)) ||
+	    (pdev = pci_get_device(PCI_VENDOR_ID_INTEL,
+			    PCI_DEVICE_ID_INTEL_82801BA_11, NULL)) ||
+	    (pdev = pci_get_device(PCI_VENDOR_ID_INTEL,
+			    PCI_DEVICE_ID_INTEL_82801BA_6, NULL)) ||
+	    (pdev = pci_get_device(PCI_VENDOR_ID_AMD,
+			    PCI_DEVICE_ID_AMD_FE_GATE_700C, NULL))) {
 		tp->tg3_flags |= TG3_FLAG_MBOX_WRITE_REORDER;
+		pci_dev_put(pdev);
+	}

If you have more than one device that matches these different tests, you will have grabbed a reference and lost it, without freeing it.

What we need is a simple "Is this pci device present right now" type function, to solve the mess that logic like this needs.

thanks,

greg k-h

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#1 lore raw
Quoted 22 lines · Greg KH
Quoted 22 lines
(quoted diff) +8 -10
@@ -7338,17 +7339,19 @@ static int __devinit tg3_get_invariants(
 	 * every mailbox register write to force the writes to be
 	 * posted to the chip in order.
 	 */
-	if (pci_find_device(PCI_VENDOR_ID_INTEL,
-			    PCI_DEVICE_ID_INTEL_82801AA_8, NULL) ||
-	    pci_find_device(PCI_VENDOR_ID_INTEL,
-			    PCI_DEVICE_ID_INTEL_82801AB_8, NULL) ||
-	    pci_find_device(PCI_VENDOR_ID_INTEL,
-			    PCI_DEVICE_ID_INTEL_82801BA_11, NULL) ||
-	    pci_find_device(PCI_VENDOR_ID_INTEL,
-			    PCI_DEVICE_ID_INTEL_82801BA_6, NULL) ||
-	    pci_find_device(PCI_VENDOR_ID_AMD,
-			    PCI_DEVICE_ID_AMD_FE_GATE_700C, NULL))
+	if ((pdev = pci_get_device(PCI_VENDOR_ID_INTEL,
+			    PCI_DEVICE_ID_INTEL_82801AA_8, NULL)) ||
+	    (pdev = pci_get_device(PCI_VENDOR_ID_INTEL,
+			    PCI_DEVICE_ID_INTEL_82801AB_8, NULL)) ||
+	    (pdev = pci_get_device(PCI_VENDOR_ID_INTEL,
+			    PCI_DEVICE_ID_INTEL_82801BA_11, NULL)) ||
+	    (pdev = pci_get_device(PCI_VENDOR_ID_INTEL,
+			    PCI_DEVICE_ID_INTEL_82801BA_6, NULL)) ||
Quoted 8 lines

Review static int __devinit tg3_get_invariants( · line 7354

Quoted 5 lines
(quoted diff) +4 -0
+	    (pdev = pci_get_device(PCI_VENDOR_ID_AMD,
+			    PCI_DEVICE_ID_AMD_FE_GATE_700C, NULL))) {
 		tp->tg3_flags |= TG3_FLAG_MBOX_WRITE_REORDER;
+		pci_dev_put(pdev);
+	}

If you have more than one device that matches these different tests, you will have grabbed a reference and lost it, without freeing it.

The || short-circuits. As soon as one of these returns a non-null pdev, the others won't be called.

Signature · 6 lines
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors