svn commit: r186182 - head/sys/dev/ata

[Alfred Perlstein]

* M. Warner Losh <imp at bsdimp.com> [081216 11:06] wrote:
> In message: <4947F363.4010909 at samsco.org>
>             Scott Long <scottl at samsco.org> writes:
> : M. Warner Losh wrote:
> : > In message: <4947D474.9040802 at samsco.org>
> : >             Scott Long <scottl at samsco.org> writes:
> : > : Alexander Motin wrote:
> : > : > Author: mav
> : > : > Date: Tue Dec 16 16:04:40 2008
> : > : > New Revision: 186182
> : > : > URL: http://svn.freebsd.org/changeset/base/186182
> : > : > 
> : > : > Log:
> : > : >   Call ata_legacy() only once on attach and save it's result. Scanning PCI
> : > : >   configuration registers (which are not going to change) on every interrupt
> : > : >   looks expensive, especially when interrupt is shared. Profiling shows me 3%
> : > : >   of time spent by atapci0 on pure network load due to IRQ sharing with em0.
> : > : > 
> : > : 
> : > : Nice change.  PCI Config registers are exceptionally slow to access on 
> : > : most systems.
> : > 
> : > And we've been recommending to people for years that they avoid config
> : > space access in interrupt handlers.  Maybe it is time for something
> : > that checks and prints a warning?
> : > 
> : 
> : With the move to memory-mapped pci config registers, there was an 
> : intention to allow low-end devices to put their registers into config
> : space.  I think I recall some legacy ultra-low end devices that also
> : put a few required registers into config space.  So while it's not ideal
> : to access it from an interrupt handler, I can't think of why it should
> : be expressly forbidden.
> 
> True.  I wasn't planning on banning it, just warning about it so we
> could be purposeful in our use of it.  Likely unworkable though...

Easy enough to stash a "once" varible in the generic device struct
and warn when returning from an isr when INVARIANTS or something is
turned on.

Then you'd only get one warning per device once it happens.

-- 
- Alfred Perlstein