[PATCH] 2.6 PPC64 EEH unbalanced dev_get/put calls

[PATCH] 2.6 PPC64 EEH unbalanced dev_get/put calls

[linas]

[-- Attachment #1: Type: text/plain, Size: 407 bytes --]


Hi Paul,

Please review and forward upstream the following patch.

This patch fixes some unbalanaced usage of pci_dev_get()/pci_dev_put() calls
in the eeh code.  The old code had too many calls to dev_put, which could
cause memory structs to be freed prematurely, possibly leading to bad
bad pointer derefs in certain cases.

Cross-ref LTC bug 9283

Signed-off-by: Linas Vepstas <linas@linas.org>

--linas

[-- Attachment #2: eeh-unbalanced-dev-put-for-bkbits.patch --]
[-- Type: text/plain, Size: 925 bytes --]

===== arch/ppc64/kernel/eeh.c 1.20 vs edited =====
--- 1.20/arch/ppc64/kernel/eeh.c	Thu Jul  1 17:27:56 2004
+++ edited/arch/ppc64/kernel/eeh.c	Fri Jul  2 13:27:58 2004
@@ -207,7 +207,6 @@
 	if (!dn) {
 		printk(KERN_WARNING "PCI: no pci dn found for dev=%s %s\n",
 			pci_name(dev), pci_pretty_name(dev));
-		pci_dev_put(dev);
 		return;
 	}
 
@@ -218,9 +217,9 @@
 		printk(KERN_INFO "PCI: skip building address cache for=%s %s\n",
 		       pci_name(dev), pci_pretty_name(dev));
 #endif
-		pci_dev_put(dev);
 		return;
 	}
+	pci_dev_get(dev);
 
 	/* Walk resources on this device, poke them into the tree */
 	for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
@@ -310,7 +309,6 @@
 	while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
 		/* Ignore PCI bridges ( XXX why ??) */
 		if ((dev->class >> 16) == PCI_BASE_CLASS_BRIDGE) {
-			pci_dev_put(dev);
 			continue;
 		}
 		pci_addr_cache_insert_device(dev);

Re: [PATCH] 2.6 PPC64 EEH unbalanced dev_get/put calls

[Paul Mackerras]

Linas,

> This patch fixes some unbalanaced usage of pci_dev_get()/pci_dev_put() calls
> in the eeh code.  The old code had too many calls to dev_put, which could
> cause memory structs to be freed prematurely, possibly leading to bad
> bad pointer derefs in certain cases.

When I apply this I end up with one pci_dev_get() call in
__pci_addr_cache_insert_device and no pci_dev_put() calls.  That can't
be right, surely?  If it is it needs a big fat comment explaining why.

> Cross-ref LTC bug 9283

Confused - that's the bug about not using ibm,fw-phb-id.

Paul.

Re: [PATCH] 2.6 PPC64 EEH unbalanced dev_get/put calls

[linas]

On Sat, Jul 03, 2004 at 03:13:02PM +1000, Paul Mackerras wrote:
> Linas,
> 
> > This patch fixes some unbalanaced usage of pci_dev_get()/pci_dev_put() calls
> > in the eeh code.  The old code had too many calls to dev_put, which could
> > cause memory structs to be freed prematurely, possibly leading to bad
> > bad pointer derefs in certain cases.
> 
> When I apply this I end up with one pci_dev_get() call in
> __pci_addr_cache_insert_device and no pci_dev_put() calls.  That can't
> be right, surely?  If it is it needs a big fat comment explaining why.


No, that's right. The device is gotten for the length of time that 
it is in the cache, and is put when it is removed from the cache.
In this way, the device is not free()'ed as long as the cache holds 
a reference to it.

I can add a comment, but it seemed 'obvious' from the description of the
cache that it would be holding a reference to the device for an indefinite 
period of time.

The patch was really to fix the result of a misunderstanding of what 
the poorly-named routine "pci_get_device()" does:  yes, it does a get(), 
but it also does a put(), which one wouldn't guess from the name :(

A better name for this might be "pci_next_device()" or 
"pci_obtain_device()" or something like that ...


> > Cross-ref LTC bug 9283
> Confused - that's the bug about not using ibm,fw-phb-id.

Oops.

--linas