Date: Sat, 13 Jul 2002 19:48:45 -0700 From: Jeffrey Hsu <hsu@FreeBSD.org> To: Don Lewis <dl-freebsd@catspoiler.org> Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/netinet udp_usrreq.c Message-ID: <0GZ700G2HX4WRM@mta6.snfc21.pbi.net> In-Reply-To: Message from Don Lewis <dl-freebsd@catspoiler.org> "of Sat, 13 Jul 2002 19:21:55 PDT." <200207140221.g6E2Ltwr020001@gw.catspoiler.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> 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(). ------------------------------------------------------------------------------- ------ 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?0GZ700G2HX4WRM>