From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: merge sosplice_solock_pair() into solock_pair()
To: Alexander Bluhm <bluhm@openbsd.org>
Cc: tech@openbsd.org
Date: Fri, 31 Jan 2025 15:55:44 +0300

On Fri, Jan 31, 2025 at 01:38:31PM +0100, Alexander Bluhm wrote:
> Hi,
> 
> sosplice_solock_pair() is basically shared netlock plus solock_pair().
> By making solock_pair() generic for inet and unix sockets, the
> socket code needs less special locking paths.
> 
> ok?
> 

ok mvs

> bluhm
> 
> Index: kern/uipc_socket.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_socket.c,v
> diff -u -p -r1.367 uipc_socket.c
> --- kern/uipc_socket.c	30 Jan 2025 14:40:50 -0000	1.367
> +++ kern/uipc_socket.c	31 Jan 2025 12:12:17 -0000
> @@ -537,18 +537,12 @@ soconnect(struct socket *so, struct mbuf
>  int
>  soconnect2(struct socket *so1, struct socket *so2)
>  {
> -	int persocket, error;
> -
> -	if ((persocket = solock_persocket(so1)))
> -		solock_pair(so1, so2);
> -	else
> -		solock(so1);
> +	int error;
>  
> +	solock_pair(so1, so2);
>  	error = pru_connect2(so1, so2);
> +	sounlock_pair(so1, so2);
>  
> -	if (persocket)
> -		sounlock(so2);
> -	sounlock(so1);
>  	return (error);
>  }
>  
> @@ -1300,38 +1294,6 @@ sorflush(struct socket *so)
>  #define so_idleto	so_sp->ssp_idleto
>  #define so_splicetask	so_sp->ssp_task
>  
> -void
> -sosplice_solock_pair(struct socket *so1, struct socket *so2)
> -{
> -	NET_LOCK_SHARED();
> -
> -	if (so1 == so2)
> -		rw_enter_write(&so1->so_lock);
> -	else if (so1 < so2) {
> -		rw_enter_write(&so1->so_lock);
> -		rw_enter_write(&so2->so_lock);
> -	} else {
> -		rw_enter_write(&so2->so_lock);
> -		rw_enter_write(&so1->so_lock);
> -	}
> -}
> -
> -void
> -sosplice_sounlock_pair(struct socket *so1, struct socket *so2)
> -{
> -	if (so1 == so2)
> -		rw_exit_write(&so1->so_lock);
> -	else if (so1 < so2) {
> -		rw_exit_write(&so2->so_lock);
> -		rw_exit_write(&so1->so_lock);
> -	} else {
> -		rw_exit_write(&so1->so_lock);
> -		rw_exit_write(&so2->so_lock);
> -	}
> -
> -	NET_UNLOCK_SHARED();
> -}
> -
>  int
>  sosplice(struct socket *so, int fd, off_t max, struct timeval *tv)
>  {
> @@ -1397,7 +1359,7 @@ sosplice(struct socket *so, int fd, off_
>  		sbunlock(&so->so_rcv);
>  		goto frele;
>  	}
> -	sosplice_solock_pair(so, sosp);
> +	solock_pair(so, sosp);
>  
>  	if ((so->so_options & SO_ACCEPTCONN) ||
>  	    (sosp->so_options & SO_ACCEPTCONN)) {
> @@ -1458,7 +1420,7 @@ sosplice(struct socket *so, int fd, off_
>  	mtx_leave(&sosp->so_snd.sb_mtx);
>  	mtx_leave(&so->so_rcv.sb_mtx);
>  
> -	sosplice_sounlock_pair(so, sosp);
> +	sounlock_pair(so, sosp);
>  	sbunlock(&sosp->so_snd);
>  
>  	if (somove(so, M_WAIT)) {
> @@ -1475,7 +1437,7 @@ sosplice(struct socket *so, int fd, off_
>  	return (0);
>  
>   release:
> -	sosplice_sounlock_pair(so, sosp);
> +	sounlock_pair(so, sosp);
>  	sbunlock(&sosp->so_snd);
>  	sbunlock(&so->so_rcv);
>   frele:
> Index: kern/uipc_socket2.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_socket2.c,v
> diff -u -p -r1.173 uipc_socket2.c
> --- kern/uipc_socket2.c	31 Jan 2025 07:53:10 -0000	1.173
> +++ kern/uipc_socket2.c	31 Jan 2025 12:12:17 -0000
> @@ -373,16 +373,22 @@ solock_persocket(struct socket *so)
>  void
>  solock_pair(struct socket *so1, struct socket *so2)
>  {
> -	KASSERT(so1 != so2);
>  	KASSERT(so1->so_type == so2->so_type);
> -	KASSERT(solock_persocket(so1));
>  
> -	if (so1 < so2) {
> -		solock(so1);
> -		solock(so2);
> +	switch (so1->so_proto->pr_domain->dom_family) {
> +	case PF_INET:
> +	case PF_INET6:
> +		NET_LOCK_SHARED();
> +		break;
> +	}
> +	if (so1 == so2) {
> +		rw_enter_write(&so1->so_lock);
> +	} else if (so1 < so2) {
> +		rw_enter_write(&so1->so_lock);
> +		rw_enter_write(&so2->so_lock);
>  	} else {
> -		solock(so2);
> -		solock(so1);
> +		rw_enter_write(&so2->so_lock);
> +		rw_enter_write(&so1->so_lock);
>  	}
>  }
>  
> @@ -416,6 +422,26 @@ void
>  sounlock_nonet(struct socket *so)
>  {
>  	rw_exit_write(&so->so_lock);
> +}
> +
> +void
> +sounlock_pair(struct socket *so1, struct socket *so2)
> +{
> +	if (so1 == so2)
> +		rw_exit_write(&so1->so_lock);
> +	else if (so1 < so2) {
> +		rw_exit_write(&so2->so_lock);
> +		rw_exit_write(&so1->so_lock);
> +	} else {
> +		rw_exit_write(&so1->so_lock);
> +		rw_exit_write(&so2->so_lock);
> +	}
> +	switch (so1->so_proto->pr_domain->dom_family) {
> +	case PF_INET:
> +	case PF_INET6:
> +		NET_UNLOCK_SHARED();
> +		break;
> +	}
>  }
>  
>  void
> Index: kern/uipc_usrreq.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_usrreq.c,v
> diff -u -p -r1.214 uipc_usrreq.c
> --- kern/uipc_usrreq.c	25 Jan 2025 22:06:41 -0000	1.214
> +++ kern/uipc_usrreq.c	31 Jan 2025 12:12:17 -0000
> @@ -924,22 +924,15 @@ unp_connect(struct socket *so, struct mb
>  		}
>  
>  		so2 = so3;
> -	} else {
> -		if (so2 != so)
> -			solock_pair(so, so2);
> -		else
> -			solock(so);
> -	}
> +	} else
> +		solock_pair(so, so2);
>  
>  	error = unp_connect2(so, so2);
>  
> -	sounlock(so);
> -
>  	/*
>  	 * `so2' can't be PRU_ABORT'ed concurrently
>  	 */
> -	if (so2 != so)
> -		sounlock(so2);
> +	sounlock_pair(so, so2);
>  put:
>  	vput(vp);
>  unlock:
> Index: sys/socketvar.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/sys/socketvar.h,v
> diff -u -p -r1.145 socketvar.h
> --- sys/socketvar.h	31 Jan 2025 07:53:10 -0000	1.145
> +++ sys/socketvar.h	31 Jan 2025 12:12:17 -0000
> @@ -436,6 +436,7 @@ void	solock_pair(struct socket *, struct
>  void	sounlock(struct socket *);
>  void	sounlock_shared(struct socket *);
>  void	sounlock_nonet(struct socket *);
> +void	sounlock_pair(struct socket *, struct socket *);
>  
>  int	sendit(struct proc *, int, struct msghdr *, int, register_t *);
>  int	recvit(struct proc *, int, struct msghdr *, caddr_t, register_t *);
>