From owner-svn-src-head@FreeBSD.ORG Sat Aug 18 09:32:45 2012 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 56FC9106566C; Sat, 18 Aug 2012 09:32:45 +0000 (UTC) (envelope-from rrs@lakerest.net) Received: from lakerest.net (lakerest.net [70.155.160.98]) by mx1.freebsd.org (Postfix) with ESMTP id D1C8E8FC0A; Sat, 18 Aug 2012 09:32:44 +0000 (UTC) Received: from [10.1.1.141] (bsd3.lakerest.net [70.155.160.101]) (authenticated bits=0) by lakerest.net (8.14.4/8.14.3) with ESMTP id q7I9VNKh010398 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NOT); Sat, 18 Aug 2012 05:31:23 -0400 (EDT) (envelope-from rrs@lakerest.net) Mime-Version: 1.0 (Apple Message framework v1278) Content-Type: text/plain; charset=windows-1252 From: Randall Stewart In-Reply-To: <201208170826.25123.jhb@freebsd.org> Date: Sat, 18 Aug 2012 05:31:24 -0400 Content-Transfer-Encoding: quoted-printable Message-Id: References: <201208170551.q7H5pkd1025308@svn.freebsd.org> <201208170826.25123.jhb@freebsd.org> To: John Baldwin X-Mailer: Apple Mail (2.1278) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r239353 - head/sys/netinet 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: Sat, 18 Aug 2012 09:32:45 -0000 On Aug 17, 2012, at 8:26 AM, John Baldwin wrote: > On Friday, August 17, 2012 1:51:46 am Randall Stewart wrote: >> Author: rrs >> Date: Fri Aug 17 05:51:46 2012 >> New Revision: 239353 >> URL: http://svn.freebsd.org/changeset/base/239353 >>=20 >> Log: >> Ok jhb, lets move the ifa_free() down to the bottom to >> assure that *all* tables and such are removed before >> we start to free. This won't protect the Hash in ip_input.c >> but in theory should protect any other uses that *do* use locks. >=20 > Eh, this is just a nop unless there is a reference counting bug. Only = the=20 > last reference would free the memory if the reference count is = correct, so=20 > this is just adding obfuscation by moving the ifa_free() away from the=20= > associated TAILQ_REMOVE(). >=20 Well there is definitely a reference counting but in here somewhere = then. And as far as "obfuscation" of the code, it was not obvious to me when first = looking at the code and it should have been since I looked at this earlier. So = without a more detailed comment I think its obscure anyway ;-) I will get SQA another INVARIANT load and see if they can recreate it = for me. The load they had when they first produced this was *supposed* to be = INVARIANT but after a bit of research on my part I find they were running the non-invariant, = which explains why things locked up instead of the assert triggering.. sigh. Note that in the main FreeBSD sources however, the fact that ip_input.c = does *not* lock when it looks at the hash table these lines are removing from here means = there is still a potential for a crash. I have a fix for this that does not require the lock, when = I get a moment I will send it your way=85 you have seen part of it ;-) R > --=20 > John Baldwin >=20 ------------------------------ Randall Stewart 803-317-4952 (cell)