Daemon News Ezine BSD News BSD Mall BSD Support Forum BSD Advocacy BSD Updates

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Patch to prevent cycles in the devclass tree



I have been doing some work recently to make the ACPI and OFW (OpenFirmware) 
PCI bus drivers inherit from the PCI bus driver.  As part of this I've run 
into an issue because the subclass drivers (ACPI PCI and OFW PCI) use the 
same name "pci" as the superclass.  They do this so that they can override 
the superclass when a "pci" device is added as a child of a "pcib" device.  
The problem I ran into is that when the subclass was registered, it called 
devclass_find_internal() with the parentname set to "pci".  This caused the 
"pci" devclass to set its dc->parent member to point to itself.  Thus, if in 
device_probe_child() we didn't find a suitable driver in the first pass of 
the for loop, we'd keep looping forever and hang during boot.

This is the fix I'm currently using but I was curious about feedback on it.  
It only checks to make sure the a devclass doesn't add itself as a parent, it 
doesn't walk up hierarchy to avoid a cycle completely:

--- //depot/vendor/freebsd/src/sys/kern/subr_bus.c	2005/10/04 22:25:30
+++ //depot/user/jhb/acpipci/kern/subr_bus.c	2006/01/04 21:32:26
@@ -781,7 +781,17 @@
 
 		bus_data_generation_update();
 	}
-	if (parentname && dc && !dc->parent) {
+
+	/*
+	 * If a parent class is specified, then set that as our parent so
+	 * that this devclass will support drivers for the parent class as
+	 * well.  If the parent class has the same name don't do this though
+	 * as it creates a cycle that can trigger an infinite loop in
+	 * device_probe_child() if a device exists for which there is no
+	 * suitable driver.
+	 */
+	if (parentname && dc && !dc->parent &&
+	    strcmp(classname, parentname) != 0) {
 		dc->parent = devclass_find_internal(parentname, 0, FALSE);
 	}
 
@@ -1240,6 +1250,7 @@
 void
 devclass_set_parent(devclass_t dc, devclass_t pdc)
 {
+	KASSERT(dc != pdc, ("%s: loop", __func__));
 	dc->parent = pdc;
 }
 
The other possible direction is to rename the subclasses to not use the same 
name ("pci"), but doing that actually involves a fair bit of work as it means 
teaching the various pcib drivers to create acpi_pci child devices rather 
than pci child devices, etc.

-- 
John Baldwin <john@xxxxxxxxxx>  <><  http://www.baldwin.cx/~john/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org