Re: Patch to prevent cycles in the devclass tree

On Tuesday 17 January 2006 19:25, Warner Losh wrote:
> From: John Baldwin <[email protected]>
> Subject: Re: Patch to prevent cycles in the devclass tree
> Date: Tue, 17 Jan 2006 13:08:40 -0500
>
> > 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.?
>
> I think this is right.

I think so too.
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-new-bus
To unsubscribe, send any mail to "[email protected]"