svn commit: r215544 - head/sys/kern
Jung-uk Kim
jkim at FreeBSD.org
Fri Nov 19 22:03:35 UTC 2010
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 at 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.
Jung-uk Kim
More information about the svn-src-head
mailing list