svn commit: r215544 - head/sys/kern

[John Baldwin]

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 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.

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.

-- 
John Baldwin