svn commit: r205090 - head/sys/dev/bge

[Pyun YongHyeon]

On Mon, Mar 15, 2010 at 09:31:52PM -0600, Scott Long wrote:
> On Mar 12, 2010, at 11:18 AM, Pyun YongHyeon wrote:
> > Author: yongari
> > Date: Fri Mar 12 18:18:04 2010
> > New Revision: 205090
> > URL: http://svn.freebsd.org/changeset/base/205090
> > 
> > Log:
> > Reorder interrupt handler a bit such that producer/consumer
> > index of status block is read first before acknowledging the
> > interrupts. Otherwise bge(4) may get stale status block as
> > acknowledging an interrupt may yield another status block update.
> > 
> 
> I'm starting a new sub-thread because it quickly became impossible to keep context straight in the conversation between you and Bruce.
> 
> The previous rev did this:
> 
> 1. Write an ACK word to the hardware
> 2. perform the memory coherency protocol
> 3  Cache the status descriptors
> 4. Execute the interrupt handlers for the descriptors
> 
> I think that your concern was that after performing step 1, the BGE hardware would be free to assert a new interrupt and/or update memory in a way that would interfere with steps 2-4, yes?  I don't believe that this is a valid concern.  By performing the ACK first, the driver is guaranteeing that any new updates done by the BGE hardware will generate a follow-on interrupt that will be seen and trigger a new run through the interrupt handle.  No matter where an unexpected update happens from the hardware, a new interrupt will be generated and will be guaranteed to be serviced, ensuring that the update is seen.  Also, the status descriptors are designed to be immune to interference of this nature; they can go stale, but that can't be corrupted.  Again, going stale is not bad.
> 
> The previous version affirms that a race exists, but guarantees that it won't be forgotten.  There's nothing wrong with this, in my opinion.  Whether you're using MSI or INTx (obviously assuming that there are no hardware bugs here), the race will be caught.
> 
> I don't like your change because it leaves the ACK step incoherent.  By deferring that write to be after the read, there's no guaranteed of when that write will actually get flushed to the hardware.  It will eventually, but maybe not as soon as we'd like.
> 

This is valid concern and I seem to missed this. I still think
tagged status would be better way to handle interrupts but it still
does not solve the issue you mentioned. I also can see a couple of
complex code path in Linux which indicates needing of forced flush
for mail box register. Old code was safe in this regard.
I'll back out the change.