Patch to prevent cycles in the devclass tree

[John Baldwin]

On Friday 06 January 2006 14:01, John Baldwin wrote:
> 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.

Comments, flames, etc.?

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