Re: Latest bk kernel does not properly free PCI IO & MEM allocations

[Prarit Bhargava <prarit@sgi.com>]

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

Hi Greg,

Greg KH wrote:

>pci_free_resources is not a legacy call, it's a function internal to the
>pci core.
>  
>
Sorry -- I read that back and realize how incorrect that was.

What I meant to say was pci_free_resources calls release_resource, where
release_region calls __release_region.  __release_region is called a 
"legacy" function?  I
assumed this meant that it was to be deprecated?  Oh ... Uh ... does the 
term Legacy
really refer to Legacy address space?  (Prarit tries not to look 
sheepish asking that question.)

>Does the following patch fix the issue for you?
>
>  
>
Nope, because the res pointer still points to an "old" memory address -- 
then the res value check succeeds and I would oops if res->parent was 
NULL. (Haven't tried it but I think it will still die if SLAB debug is 
on ...)

How about the attached patch?  (I think the warning is a bit wordy ... 
if you have a better idea I'm all for it ... If you want to dump the 
warning I'm fine with that too...)

P.

P.S.  Greg, I've sent 3 separate requests to the linux PCI list but 
haven't heard anything back.  Do you know who the maintainer is?  ... 
I'll double check my spam filter ...




[-- Attachment #2: pci.patch --]
[-- Type: text/x-patch, Size: 928 bytes --]

===== drivers/pci/remove.c 1.6 vs edited =====
--- 1.6/drivers/pci/remove.c	2005-01-14 18:06:55 -05:00
+++ edited/drivers/pci/remove.c	2005-03-11 21:18:22 -05:00
@@ -19,8 +19,12 @@
 	pci_cleanup_rom(dev);
 	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
 		struct resource *res = dev->resource + i;
-		if (res->parent)
+		if (res && res->parent) {
+			printk(KERN_WARNING "WARNING: PCI RESOURCE 0x%lx:0x%lx for dev %s free'd by PCI subsystem.",
+				pci_resource_start(dev,i), pci_resource_end(dev,i), dev->slot_name);
+			printk(KERN_WARNING " This should be done at the driver level.");
 			release_resource(res);
+		}
 	}
 }
 
===== kernel/resource.c 1.26 vs edited =====
--- 1.26/kernel/resource.c	2005-01-08 00:44:13 -05:00
+++ edited/kernel/resource.c	2005-03-11 20:56:47 -05:00
@@ -505,6 +505,7 @@
 			*p = res->sibling;
 			write_unlock(&resource_lock);
 			kfree(res);
+			res = NULL;
 			return;
 		}
 		p = &res->sibling;