Index | Thread | Search

From:
Alexander Bluhm <alexander.bluhm@gmx.net>
Subject:
Re: Use `sb_mtx' to protect `sb_timeo_nsecs'
To:
Vitaliy Makkoveev <mvs@openbsd.org>
Cc:
tech@openbsd.org
Date:
Mon, 5 Feb 2024 21:04:43 +0100

Download raw body.

Thread
On Mon, Feb 05, 2024 at 12:31:06PM +0300, Vitaliy Makkoveev wrote:
> Start moving 'sockbuf' structure data under `sb_mtx' mutex(9). In most
> places solock() is still held because other 'sockbuf' members require
> it, but in so{g,s}etopt() paths solock() is avoided.
> 
> This diff doesn't conflict with "Use `sb_mtx' instead of `inp_mtx'..."
> diff.

OK bluhm@

> Index: sys/kern/uipc_socket.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.316
> diff -u -p -r1.316 uipc_socket.c
> --- sys/kern/uipc_socket.c	3 Feb 2024 22:50:08 -0000	1.316
> +++ sys/kern/uipc_socket.c	5 Feb 2024 09:13:10 -0000
> @@ -1224,7 +1224,9 @@ sorflush(struct socket *so)
>  	m = sb->sb_mb;
>  	memset(&sb->sb_startzero, 0,
>  	     (caddr_t)&sb->sb_endzero - (caddr_t)&sb->sb_startzero);
> +	mtx_enter(&sb->sb_mtx);
>  	sb->sb_timeo_nsecs = INFSLP;
> +	mtx_leave(&sb->sb_mtx);
>  	sbunlock(so, sb);
>  	if (pr->pr_flags & PR_RIGHTS && pr->pr_domain->dom_dispose)
>  		(*pr->pr_domain->dom_dispose)(m);
> @@ -1907,9 +1909,9 @@ sosetopt(struct socket *so, int level, i
>  			if (nsecs == 0)
>  				nsecs = INFSLP;
>  
> -			solock(so);
> +			mtx_enter(&sb->sb_mtx);
>  			sb->sb_timeo_nsecs = nsecs;
> -			sounlock(so);
> +			mtx_leave(&sb->sb_mtx);
>  			break;
>  		    }
>  
> @@ -2047,9 +2049,9 @@ sogetopt(struct socket *so, int level, i
>  			struct timeval tv;
>  			uint64_t nsecs;
>  
> -			solock_shared(so);
> +			mtx_enter(&sb->sb_mtx);
>  			nsecs = sb->sb_timeo_nsecs;
> -			sounlock_shared(so);
> +			mtx_leave(&sb->sb_mtx);
>  
>  			m->m_len = sizeof(struct timeval);
>  			memset(&tv, 0, sizeof(tv));
> Index: sys/kern/uipc_socket2.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
> retrieving revision 1.141
> diff -u -p -r1.141 uipc_socket2.c
> --- sys/kern/uipc_socket2.c	3 Feb 2024 22:50:08 -0000	1.141
> +++ sys/kern/uipc_socket2.c	5 Feb 2024 09:13:10 -0000
> @@ -216,10 +216,14 @@ sonewconn(struct socket *head, int conns
>  		goto fail;
>  	so->so_snd.sb_wat = head->so_snd.sb_wat;
>  	so->so_snd.sb_lowat = head->so_snd.sb_lowat;
> +	mtx_enter(&head->so_snd.sb_mtx);
>  	so->so_snd.sb_timeo_nsecs = head->so_snd.sb_timeo_nsecs;
> +	mtx_leave(&head->so_snd.sb_mtx);
>  	so->so_rcv.sb_wat = head->so_rcv.sb_wat;
>  	so->so_rcv.sb_lowat = head->so_rcv.sb_lowat;
> +	mtx_enter(&head->so_rcv.sb_mtx);
>  	so->so_rcv.sb_timeo_nsecs = head->so_rcv.sb_timeo_nsecs;
> +	mtx_leave(&head->so_rcv.sb_mtx);
>  
>  	sigio_copy(&so->so_sigio, &head->so_sigio);
>  
> @@ -506,15 +510,17 @@ sosleep_nsec(struct socket *so, void *id
>  int
>  sbwait(struct socket *so, struct sockbuf *sb)
>  {
> +	uint64_t timeo_nsecs;
>  	int prio = (sb->sb_flags & SB_NOINTR) ? PSOCK : PSOCK | PCATCH;
>  
>  	soassertlocked(so);
>  
>  	mtx_enter(&sb->sb_mtx);
> +	timeo_nsecs = sb->sb_timeo_nsecs;
>  	sb->sb_flags |= SB_WAIT;
>  	mtx_leave(&sb->sb_mtx);
>  
> -	return sosleep_nsec(so, &sb->sb_cc, prio, "netio", sb->sb_timeo_nsecs);
> +	return sosleep_nsec(so, &sb->sb_cc, prio, "netio", timeo_nsecs);
>  }
>  
>  int
> Index: sys/nfs/nfs_socket.c
> ===================================================================
> RCS file: /cvs/src/sys/nfs/nfs_socket.c,v
> retrieving revision 1.144
> diff -u -p -r1.144 nfs_socket.c
> --- sys/nfs/nfs_socket.c	3 Aug 2023 09:49:09 -0000	1.144
> +++ sys/nfs/nfs_socket.c	5 Feb 2024 09:13:10 -0000
> @@ -295,7 +295,6 @@ nfs_connect(struct nfsmount *nmp, struct
>  			goto bad;
>  	}
>  
> -	solock(so);
>  	/*
>  	 * Protocols that do not require connections may be optionally left
>  	 * unconnected for servers that reply from a port other than NFS_PORT.
> @@ -303,9 +302,10 @@ nfs_connect(struct nfsmount *nmp, struct
>  	if (nmp->nm_flag & NFSMNT_NOCONN) {
>  		if (nmp->nm_soflags & PR_CONNREQUIRED) {
>  			error = ENOTCONN;
> -			goto bad_locked;
> +			goto bad;
>  		}
>  	} else {
> +		solock(so);
>  		error = soconnect(so, nmp->nm_nam);
>  		if (error)
>  			goto bad_locked;
> @@ -330,17 +330,21 @@ nfs_connect(struct nfsmount *nmp, struct
>  			so->so_error = 0;
>  			goto bad_locked;
>  		}
> +		sounlock(so);
>  	}
>  	/*
>  	 * Always set receive timeout to detect server crash and reconnect.
>  	 * Otherwise, we can get stuck in soreceive forever.
>  	 */
> +	mtx_enter(&so->so_rcv.sb_mtx);
>  	so->so_rcv.sb_timeo_nsecs = SEC_TO_NSEC(5);
> +	mtx_leave(&so->so_rcv.sb_mtx);
> +	mtx_enter(&so->so_snd.sb_mtx);
>  	if (nmp->nm_flag & (NFSMNT_SOFT | NFSMNT_INT))
>  		so->so_snd.sb_timeo_nsecs = SEC_TO_NSEC(5);
>  	else
>  		so->so_snd.sb_timeo_nsecs = INFSLP;
> -	sounlock(so);
> +	mtx_leave(&so->so_snd.sb_mtx);
>  	if (nmp->nm_sotype == SOCK_DGRAM) {
>  		sndreserve = nmp->nm_wsize + NFS_MAXPKTHDR;
>  		rcvreserve = (max(nmp->nm_rsize, nmp->nm_readdirsize) +
> Index: sys/nfs/nfs_syscalls.c
> ===================================================================
> RCS file: /cvs/src/sys/nfs/nfs_syscalls.c,v
> retrieving revision 1.120
> diff -u -p -r1.120 nfs_syscalls.c
> --- sys/nfs/nfs_syscalls.c	12 Jan 2024 08:47:46 -0000	1.120
> +++ sys/nfs/nfs_syscalls.c	5 Feb 2024 09:13:10 -0000
> @@ -277,9 +277,13 @@ nfssvc_addsock(struct file *fp, struct m
>  	}
>  	solock(so);
>  	so->so_rcv.sb_flags &= ~SB_NOINTR;
> +	mtx_enter(&so->so_rcv.sb_mtx);
>  	so->so_rcv.sb_timeo_nsecs = INFSLP;
> +	mtx_leave(&so->so_rcv.sb_mtx);
>  	so->so_snd.sb_flags &= ~SB_NOINTR;
> +	mtx_enter(&so->so_snd.sb_mtx);
>  	so->so_snd.sb_timeo_nsecs = INFSLP;
> +	mtx_leave(&so->so_snd.sb_mtx);
>  	sounlock(so);
>  	if (tslp)
>  		slp = tslp;