From owner-freebsd-hackers@FreeBSD.ORG Wed Jun 17 22:32:44 2009 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1E3BA106564A; Wed, 17 Jun 2009 22:32:44 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id EB2D28FC12; Wed, 17 Jun 2009 22:32:43 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from fledge.watson.org (fledge.watson.org [65.122.17.41]) by cyrus.watson.org (Postfix) with ESMTPS id 7A18546B51; Wed, 17 Jun 2009 18:32:43 -0400 (EDT) Date: Wed, 17 Jun 2009 23:32:43 +0100 (BST) From: Robert Watson X-X-Sender: robert@fledge.watson.org To: John Baldwin In-Reply-To: <200906171717.37677.jhb@freebsd.org> Message-ID: References: <200906151353.06630.mel.flynn+fbsd.hackers@mailing.thruhere.net> <200906170815.26650.jhb@freebsd.org> <200906171152.55438.mel.flynn+fbsd.hackers@mailing.thruhere.net> <200906171717.37677.jhb@freebsd.org> User-Agent: Alpine 2.00 (BSF 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-hackers@freebsd.org, Mel Flynn Subject: Re: How best to debug locking/scheduler problems X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Jun 2009 22:32:44 -0000 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 >