How best to debug locking/scheduler problems

[Robert Watson]

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
>