From owner-svn-src-all@FreeBSD.ORG Fri Nov 19 23:12:52 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from [127.0.0.1] (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by hub.freebsd.org (Postfix) with ESMTP id D30EC1065670; Fri, 19 Nov 2010 23:12:51 +0000 (UTC) (envelope-from jkim@FreeBSD.org) From: Jung-uk Kim To: John Baldwin Date: Fri, 19 Nov 2010 18:12:30 -0500 User-Agent: KMail/1.6.2 References: <201011191943.oAJJhv3i087205@svn.freebsd.org> <201011191703.26742.jkim@FreeBSD.org> <201011191716.03279.jhb@freebsd.org> In-Reply-To: <201011191716.03279.jhb@freebsd.org> MIME-Version: 1.0 Content-Disposition: inline Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201011191812.43712.jkim@FreeBSD.org> Cc: Attilio Rao , svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r215544 - head/sys/kern X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Nov 2010 23:12:52 -0000 On Friday 19 November 2010 05:16 pm, John Baldwin wrote: > 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 : > > > > > 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 > > > >> > 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. It may be unnecessary but I like consistency. ;-) Jung-uk Kim