Date: Wed, 15 Nov 2006 16:14:45 -0700 (MST) From: "M. Warner Losh" <imp@bsdimp.com> To: scottl@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 Message-ID: <20061115.161445.43008442.imp@bsdimp.com> In-Reply-To: <455B9D3F.5010703@samsco.org> References: <20061115.155143.1021575615.imp@bsdimp.com> <200611151756.31047.jhb@freebsd.org> <455B9D3F.5010703@samsco.org>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <455B9D3F.5010703@samsco.org>
Scott Long <scottl@samsco.org> writes:
: John Baldwin wrote:
: > On Wednesday 15 November 2006 17:51, M. Warner Losh wrote:
: >> In message: <455B963A.4050200@samsco.org>
: >> Scott Long <scottl@samsco.org> 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.
: >
:
: As unpleasant as this is, I think it needs to be up to the driver to
: print the line, since John is correct about having informational and
: error messages come after the device message instead of before. This
: means changing every driver to intentionally call a method to do the
: printf. Ugly.
I think that's too ugly to contemplate... One of the main reasons it
is done in the parent bus now is that too many drivers got it wrong,
or had different notions of what they thought "right" output should
be.
: An alternative might be to add magic to device_printf() so that anything
: printed through it gets buffered and delayed until after the attach
: routine completes (successfully or not). Once it completes, the
: device info line gets printed out with the actual resources (subject to
: what Warner is saying), and then all of the buffered device_printfs get
: dumped out.
I'm not sure this is much better.
Another problem with doing after rather than before is hangs. Right
now if the fxp driver is wedging my system on attach, I get to see the
fxp driver's dmesg line as the last thing before the wedge. If we
move to after, or driver prints, this line wouldn't appear.
Another option would be to omit resources entirely and go to a two
line dmesg. First line would be location, second line would be
resources (maybe hidden under bootverbose).
Warner
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20061115.161445.43008442.imp>