Date: Sat, 13 Jul 2002 21:07:02 -0700 (PDT) From: Don Lewis <dl-freebsd@catspoiler.org> To: hsu@FreeBSD.org Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/netinet udp_usrreq.c Message-ID: <200207140407.g6E472wr020147@gw.catspoiler.org> In-Reply-To: <0GZ700G2HX4WRM@mta6.snfc21.pbi.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <dl-freebsd@catspoiler.org> > 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200207140407.g6E472wr020147>