From owner-freebsd-current@FreeBSD.ORG Thu May 14 19:48:36 2009 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 04B7B10656CA for ; Thu, 14 May 2009 19:48:35 +0000 (UTC) (envelope-from delphij@delphij.net) Received: from tarsier.delphij.net (delphij-pt.tunnel.tserv2.fmt.ipv6.he.net [IPv6:2001:470:1f03:2c9::2]) by mx1.freebsd.org (Postfix) with ESMTP id 7FCA48FC1E for ; Thu, 14 May 2009 19:48:35 +0000 (UTC) (envelope-from delphij@delphij.net) Received: from tarsier.geekcn.org (tarsier.geekcn.org [211.166.10.233]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by tarsier.delphij.net (Postfix) with ESMTPS id 8EDA45C026 for ; Fri, 15 May 2009 03:48:34 +0800 (CST) Received: from localhost (tarsier.geekcn.org [211.166.10.233]) by tarsier.geekcn.org (Postfix) with ESMTP id 3FC8555D1755; Fri, 15 May 2009 03:48:34 +0800 (CST) X-Virus-Scanned: amavisd-new at geekcn.org Received: from tarsier.geekcn.org ([211.166.10.233]) by localhost (mail.geekcn.org [211.166.10.233]) (amavisd-new, port 10024) with ESMTP id edCJ2f9-Z6YN; Fri, 15 May 2009 03:47:40 +0800 (CST) Received: from charlie.delphij.net (adsl-76-237-33-62.dsl.pltn13.sbcglobal.net [76.237.33.62]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by tarsier.geekcn.org (Postfix) with ESMTPSA id 1024455D1754; Fri, 15 May 2009 03:47:33 +0800 (CST) DomainKey-Signature: a=rsa-sha1; s=default; d=delphij.net; c=nofws; q=dns; h=message-id:date:from:reply-to:organization:user-agent: mime-version:to:cc:subject:references:in-reply-to: x-enigmail-version:openpgp:content-type:content-transfer-encoding; b=b2zzNglzPKSm7DnlWHFqVnJyql1NwrQzCmIVgslj2diPcGQb8je4pT2aFW21LQ5Bh vzdYnW1JL+2W9wZF+BHoQ== Message-ID: <4A0C7544.6010304@delphij.net> Date: Thu, 14 May 2009 12:47:16 -0700 From: Xin LI Organization: The FreeBSD Project User-Agent: Thunderbird 2.0.0.21 (X11/20090408) MIME-Version: 1.0 To: Alexander Sack References: <3c0b01820905141202w113966dp4bfbab73d84d585@mail.gmail.com> In-Reply-To: <3c0b01820905141202w113966dp4bfbab73d84d585@mail.gmail.com> X-Enigmail-Version: 0.95.7 OpenPGP: id=18EDEBA0; url=http://www.delphij.net/delphij.asc Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: freebsd-current@freebsd.org Subject: Re: Broadcom bge(4) panics while shutting down X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: d@delphij.net List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 May 2009 19:48:48 -0000 -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi, Alexander, Alexander Sack wrote: > Hello: > > Under heavy traffic (100% utilization GIGE on a 2 port BGE card) > running BGE CURRENT driver I see panics on shutdown. The reason is > because bge_rxeof() while processing its RX ring of BD's drops the > softc lock when it hands it off to its input function. If bge_stop() > is waiting for it, it will then proceed to acquire lock and then > quiesce the hardware (reseting the card, clearing out BDs etc.). Once > bge_stop() releases the softc lock, then bge_rxeof() under an > interrupt context (no polling here) will reacquire and continue to > process the ring which is a bad idea. It should check to see if the > card is still running before continuing processing BDs (i.e. once > IF_DRV_RUNNING has been reset by bge_stop(), bge_rxeof() is done, bail > out). > > Here is my first go around with this patch: > > > -- if_bge.c.CURRENT 2009-05-14 14:39:39.000000000 -0400 > +++ if_bge.c 2009-05-14 14:39:24.000000000 -0400 > @@ -3081,6 +3081,10 @@ > uint16_t vlan_tag = 0; > int have_tag = 0; > > + if (!(ifp->if_drv_flags & IFF_DRV_RUNNING)) { > + return; > + } > + > #ifdef DEVICE_POLLING > if (ifp->if_capenable & IFCAP_POLLING) { > if (sc->rxcycles <= 0) > > > This prevents any panics during shutdown under heavy load and AS IT > TURNS out (I feel stupid for not looking) that em(4) already had this > check in its em_rxeof() function (right at the top of the loop). I'm > more than happy changing it to the em style but above seems reasonable > to me though I have to verify there isn't anything missing off the > loop from a hardware standpoint (I don't think so because bge_stop() > did all the dirty work so I believe touching any registers after that > from bge_rxeof() is a bad idea). > > Preliminary testing shows no more panics start and stopping ports > under heavy load (panics were almost immediate otherwise). > > Thoughts? I think this would solve the problem but I'm not sure whether this would increase some overhead on the RX path. It seems that there is a race between bge_release_resources() and bge_intr(), I mean, it might be a good idea to "drain" bge_intr() instead? Cheers, - -- Xin LI http://www.delphij.net/ FreeBSD - The Power to Serve! -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.11 (FreeBSD) iEYEARECAAYFAkoMdUMACgkQi+vbBBjt66AU5wCgndWdms9jftsOE/C2EwRGGdRS 5nAAn2Fhw3dA0vojn205ckC+ZFU5+imY =oeON -----END PGP SIGNATURE-----