On Wed, Nov 22, 2017 at 01:01:25PM +0100, Martin Pieuchot wrote:
> When we converted most of the splsoftnet() to NET_LOCK() we ended up
> with some "lock" inversions between the socket buffer "lock" and the
> NET_LOCK(). At that time the KERNEL_LOCK() was serializing access to
> `sb_flags'.
>
> We're now moving towards running sockets functions w/o KERNEL_LOCK()
> in protocol input paths. So I'd like to assert that the socket lock
> is held when sb_flags are modified.
>
> There's currently one exception to this rule, FIFO kqueue filters,
> that I'll address in a later diff.
>
> ok?
OK bluhm@
> Index: kern/uipc_socket.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.207
> diff -u -p -r1.207 uipc_socket.c
> --- kern/uipc_socket.c 4 Nov 2017 14:13:53 -0000 1.207
> +++ kern/uipc_socket.c 22 Nov 2017 11:22:02 -0000
> @@ -453,7 +453,7 @@ restart:
> (atomic || space < so->so_snd.sb_lowat))) {
> if ((so->so_state & SS_NBIO) || (flags & MSG_DONTWAIT))
> snderr(EWOULDBLOCK);
> - sbunlock(&so->so_snd);
> + sbunlock(so, &so->so_snd);
> error = sbwait(so, &so->so_snd);
> so->so_state &= ~SS_ISSENDING;
> if (error)
> @@ -497,7 +497,7 @@ restart:
>
> release:
> so->so_state &= ~SS_ISSENDING;
> - sbunlock(&so->so_snd);
> + sbunlock(so, &so->so_snd);
> out:
> sounlock(s);
> m_freem(top);
> @@ -736,7 +736,7 @@ restart:
> }
> SBLASTRECORDCHK(&so->so_rcv, "soreceive sbwait 1");
> SBLASTMBUFCHK(&so->so_rcv, "soreceive sbwait 1");
> - sbunlock(&so->so_rcv);
> + sbunlock(so, &so->so_rcv);
> error = sbwait(so, &so->so_rcv);
> sounlock(s);
> if (error)
> @@ -957,7 +957,7 @@ dontblock:
> SBLASTMBUFCHK(&so->so_rcv, "soreceive sbwait 2");
> error = sbwait(so, &so->so_rcv);
> if (error) {
> - sbunlock(&so->so_rcv);
> + sbunlock(so, &so->so_rcv);
> sounlock(s);
> return (0);
> }
> @@ -993,7 +993,7 @@ dontblock:
> }
> if (orig_resid == uio->uio_resid && orig_resid &&
> (flags & MSG_EOR) == 0 && (so->so_state & SS_CANTRCVMORE) == 0) {
> - sbunlock(&so->so_rcv);
> + sbunlock(so, &so->so_rcv);
> sounlock(s);
> goto restart;
> }
> @@ -1004,7 +1004,7 @@ dontblock:
> if (flagsp)
> *flagsp |= flags;
> release:
> - sbunlock(&so->so_rcv);
> + sbunlock(so, &so->so_rcv);
> sounlock(s);
> return (error);
> }
> @@ -1049,7 +1049,7 @@ sorflush(struct socket *so)
> /* with SB_NOINTR and M_WAITOK sblock() must not fail */
> KASSERT(error == 0);
> socantrcvmore(so);
> - sbunlock(sb);
> + sbunlock(so, sb);
> aso.so_proto = pr;
> aso.so_rcv = *sb;
> memset(sb, 0, sizeof (*sb));
> @@ -1110,7 +1110,7 @@ sosplice(struct socket *so, int fd, off_
> }
> if (so->so_sp->ssp_socket)
> sounsplice(so, so->so_sp->ssp_socket, 1);
> - sbunlock(&so->so_rcv);
> + sbunlock(so, &so->so_rcv);
> return (0);
> }
>
> @@ -1139,7 +1139,7 @@ sosplice(struct socket *so, int fd, off_
> return (error);
> }
> if ((error = sblock(so, &sosp->so_snd, M_WAITOK)) != 0) {
> - sbunlock(&so->so_rcv);
> + sbunlock(so, &so->so_rcv);
> FRELE(fp, curproc);
> return (error);
> }
> @@ -1183,8 +1183,8 @@ sosplice(struct socket *so, int fd, off_
> }
>
> release:
> - sbunlock(&sosp->so_snd);
> - sbunlock(&so->so_rcv);
> + sbunlock(sosp, &sosp->so_snd);
> + sbunlock(so, &so->so_rcv);
> FRELE(fp, curproc);
> return (error);
> }
> Index: kern/uipc_socket2.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
> retrieving revision 1.86
> diff -u -p -r1.86 uipc_socket2.c
> --- kern/uipc_socket2.c 11 Aug 2017 21:24:19 -0000 1.86
> +++ kern/uipc_socket2.c 22 Nov 2017 11:22:37 -0000
> @@ -342,7 +342,6 @@ sblock(struct socket *so, struct sockbuf
> {
> int error, prio = (sb->sb_flags & SB_NOINTR) ? PSOCK : PSOCK | PCATCH;
>
> - KERNEL_ASSERT_LOCKED();
> soassertlocked(so);
>
> if ((sb->sb_flags & SB_LOCK) == 0) {
> @@ -363,9 +362,9 @@ sblock(struct socket *so, struct sockbuf
> }
>
> void
> -sbunlock(struct sockbuf *sb)
> +sbunlock(struct socket *so, struct sockbuf *sb)
> {
> - KERNEL_ASSERT_LOCKED();
> + soassertlocked(so);
>
> sb->sb_flags &= ~SB_LOCK;
> if (sb->sb_flags & SB_WANT) {
> Index: sys/socketvar.h
> ===================================================================
> RCS file: /cvs/src/sys/sys/socketvar.h,v
> retrieving revision 1.77
> diff -u -p -r1.77 socketvar.h
> --- sys/socketvar.h 4 Nov 2017 14:13:53 -0000 1.77
> +++ sys/socketvar.h 22 Nov 2017 11:22:49 -0000
> @@ -244,7 +244,7 @@ soreadable(struct socket *so)
> int sblock(struct socket *, struct sockbuf *, int);
>
> /* release lock on sockbuf sb */
> -void sbunlock(struct sockbuf *);
> +void sbunlock(struct socket *, struct sockbuf *);
>
> #define SB_EMPTY_FIXUP(sb) do {
> \
> if ((sb)->sb_mb == NULL) { \
