From owner-svn-src-head@FreeBSD.ORG Tue Mar 16 03:32:00 2010 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9B5B3106566B; Tue, 16 Mar 2010 03:32:00 +0000 (UTC) (envelope-from scottl@samsco.org) Received: from pooker.samsco.org (pooker.samsco.org [168.103.85.57]) by mx1.freebsd.org (Postfix) with ESMTP id 382728FC12; Tue, 16 Mar 2010 03:31:59 +0000 (UTC) Received: from [IPv6:::1] (pooker.samsco.org [168.103.85.57]) by pooker.samsco.org (8.14.4/8.14.2) with ESMTP id o2G3Vqer012972; Mon, 15 Mar 2010 21:31:52 -0600 (MDT) (envelope-from scottl@samsco.org) Mime-Version: 1.0 (Apple Message framework v1077) Content-Type: text/plain; charset=us-ascii From: Scott Long In-Reply-To: <201003121818.o2CII4ri076014@svn.freebsd.org> Date: Mon, 15 Mar 2010 21:31:52 -0600 Content-Transfer-Encoding: quoted-printable Message-Id: References: <201003121818.o2CII4ri076014@svn.freebsd.org> To: Pyun YongHyeon X-Mailer: Apple Mail (2.1077) X-Spam-Status: No, score=-4.1 required=3.8 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.1.8 X-Spam-Checker-Version: SpamAssassin 3.1.8 (2007-02-13) on pooker.samsco.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 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Mar 2010 03:32:00 -0000 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