Re: 2.6 probe.c "pcibus_class" Device Class, release function

[Matthew Dobson <colpatch@us.ibm.com>]

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

John Rose wrote:
> The function release_pcibus_dev() in probe.c defines the release procedure for
> device class pcibus_class.  I want to suggest that this function be scrapped :)
> 
> This release function is called in the code path of class_device_unregister().
> The pcibus_class devices aren't currently unregistered anywhere, from what I
> can tell, so this release function is currently unused.  The runtime removal of
> PCI buses from logical partitions on PPC64 requires the unregistration of these
> class devices.  The natural place to do this IMHO is in pci_remove_bus_device()
> in remove.c.  

You're right that the class device isn't currently unregistered, and 
that was an oversight in the patch I originally sent.  Attatched is a 
patch that remedies that situation.  pci_remove_bus_device() *is* the 
natural place to unregister the class_dev, and that's just what the 
patch does.


> The problem is that this calls pci_destroy_dev(), which calls put() on the same
> "bridge" device that the release function does.  This should only be done once
> in the course of removing a pci_bus, and I doubt that we want to change
> pci_destroy_dev().   The kfree() of the pci_bus struct is also done in both
> pci_remove_bus_device() and release_pcibus_dev().  

Yep.  The patch pulls the kfree() out of pci_remove_bus_device() and 
calls class_device_unregister() in it's place.  This will put() the 
bridge device and free the pci_bus as needed.  put() does need to be 
called twice because the bridge device is get()'d twice: once when the 
device is registered and once when it's bus device grabs a reference to it.


> So the only two operations in the release function are redundantly performed in
> the place where it makes sense to unregister.  For these reasons, I think we
> should scrap the release function altogether and set that pointer in the struct
> class to NULL.
> 
> Thoughts?
> John

Disagree, for the reasons above. ;)  Patch attatched.

Cheers!

-Matt


[-- Attachment #2: pcibus_memleak-fix.patch --]
[-- Type: text/plain, Size: 495 bytes --]

diff -Nurp --exclude-from=/home/mcd/.dontdiff linux-2.6.2-rc3/drivers/pci/remove.c linux-2.6.2-rc3+pcibus_memleak-fix/drivers/pci/remove.c
--- linux-2.6.2-rc3/drivers/pci/remove.c	Thu Jan  8 22:59:10 2004
+++ linux-2.6.2-rc3+pcibus_memleak-fix/drivers/pci/remove.c	Tue Feb  3 17:17:30 2004
@@ -83,7 +83,7 @@ void pci_remove_bus_device(struct pci_de
 		list_del(&b->node);
 		spin_unlock(&pci_bus_lock);
 
-		kfree(b);
+		class_device_unregister(&b->class_dev);
 		dev->subordinate = NULL;
 	}