Date: Wed, 17 Jun 2009 23:32:43 +0100 (BST) From: Robert Watson <rwatson@FreeBSD.org> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-hackers@freebsd.org, Mel Flynn <mel.flynn+fbsd.hackers@mailing.thruhere.net> Subject: Re: How best to debug locking/scheduler problems Message-ID: <alpine.BSF.2.00.0906172332010.16422@fledge.watson.org> In-Reply-To: <200906171717.37677.jhb@freebsd.org> References: <200906151353.06630.mel.flynn%2Bfbsd.hackers@mailing.thruhere.net> <200906170815.26650.jhb@freebsd.org> <200906171152.55438.mel.flynn%2Bfbsd.hackers@mailing.thruhere.net> <200906171717.37677.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 17 Jun 2009, John Baldwin wrote:
> These are the key frames. It looks like uipc_peeraddr() tries to lock two
> unp locks w/o any protection from the global unp linkage lock. I've changed
> it to use the same locking as uipc_accept() where it first grabs a read lock
> on the linkage lock and then just locks the other end of the connection to
> copy out its sockaddr.
This change looks reasonable to me, thanks for tracking this down!
Robert N M Watson
Computer Laboratory
University of Cambridge
>
> --- //depot/user/jhb/socket/kern/uipc_usrreq.c
> +++ /home/jhb/work/p4/socket/kern/uipc_usrreq.c
> @@ -671,7 +671,7 @@
> KASSERT(unp != NULL, ("uipc_peeraddr: unp == NULL"));
>
> *nam = malloc(sizeof(struct sockaddr_un), M_SONAME, M_WAITOK);
> - UNP_PCB_LOCK(unp);
> + UNP_LINK_RLOCK();
> /*
> * XXX: It seems that this test always fails even when connection is
> * established. So, this else clause is added as workaround to
> @@ -681,7 +681,7 @@
> if (unp2 != NULL) {
> UNP_PCB_LOCK(unp2);
> if (unp2->unp_addr != NULL)
> - sa = (struct sockaddr *) unp->unp_conn->unp_addr;
> + sa = (struct sockaddr *) unp2->unp_addr;
> else
> sa = &sun_noname;
> bcopy(sa, *nam, sa->sa_len);
> @@ -690,7 +690,7 @@
> sa = &sun_noname;
> bcopy(sa, *nam, sa->sa_len);
> }
> - UNP_PCB_UNLOCK(unp);
> + UNP_LINK_RUNLOCK();
> return (0);
> }
>
> @@ -850,7 +850,7 @@
> * return the slightly counter-intuitive but otherwise
> * correct error that the socket is not connected.
> *
> - * Locking here must be done carefully: the inkage lock
> + * Locking here must be done carefully: the linkage lock
> * prevents interconnections between unpcbs from changing, so
> * we can traverse from unp to unp2 without acquiring unp's
> * lock. Socket buffer locks follow unpcb locks, so we can
>
> --
> John Baldwin
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.BSF.2.00.0906172332010.16422>