Date: Mon, 15 Mar 2010 21:31:52 -0600 From: Scott Long <scottl@samsco.org> To: Pyun YongHyeon <yongari@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r205090 - head/sys/dev/bge Message-ID: <F3F82256-854D-4E09-BC26-6D8AF51486E2@samsco.org> In-Reply-To: <201003121818.o2CII4ri076014@svn.freebsd.org> References: <201003121818.o2CII4ri076014@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 >=20 > 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. >=20 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. Scott
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?F3F82256-854D-4E09-BC26-6D8AF51486E2>