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

[Scott Long]

On Dec 26, 2008, at 5:38 PM, Alfred Perlstein <alfred at freebsd.org>  
wrote:

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

I prefer to do nothing.  It's not unsafe or erroneous to access cfg  
registers.  But if some kind of message does get added, I insist that  
it drop all pretenses and say, "you're too poor to run freebsd, come  
back when you can afford better hardware.". :)

Scott