From owner-cvs-src@FreeBSD.ORG Wed Nov 15 23:56:14 2006 Return-Path: X-Original-To: cvs-src@freebsd.org Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id BE96316A407; Wed, 15 Nov 2006 23:56:14 +0000 (UTC) (envelope-from scottl@samsco.org) Received: from pooker.samsco.org (pooker.samsco.org [168.103.85.57]) by mx1.FreeBSD.org (Postfix) with ESMTP id 50C9643D55; Wed, 15 Nov 2006 23:56:14 +0000 (GMT) (envelope-from scottl@samsco.org) Received: from [192.168.254.11] (phobos.samsco.home [192.168.254.11]) (authenticated bits=0) by pooker.samsco.org (8.13.4/8.13.4) with ESMTP id kAFNu5rs008011; Wed, 15 Nov 2006 16:56:11 -0700 (MST) (envelope-from scottl@samsco.org) Message-ID: <455BA914.7010602@samsco.org> Date: Wed, 15 Nov 2006 16:56:04 -0700 From: Scott Long User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.7) Gecko/20060910 SeaMonkey/1.0.5 MIME-Version: 1.0 To: "M. Warner Losh" References: <200611151756.31047.jhb@freebsd.org> <20061115.160821.1973602865.imp@bsdimp.com> <455BA0B6.1000109@samsco.org> <20061115.165057.-1632628388.imp@bsdimp.com> In-Reply-To: <20061115.165057.-1632628388.imp@bsdimp.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-1.4 required=3.8 tests=ALL_TRUSTED autolearn=failed version=3.1.1 X-Spam-Checker-Version: SpamAssassin 3.1.1 (2006-03-10) on pooker.samsco.org Cc: cvs-src@freebsd.org, src-committers@freebsd.org, ru@freebsd.org, cvs-all@freebsd.org, jhb@freebsd.org Subject: Re: cvs commit: src/sys/dev/bce if_bce.c src/sys/dev/em if_em.c if_em.h src/sys/dev/mpt mpt.h mpt_pci.c X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Nov 2006 23:56:14 -0000 M. Warner Losh wrote: > In message: <455BA0B6.1000109@samsco.org> > Scott Long writes: > : M. Warner Losh wrote: > : > In message: <200611151756.31047.jhb@freebsd.org> > : > John Baldwin writes: > : > : On Wednesday 15 November 2006 17:51, M. Warner Losh wrote: > : > : > In message: <455B963A.4050200@samsco.org> > : > : > Scott Long writes: > : > : > : John Baldwin wrote: > : > : > : > On Wednesday 15 November 2006 16:51, Ruslan Ermilov wrote: > : > : > : >> On Thu, Nov 16, 2006 at 12:51:19AM +0300, Ruslan Ermilov wrote: > : > : > : >>> On Wed, Nov 15, 2006 at 08:04:57PM +0000, John Baldwin wrote: > : > : > : >>>> jhb 2006-11-15 20:04:57 UTC > : > : > : >>>> > : > : > : >>>> FreeBSD src repository > : > : > : >>>> > : > : > : >>>> Modified files: > : > : > : >>>> sys/dev/bce if_bce.c > : > : > : >>>> sys/dev/em if_em.c if_em.h > : > : > : >>>> sys/dev/mpt mpt.h mpt_pci.c > : > : > : >>>> Log: > : > : > : >>>> Add MSI support to em(4), bce(4), and mpt(4). For now, we only > : > : > : > support > : > : > : >>>> devices that support a maximum of 1 message, and we use that 1 > : > : message > : > : > : >>>> instead of the INTx rid 0 IRQ with the same interrupt handler, etc. > : > : > : >>>> > : > : > : >>>> Revision Changes Path > : > : > : >>>> 1.19 +11 -3 src/sys/dev/bce/if_bce.c > : > : > : >>>> 1.164 +11 -2 src/sys/dev/em/if_em.c > : > : > : >>>> 1.56 +1 -0 src/sys/dev/em/if_em.h > : > : > : >>>> 1.31 +1 -0 src/sys/dev/mpt/mpt.h > : > : > : >>>> 1.39 +14 -1 src/sys/dev/mpt/mpt_pci.c > : > : > : >>>> > : > : > : >>> How will the "vmstat -i" output look like for MSI-enabled devices? > : > : > : >>> > : > : > : >> irqXXXX, where XXXX>=1024? > : > : > : > > : > : > : > s/1024/256/ > : > : > : > > : > : > : > : > : > : There is a problem here, though. Newbus prints out the IRQ number after > : > : > : a successful device probe phase. It has no knowledge of MSI at that > : > : > : point, so it just prints out the traditional IRQ value. At some point, > : > : > : this needs to be fixed. Having the driver tell newbus about its MSI > : > : > : intentions in the probe routine is unrealistic, so there is no quick > : > : > : fix there. Probably need to delay printing the device message until > : > : > : later in the attach routine, once the driver has set up all of the > : > : > : resources. > : > : > > : > : > I've been wanting to move the printing of the attach string from > : > : > post-probe, pre-attach to post-attach for some time now. We would > : > : > then report the resources assigned to the device. I'm not sure if I'd > : > : > print all the resources assigned, or only those the driver activates. > : > : > Both sides of the argument have merit, imho. On the pro side, > : > : > resources are used, and printing them will help highlight conflicts. > : > : > On the con side, people think it clutters things up too much and might > : > : > lead to false expectations. > : > : > > : > : > When I've mentioned this desire at various developer summits, I was > : > : > told basically "go for it, but only at freebsd X.0 since people have > : > : > dmesg parsers" by many people (maybe even including Scott). > : > : > : > : Also, the way it works now, if attach fails and the routine prints out error > : > : messages, you get to see line for the device first with the initial resources > : > : and then you see the error message from the driver. If you move the printf > : > : down, then all you get is an error message from the driver. At the very > : > : least this would break POLA for a lot of our users. I'm still undecided. > : > : I could add a printf when a device ends up succesfully allocating MSI or MSI-X > : > : IRQs if people desired. > : > > : > I'd prefer those be under bootverbose. > : > > : > I think the real answer may be to get devinfo output to be real... > : > > : > Warner > : > : Yeah, the resource info really is superfluous during boot. It looks > : cool in the old-school unix way, but that's about it. Maybe it's time > : to change newbus to only print the driver name and driver-supplied > : description after the probe phase, like you hint at. newbus could then > : print resource info as the driver activates it, if bootverbose is set. > : I'd support something like this for 7.0, even if devinfo doesn't get > : fully fixed. If MSI gets merged to RELENG_6, then we'll just have to > : live with the incorrect newbus prints. > : > : So for example, I would change this: > : > : atapci0: port > : 0x1f0-0x1f7,0x3f6,0x170-0x177,0x376,0xfc00-0xfc0f at device 31.2 on pci0 > : > : into: > : > : atapci0: at device 31.2 on pci0 > > That's exactly what I'm thinking too. Glad to see we're on the same > page. > > : Calls to bus_activate_resouce/bus_alloc_resource(RF_ACTIVE) could then > : print the various resources, contingent on bootverbose being set. This > : would make the ugly [GIANT_LOCKED] and [FAST] messages that get printed > : right now have a chance of being more consistent. > > I was thinking that if we printed things last, we could potentially > eliminate them from normal use and just print them as part of the > resources (optionally?): > > atapci0: port 0x1f0-0x1f7,0x3f6,0x170-0x177,0x376,0xfc00-0xfc0f irq 23 [FAST] > > or something similar... > > Warner Sounds good. Beware that MSI makes it possible to have as many as 64 interrupts per device, though it's more common to have anywhere from 1 to 8. I'd probably split up your above line into 2 lines, one for memory resources and one for interrupt resources. Scott