From owner-svn-src-all@FreeBSD.ORG Tue Mar 16 17:18:37 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5CFCB106564A; Tue, 16 Mar 2010 17:18:37 +0000 (UTC) (envelope-from pyunyh@gmail.com) Received: from fg-out-1718.google.com (fg-out-1718.google.com [72.14.220.158]) by mx1.freebsd.org (Postfix) with ESMTP id ED68D8FC1D; Tue, 16 Mar 2010 17:18:35 +0000 (UTC) Received: by fg-out-1718.google.com with SMTP id 19so17807fgg.13 for ; Tue, 16 Mar 2010 10:18:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:received:from:date:to:cc :subject:message-id:reply-to:references:mime-version:content-type :content-disposition:in-reply-to:user-agent; bh=BMKKRsc7qgvqGvaK9zWndw0RP/nlZWUDJbtV8Y4+S5Q=; b=mXTvCM3xPX+pJsYoaEvcDKVdCH1tVR2JGlRYeS1PbekN6w/JODyLb5N0mPfoz+liYh kV9gJRGkHX2hMDUIL61v0xZ6s+RzgzWyODdA5PnI+UPCC8TiHHl/uk/M+OHsZ0tFeDAE 8tckFOlf+ISf1yn9hgmS9HAwxv3gxC5GrGXwE= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=from:date:to:cc:subject:message-id:reply-to:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; b=vixYTKSrpyhlpbhFo8y0F/9S7cRUW2euWVaXmziKxA0hibqk9yVg9s/oSiRf97dOX6 j2op8+mHadkCNhATwX4ttGT4FCorCoFaLQQyXMiNzJTu1tkHu6Y4TGLXR1MoFksws7qV jfU424iiruyJi2y7vFhvxlxl1YEdsnI/v7z5g= Received: by 10.87.68.15 with SMTP id v15mr11157399fgk.64.1268759914806; Tue, 16 Mar 2010 10:18:34 -0700 (PDT) Received: from pyunyh@gmail.com ([174.35.1.224]) by mx.google.com with ESMTPS id l19sm984499fgb.23.2010.03.16.10.18.30 (version=TLSv1/SSLv3 cipher=RC4-MD5); Tue, 16 Mar 2010 10:18:32 -0700 (PDT) Received: by pyunyh@gmail.com (sSMTP sendmail emulation); Tue, 16 Mar 2010 10:18:00 -0700 From: Pyun YongHyeon Date: Tue, 16 Mar 2010 10:18:00 -0700 To: Scott Long Message-ID: <20100316171800.GC2001@michelle.cdnetworks.com> References: <201003121818.o2CII4ri076014@svn.freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Pyun YongHyeon Subject: Re: svn commit: r205090 - head/sys/dev/bge X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: pyunyh@gmail.com List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Mar 2010 17:18:37 -0000 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.