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

[Garrett Cooper]

On Dec 26, 2008, at 18:33, Scott Long <scottl at samsco.org> wrote:

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

Unfortunately embedded systems can't win the lottery ;.).
-Garrett