Date: Fri, 28 Feb 2003 21:59:10 +1100 (EST) From: Bruce Evans <bde@zeta.org.au> To: Maksim Yevmenkin <myevmenk@exodus.net> Cc: current@FreeBSD.ORG Subject: Re: PATCH: typo in socreate() or i'm missing something Message-ID: <20030228212340.Q22122-100000@gamplex.bde.org> In-Reply-To: <3E5E9FF0.3030106@exodus.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030228212340.Q22122-100000>