Date: Mon, 22 Nov 2010 09:32:48 -0500 From: John Baldwin <jhb@freebsd.org> To: Attilio Rao <attilio@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Jung-uk Kim <jkim@freebsd.org> Subject: Re: svn commit: r215544 - head/sys/kern Message-ID: <201011220932.48629.jhb@freebsd.org> In-Reply-To: <AANLkTi=5offAR1-Qs9EAHDpmTmw_d-XgLMRJx0Db7qgs@mail.gmail.com> References: <201011191943.oAJJhv3i087205@svn.freebsd.org> <201011191716.03279.jhb@freebsd.org> <AANLkTi=5offAR1-Qs9EAHDpmTmw_d-XgLMRJx0Db7qgs@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Friday, November 19, 2010 5:41:00 pm Attilio Rao wrote: > 2010/11/19 John Baldwin <jhb@freebsd.org>: > > On Friday, November 19, 2010 5:03:25 pm Jung-uk Kim wrote: > >> On Friday 19 November 2010 04:46 pm, John Baldwin wrote: > >> > On Friday, November 19, 2010 4:31:44 pm Attilio Rao wrote: > >> > > 2010/11/19 John Baldwin <jhb@freebsd.org>: > >> > > > On Friday, November 19, 2010 4:09:28 pm Jung-uk Kim wrote: > >> > > >> On Friday 19 November 2010 02:43 pm, Attilio Rao wrote: > >> > > >> > Author: attilio > >> > > >> > Date: Fri Nov 19 19:43:56 2010 > >> > > >> > New Revision: 215544 > >> > > >> > URL: http://svn.freebsd.org/changeset/base/215544 > >> > > >> > > >> > > >> > Log: > >> > > >> > Scan the list in reverse order for the shutdown handlers > >> > > >> > of loaded modules. This way, when there is a dependency > >> > > >> > between two modules, the handler of the latter probed runs > >> > > >> > first. > >> > > >> > > >> > > >> > This is a similar approach as the modules are unloaded in > >> > > >> > the same linkerfile. > >> > > >> > > >> > > >> > Sponsored by: Sandvine Incorporated > >> > > >> > Submitted by: Nima Misaghian <nmisaghian at sandvine > >> > > >> > dot com> MFC after: 1 week > >> > > >> > >> > > >> Hmm... It is not directly related but I was thinking about > >> > > >> doing similar things for sys/kern/subr_bus.c. What do you > >> > > >> think about the attached patch? > >> > > > > >> > > > Hmm, the patches for suspend and resume that I had for this > >> > > > took the opposite order, they did suspend in forward order, but > >> > > > resume in backwards order. Like so: > >> > > > > >> > > > --- //depot/vendor/freebsd/src/sys/kern/subr_bus.c > >> > > > 2010-11-17 22:30:24.000000000 0000 +++ > >> > > > //depot/user/jhb/acpipci/kern/subr_bus.c 2010-11-19 > >> > > > 17:19:02.000000000 00 @@ -3426,9 +3429,9 @@ > >> > > > TAILQ_FOREACH(child, &dev->children, link) { > >> > > > error = DEVICE_SUSPEND(child); > >> > > > if (error) { > >> > > > - for (child2 = > >> > > > TAILQ_FIRST(&dev->children); - > >> > > > child2 && child2 != child; - child2 > >> > > > = TAILQ_NEXT(child2, link)) + for (child2 > >> > > > = TAILQ_PREV(child, device_list, link); + > >> > > > child2 != NULL; > >> > > > + child2 = TAILQ_PREV(child2, > >> > > > device_list, link)) DEVICE_RESUME(child2); > >> > > > return (error); > >> > > > } > >> > > > @@ -3447,7 +3450,7 @@ > >> > > > { > >> > > > device_t child; > >> > > > > >> > > > - TAILQ_FOREACH(child, &dev->children, link) { > >> > > > + TAILQ_FOREACH_REVERSE(child, &dev->children, > >> > > > device_list, link) { DEVICE_RESUME(child); > >> > > > /* if resume fails, there's nothing we can > >> > > > usefully do... */ } > >> > > > > >> > > > (Likely mangled whitespace.) > >> > > > > >> > > > I couldn't convince myself which order was "more" correct for > >> > > > suspend and resume. > >> > > > >> > > Considering loading in starting point, I think suspend should go > >> > > in reverse logic and resume in the same module load logic. > >> > > So that dependent modules are suspended first and resumed after. > >> > > Don't you agree? > >> > > >> > These are devices, and the ordering here is the order of sibling > >> > devices on a given bus. That is, if you have a PCI bus with two em > >> > interfaces, does it really matter if em0 suspends before em1 vs > >> > after em1? I think it actually doesn't matter. The passes from > >> > the multipass boot probe might make some sense to order on. > >> > However, I don't think the order of siblings on a bus is meaningful > >> > for suspend and resume (which is why I've never committed the above > >> > patches). > >> > > >> > Specifically, there is no dependency relationship between siblings > >> > on a bus. Certain buses may in fact have a dependency order of > >> > sorts (vgapci0 comes to mind), but those buses should manage that. > >> > There is no generic dependency between siblings that should be > >> > encoded into subr_bus.c > >> > >> Generally siblings don't interact with each other directly, no. > >> However, some modern chipsets *do* have strong relationship. For > >> example, some chipsets reference SMB controller to get current > >> configuration, e.g., function A depends on function B on the same > >> chip. > > > > That may be true, but that is not generic to all buses and devices. > > That isn't even really generic to PCI. If there are specific instances > > where there are dependencies, the drivers for that hardware should > > manage that. If specific buses have specific orders, then they can > > manage that order explicitly in their own suspend and resume routines. > > However, I don't see a valid reason for enforcing a particular order > > among siblings for all devices. We certainly do enforce some orders > > with respect to children and parents (parents attach first and resume > > first, children detach first and suspend first, PCI even mandates this > > for powering down a bus), but the same is not true for siblings. > > However, it would be good if any ordering is maintained between > devices linked via MODDEPEND stuff. > It happens they may also be sibillings, and not havign parent-child > relationship in some cases, but I'm also wondering if maybe they > should ensure consistency in their code itself rather than relying > strongly on ordering by the PCi/whatever subsystem? Almost all of those devices have parent -> child relationships, not sibling relationships (e.g. NIC drivers that depend on certain phy drivers). Do you have a specific example of drivers that have dependencies where the current order is not correct? -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201011220932.48629.jhb>