From owner-cvs-all Sat Jul 13 21: 7:19 2002 Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.FreeBSD.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id C116837B400; Sat, 13 Jul 2002 21:07:11 -0700 (PDT) Received: from gw.catspoiler.org (217-ip-163.nccn.net [209.79.217.163]) by mx1.FreeBSD.org (Postfix) with ESMTP id 293CA43E58; Sat, 13 Jul 2002 21:07:11 -0700 (PDT) (envelope-from dl-freebsd@catspoiler.org) Received: from mousie.catspoiler.org (mousie.catspoiler.org [192.168.101.2]) by gw.catspoiler.org (8.12.5/8.12.5) with ESMTP id g6E472wr020147; Sat, 13 Jul 2002 21:07:06 -0700 (PDT) (envelope-from dl-freebsd@catspoiler.org) Message-Id: <200207140407.g6E472wr020147@gw.catspoiler.org> Date: Sat, 13 Jul 2002 21:07:02 -0700 (PDT) From: Don Lewis Subject: Re: cvs commit: src/sys/netinet udp_usrreq.c To: hsu@FreeBSD.org Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org In-Reply-To: <0GZ700G2HX4WRM@mta6.snfc21.pbi.net> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 13 Jul, Jeffrey Hsu wrote: > > I dug through the source and didn't see anything that made me believe > > that we need to hold this lock in the getcred code. > > If you look at the quoted email, you'll see that we were talking about > tcp_close(), not tcp_getcred(). Sorry, I guess that message was unintentionally ambiguous. My comments in the second paragraph were meant to apply to the need for locking inp in the *_getcred() code. > ------------------------------------------------------------------------------- > ------ > Subject: Re: cvs commit: src/sys/netinet udp_usrreq.c > From: Don Lewis > Date: Fri, 12 Jul 2002 04:07:12 -0700 (PDT) > To: hsu@FreeBSD.org > Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org, > jlemon@FreeBSD.org > > On 12 Jul, Jeffrey Hsu wrote: >> > Now that I look at it, I think TCP has the opposite problem. I don't >> > see either inp or tcbinfo being locked in tcp_close() before it >> > calls in_pcbdetach(). >> >> They're locked higher up in the call graph by the functions that call >> tcp_close(). > > Yup, but judging by the number of callers to tcp_close(), it might not > be a bad idea to add an assertion to verify that the proper locks are > held. > > I've been doing some digging through the code and it looks to me like > both the INP_LOCK() and inp->inp_socket == NULL test can be eliminated. > I think the INP_INFO_RUNLOCK provides sufficient protection. If we were > counting on just the INP_LOCK, we could end up using a stale pcb on the > free list. With one exception, it looks like the rest of the code just > blindly dereferences inp->inp_socket if inp is valid. It looks like the > socket and pcb are pretty tightly bound together over their lifetime. > The only exception is in tcp_ctlinput(), which appears to have inherited > an equivalent check from in_pcbnotify() when the code > was rototilled back at version 1.94 (but udp_ctlinput() didn't pick up > this check). The inp->inp_socket == NULL check shows up in the code > listing in _TCP/IP Illustrated Volume 2_, but there doesn't seem to be > any need for it there. It probably slipped in to "fix" panic some time > in the deep dark past when there were probably locking problems in the > code. > To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message