From owner-freebsd-current Fri Feb 28 2:57:46 2003 Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 0496F37B401 for ; Fri, 28 Feb 2003 02:57:45 -0800 (PST) Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by mx1.FreeBSD.org (Postfix) with ESMTP id C2FF443FBF for ; Fri, 28 Feb 2003 02:57:43 -0800 (PST) (envelope-from bde@zeta.org.au) Received: from gamplex.bde.org (katana.zip.com.au [61.8.7.246]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id VAA12267; Fri, 28 Feb 2003 21:57:36 +1100 Date: Fri, 28 Feb 2003 21:59:10 +1100 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: Maksim Yevmenkin Cc: current@FreeBSD.ORG Subject: Re: PATCH: typo in socreate() or i'm missing something In-Reply-To: <3E5E9FF0.3030106@exodus.net> Message-ID: <20030228212340.Q22122-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Thu, 27 Feb 2003, Maksim Yevmenkin wrote: > Please find the attached patch for socreate() in uipc_socket.c. > I think the code was supposed to call soalloc(0) rather then > soalloc(M_NOWAIT). Note M_NOWAIT defined as 1. > > Is that a real typo or i'm missing something here? soalloc(1) was intended (and was obtained by misspelling 1 as M_NOWAIT), but that was probably wrong. Things have regressed from RELENG_4 where the code was: so = soalloc(p != 0); socreate()'s `p' arg was abused as a flag to say that the call is made in process context. In -current, the corresponding `td' is never NULL so it is harder to tell if we are in process context or if this distinction actually matters. The code first rotted to: so = soalloc(td != 0); Then it rotted to: so = soalloc(M_NOWAIT); which is equivalent to the previously rotted state since td is never NULL and M_NOWAIT happens to be 1 (boolean true). Changing it to your version: so = soalloc(M_NOWAIT); is safer since it assumes the worst case (that socreate() is called in non-process context and/or with locks held), but it leaves soalloc()'s interface bogus (it's waitfor arg is always 0). I don't trust the locking for this even in RELENG_4. Networking code liked to do things like: s = splnet(); /* "Lock" something or other. */ lotsa_stuff(); splx(s); where lotsa_stuff() calls malloc(..., M_WAITOK) from a deeply nested routine. This may or may not be a race, depending on whether the code depends on splnet() to lock _everything_ against softnet activity until the splx(). I'm not sure if this happens for socreate(). Eventually, locking bugs in this area will be caught by locking assertions. I think they mostly aren't now, since the interrupt context check in malloc() has rotted and other checks aren't reached unless malloc() actually sleeps (which rarely happens). Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message