Hi,

I would like to remove a useless kernel lock during socket splicing.

We have a socket "so" that splices data to socket "sosp".  Everytime
when space in sosp gets available, we add a task to move data from
so to sosp.  Additionally we call sowakeup() from sowwakeup().  I
have added this as it is possible to splice from so to sosp and
write from userland to sosp simultaneously.  In practice this does
not make sense as the data streams from two sources would get mixed.
Nothing uses this.  So it is not neccessay to inform userland that
it is possible to write.

Note that sowakeup() takes the kernel lock.  So when I do not call
it for a spliced socket, there is no kernel lock in the TCP splicing
path anymore.

But then I have to wakeup userland for the wirting socket in
sounsplice().

ok?

bluhm

Index: sys/kern/uipc_socket.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.231
diff -u -p -r1.231 uipc_socket.c
--- sys/kern/uipc_socket.c      17 Dec 2018 16:46:59 -0000      1.231
+++ sys/kern/uipc_socket.c      3 Jul 2019 13:56:09 -0000
@@ -220,9 +220,10 @@ sofree(struct socket *so, int s)
        if (so->so_sp) {
                if (issplicedback(so))
                        sounsplice(so->so_sp->ssp_soback, so,
-                           so->so_sp->ssp_soback != so);
+                           so->so_sp->ssp_soback == so ? 3 : 2);
                if (isspliced(so))
-                       sounsplice(so, so->so_sp->ssp_socket, 0);
+                       sounsplice(so, so->so_sp->ssp_socket,
+                           so == so->so_sp->ssp_socket ? 3 : 1);
        }
 #endif /* SOCKET_SPLICE */
        sbrelease(so, &so->so_snd);
@@ -1148,7 +1149,7 @@ sosplice(struct socket *so, int fd, off_
                        return (error);
                }
                if (so->so_sp->ssp_socket)
-                       sounsplice(so, so->so_sp->ssp_socket, 1);
+                       sounsplice(so, so->so_sp->ssp_socket, 0);
                sbunlock(so, &so->so_rcv);
                return (0);
        }
@@ -1227,7 +1228,7 @@ sosplice(struct socket *so, int fd, off_
 }

 void
-sounsplice(struct socket *so, struct socket *sosp, int wakeup)
+sounsplice(struct socket *so, struct socket *sosp, int freeing)
 {
        soassertlocked(so);

@@ -1236,8 +1237,11 @@ sounsplice(struct socket *so, struct soc
        sosp->so_snd.sb_flags &= ~SB_SPLICE;
        so->so_rcv.sb_flags &= ~SB_SPLICE;
        so->so_sp->ssp_socket = sosp->so_sp->ssp_soback = NULL;
-       if (wakeup && soreadable(so))
+       /* Do not wakeup a socket that is about to be freed. */
+       if ((freeing & 1) == 0 && soreadable(so))
                sorwakeup(so);
+       if ((freeing & 2) == 0 && sowriteable(sosp))
+               sowwakeup(sosp);
 }

 void
@@ -1249,7 +1253,7 @@ soidle(void *arg)
        s = solock(so);
        if (so->so_rcv.sb_flags & SB_SPLICE) {
                so->so_error = ETIMEDOUT;
-               sounsplice(so, so->so_sp->ssp_socket, 1);
+               sounsplice(so, so->so_sp->ssp_socket, 0);
        }
        sounlock(so, s);
 }
@@ -1574,7 +1578,7 @@ somove(struct socket *so, int wait)
                so->so_error = error;
        if (((so->so_state & SS_CANTRCVMORE) && so->so_rcv.sb_cc == 0) ||
            (sosp->so_state & SS_CANTSENDMORE) || maxreached || error) {
-               sounsplice(so, sosp, 1);
+               sounsplice(so, sosp, 0);
                return (0);
        }
        if (timerisset(&so->so_idletv))
@@ -1620,6 +1624,8 @@ sowwakeup(struct socket *so)
 #ifdef SOCKET_SPLICE
        if (so->so_snd.sb_flags & SB_SPLICE)
                task_add(sosplice_taskq, &so->so_sp->ssp_soback->so_splicetask);
+       if (issplicedback(so))
+               return;
 #endif
        sowakeup(so, &so->so_snd);
 }
Index: share/man/man9/sosplice.9
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/share/man/man9/sosplice.9,v
retrieving revision 1.9
diff -u -p -r1.9 sosplice.9
--- share/man/man9/sosplice.9   15 Aug 2018 12:10:49 -0000      1.9
+++ share/man/man9/sosplice.9   3 Jul 2019 21:42:34 -0000
@@ -206,10 +206,13 @@ will call
 to trigger the transfer when new data or buffer space is available.
 While socket splicing is active, any
 .Xr read 2
-from the source socket will block and the wakeup will not be delivered
-to the file descriptor.
-A read event or a socket error is signaled to userland after
-dissolving.
+from the source socket will block.
+Neither read nor write wakeups will be delivered to the file
+descriptors.
+After dissolving, a read event or a socket error is signaled to
+userland on the source socket.
+If space is available, a write event will be signaled on the drain
+socket.
 .Sh RETURN VALUES
 .Fn sosplice
 returns 0 on success and otherwise the error number.

Reply via email to