From owner-freebsd-current@FreeBSD.ORG Thu May 14 23:03:23 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 476391065679 for ; Thu, 14 May 2009 23:03:23 +0000 (UTC) (envelope-from pisymbol@gmail.com) Received: from yw-out-2324.google.com (yw-out-2324.google.com [74.125.46.31]) by mx1.freebsd.org (Postfix) with ESMTP id EDC738FC08 for ; Thu, 14 May 2009 23:03:22 +0000 (UTC) (envelope-from pisymbol@gmail.com) Received: by yw-out-2324.google.com with SMTP id 9so904265ywe.13 for ; Thu, 14 May 2009 16:03:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=NLb563U6gRyBaxAw7HJkuAf+J7NQ0kIKgIWv3im8LKg=; b=XiJIiqsPatJy/kw6LXUCghIh3YH1FR6wLKLrEp8NxiDqwkEhh85Jqo3I3y/XinGmUo CzYxXqMIc9wdQLxmZ6Lpnqfb6voR+jH1eXW0xrsaV7NO/kzRFEQ169O/Yh7XjPYXGhH8 pLG1KRbY4oxX3My5Vcg2hAQ7w6cxqz932C5Ds= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=lVJp3nHkQyXM4ATqKIyHVmFcnc5V3EkcvQseEyS3AygDsmfAe++JTq2r+ZB05neACL sQ6QSU6B52VqikfZzYuzEMrPf4D7MoXaGJjYKblJD2SLfmLiIijMSg+9OcgRYOSwZOWt IMgtuV7AlkZ6PajHPL0HkbiydkBF7rXBE/Pmw= MIME-Version: 1.0 Received: by 10.100.41.9 with SMTP id o9mr3807055ano.155.1242342202139; Thu, 14 May 2009 16:03:22 -0700 (PDT) In-Reply-To: <4A0CA096.4020706@delphij.net> References: <3c0b01820905141202w113966dp4bfbab73d84d585@mail.gmail.com> <4A0C7544.6010304@delphij.net> <3c0b01820905141301h1b08fc0ay1e6a1676b5a149d4@mail.gmail.com> <4A0C8F30.8080404@delphij.net> <3c0b01820905141505r5b8ea64bq42cdea70ee288015@mail.gmail.com> <4A0CA096.4020706@delphij.net> Date: Thu, 14 May 2009 19:03:22 -0400 Message-ID: <3c0b01820905141603g59e439c1y7202532bc1f4f87@mail.gmail.com> From: Alexander Sack To: d@delphij.net Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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 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 23:03:23 -0000 On Thu, May 14, 2009 at 6:52 PM, Xin LI wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Hi, Alexander, > > Alexander Sack wrote: > [...] >>> @@ -3193,6 +3193,9 @@ bge_rxeof(struct bge_softc *sc) >>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0BGE_UNLOCK(sc); >>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(*ifp->if_input)(ifp, m); >>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0BGE_LOCK(sc); >>> + >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!(ifp->if_drv_flags & IFF_DRV_RUNNING= )) >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return; >>> =A0 =A0 =A0 =A0} >> >> Xin this looks fine by me, I actually put this up in the while loop as >> I mentioned before which I think is functionality equivalent (can you >> gain some optimizations by putting in the while loop though compiler >> wise than a separate compilation unit?). > > I think the two is not semantically the same... =A0For this case an > explicit 'return' would mean that no further actions, say the things > right after the while loop, would be taken. =A0In my opinion that this is > better since there is no protection over these DMA maps (which could > have been released elsewhere). Oh snap, I had a brain fart. Ugh. You are correct Xin. Oh man, I even said in my first post I want to AVOID touching the DMA maps and/or hardware registers (notice the access in the jumbo case) as soon as IFF_DRV_RUNNING gets reset. I like the explicit return much better that's why I had it that way but as John pointed out, tis better to do it where you put it which is right after reacquiring the lock. > Note that I'm still not quite confident about the logic, we might just > narrowed but not closed the race completely - these > BGE_LOCK()/UNLOCK()'s could hit some problem if one thread has done > bge_stop() very quickly. =A0That would require more work, though, I don't > have a very good plan at this moment... This has been a constant problem that I've seen in the labs. I've seen this with polling as well as tuning the number of RING descriptors (there is a thread I had a while back ago about this when I was dealing with BPF drops and bge(4)). I think the patch as it stands though will suffice for the majority of cases and again I thank you and John for addressing it in short-order. -aps