Date: Fri, 19 Nov 2010 23:41:00 +0100 From: Attilio Rao <attilio@freebsd.org> To: John Baldwin <jhb@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: <AANLkTi=5offAR1-Qs9EAHDpmTmw_d-XgLMRJx0Db7qgs@mail.gmail.com> In-Reply-To: <201011191716.03279.jhb@freebsd.org> References: <201011191943.oAJJhv3i087205@svn.freebsd.org> <201011191646.12106.jhb@freebsd.org> <201011191703.26742.jkim@FreeBSD.org> <201011191716.03279.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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: >> > > >> > =C2=A0 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. >> > > >> > >> > > >> > =C2=A0 This is a similar approach as the modules are unloaded i= n >> > > >> > the same linkerfile. >> > > >> > >> > > >> > =C2=A0 Sponsored by: =C2=A0 =C2=A0 Sandvine Incorporated >> > > >> > =C2=A0 Submitted by: =C2=A0 =C2=A0 Nima Misaghian <nmisaghian a= t sandvine >> > > >> > dot com> MFC after: =C2=A0 =C2=A0 =C2=A0 =C2=A01 week >> > > >> >> > > >> Hmm... =C2=A0It is not directly related but I was thinking about >> > > >> doing similar things for sys/kern/subr_bus.c. =C2=A0What 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 =C2=A0 =C2=A02010-11-19 >> > > > 17:19:02.000000000 00 @@ -3426,9 +3429,9 @@ >> > > > =C2=A0 =C2=A0 =C2=A0 =C2=A0TAILQ_FOREACH(child, &dev->children, li= nk) { >> > > > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D D= EVICE_SUSPEND(child); >> > > > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (error) = { >> > > > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 for (child2 =3D >> > > > TAILQ_FIRST(&dev->children); - >> > > > child2 && child2 !=3D child; - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0child2 >> > > > =3D TAILQ_NEXT(child2, link)) + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for (child2 >> > > > =3D TAILQ_PREV(child, device_list, link); + >> > > > =C2=A0 =C2=A0 =C2=A0child2 !=3D NULL; >> > > > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0child2 =3D TAILQ_PREV(child2, >> > > > device_list, link)) DEVICE_RESUME(child2); >> > > > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0return (error); >> > > > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} >> > > > @@ -3447,7 +3450,7 @@ >> > > > =C2=A0{ >> > > > =C2=A0 =C2=A0 =C2=A0 =C2=A0device_t =C2=A0 =C2=A0 =C2=A0 =C2=A0chi= ld; >> > > > >> > > > - =C2=A0 =C2=A0 =C2=A0 TAILQ_FOREACH(child, &dev->children, link) = { >> > > > + =C2=A0 =C2=A0 =C2=A0 TAILQ_FOREACH_REVERSE(child, &dev->children= , >> > > > device_list, link) { DEVICE_RESUME(child); >> > > > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* if resum= e 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. =C2=A0That is, if you have a PCI bus with two = em >> > interfaces, does it really matter if em0 suspends before em1 vs >> > after em1? =C2=A0I think it actually doesn't matter. =C2=A0The 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. =C2=A0For >> 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. =C2=A0If there are specific instan= ces > where there are dependencies, the drivers for that hardware should > manage that. =C2=A0If 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. =C2=A0We 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? Attilio --=20 Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTi=5offAR1-Qs9EAHDpmTmw_d-XgLMRJx0Db7qgs>