Get rid of locking during UDP and TCP listen

We already have a synchronization mechanism when starting the UDP and
TCP listener children - barriers.  Change how we start the first-born
child (tid == 0), so we don't have to race for sock->parent->result and
sock->parent->fd.

diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index d85742fe9e0a8b1a916e6f3bceee2e5009663dbe..10eac867adc5b4d44a69b0ba7e5ce8fdd31ff0a6 100644 (file)
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -663,8 +663,6 @@ nmsocket_cleanup(isc_nmsocket_t *sock) {
 
        sock->magic = 0;
 
-       isc_mutex_destroy(&sock->lock);
-
        /* Don't free child socket */
        if (sock->parent == NULL) {
                REQUIRE(sock->tid == isc_tid());
@@ -858,8 +856,6 @@ isc___nmsocket_init(isc_nmsocket_t *sock, isc__networker_t *worker,
                .active_link = ISC_LINK_INITIALIZER,
        };
 
-       isc_mutex_init(&sock->lock);
-
        if (iface != NULL) {
                family = iface->type.sa.sa_family;
                sock->iface = *iface;

diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c
index c54ba1171d1b287e797e8af00576b3ff2e80b8bc..d0ce958278fc968cec51aef1e430c7f795c9c015 100644 (file)
--- a/lib/isc/netmgr/tcp.c
+++ b/lib/isc/netmgr/tcp.c
@@ -417,20 +417,23 @@ isc_nm_listentcp(isc_nm_t *mgr, uint32_t workers, isc_sockaddr_t *iface,
                fd = isc__nm_tcp_lb_socket(mgr, iface->type.sa.sa_family);
        }
 
+       start_tcp_child(mgr, iface, sock, fd, 0);
+       result = sock->children[0].result;
+       INSIST(result != ISC_R_UNSET);
+
        for (size_t i = 1; i < sock->nchildren; i++) {
                start_tcp_child(mgr, iface, sock, fd, i);
        }
 
-       start_tcp_child(mgr, iface, sock, fd, 0);
+       isc_barrier_wait(&sock->listen_barrier);
 
        if (!mgr->load_balance_sockets) {
                isc__nm_closesocket(fd);
        }
 
-       LOCK(&sock->lock);
-       result = sock->result;
-       UNLOCK(&sock->lock);
-       INSIST(result != ISC_R_UNSET);
+       for (size_t i = 1; i < sock->nchildren; i++) {
+               INSIST(result == sock->children[i].result);
+       }
 
        atomic_store(&sock->active, true);
 
@@ -457,14 +460,12 @@ isc__nm_async_tcplisten(isc__networker_t *worker, isc__netievent_t *ev0) {
        isc_result_t result = ISC_R_UNSET;
 
        REQUIRE(VALID_NMSOCK(ievent->sock));
-       REQUIRE(ievent->sock->tid == isc_tid());
        REQUIRE(VALID_NMSOCK(ievent->sock->parent));
 
        sock = ievent->sock;
        sa_family = sock->iface.type.sa.sa_family;
 
        REQUIRE(sock->type == isc_nm_tcpsocket);
-       REQUIRE(sock->parent != NULL);
        REQUIRE(sock->tid == isc_tid());
 
        (void)isc__nm_socket_min_mtu(sock->fd, sa_family);
@@ -499,25 +500,17 @@ isc__nm_async_tcplisten(isc__networker_t *worker, isc__netievent_t *ev0) {
                        isc__nm_incstats(sock, STATID_BINDFAIL);
                        goto done;
                }
-       } else {
-               LOCK(&sock->parent->lock);
-               if (sock->parent->fd == -1) {
-                       r = isc__nm_tcp_freebind(&sock->uv_handle.tcp,
-                                                &sock->iface.type.sa, flags);
-                       if (r < 0) {
-                               isc__nm_incstats(sock, STATID_BINDFAIL);
-                               UNLOCK(&sock->parent->lock);
-                               goto done;
-                       }
-                       sock->parent->uv_handle.tcp.flags =
-                               sock->uv_handle.tcp.flags;
-                       sock->parent->fd = sock->fd;
-               } else {
-                       /* The socket is already bound, just copy the flags */
-                       sock->uv_handle.tcp.flags =
-                               sock->parent->uv_handle.tcp.flags;
+       } else if (sock->tid == 0) {
+               r = isc__nm_tcp_freebind(&sock->uv_handle.tcp,
+                                        &sock->iface.type.sa, flags);
+               if (r < 0) {
+                       isc__nm_incstats(sock, STATID_BINDFAIL);
+                       goto done;
                }
-               UNLOCK(&sock->parent->lock);
+               sock->parent->uv_handle.tcp.flags = sock->uv_handle.tcp.flags;
+       } else {
+               /* The socket is already bound, just copy the flags */
+               sock->uv_handle.tcp.flags = sock->parent->uv_handle.tcp.flags;
        }
 
        isc__nm_set_network_buffers(sock->worker->netmgr,
@@ -546,16 +539,13 @@ done:
                sock->pquota = NULL;
        }
 
-       LOCK(&sock->parent->lock);
-       if (sock->parent->result == ISC_R_UNSET) {
-               sock->parent->result = result;
-       } else {
-               REQUIRE(sock->parent->result == result);
-       }
-       UNLOCK(&sock->parent->lock);
+       sock->result = result;
 
        REQUIRE(!worker->loop->paused);
-       isc_barrier_wait(&sock->parent->listen_barrier);
+
+       if (sock->tid != 0) {
+               isc_barrier_wait(&sock->parent->listen_barrier);
+       }
 }
 
 static void

diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c
index 578842528cde4f5cdb9ccdce7da51e7582e7ec50..9072d688b463f75a6b3eecad14e986cefdf35ad4 100644 (file)
--- a/lib/isc/netmgr/udp.c
+++ b/lib/isc/netmgr/udp.c
@@ -163,20 +163,23 @@ isc_nm_listenudp(isc_nm_t *mgr, uint32_t workers, isc_sockaddr_t *iface,
                fd = isc__nm_udp_lb_socket(mgr, iface->type.sa.sa_family);
        }
 
+       start_udp_child(mgr, iface, sock, fd, 0);
+       result = sock->children[0].result;
+       INSIST(result != ISC_R_UNSET);
+
        for (size_t i = 1; i < sock->nchildren; i++) {
                start_udp_child(mgr, iface, sock, fd, i);
        }
 
-       start_udp_child(mgr, iface, sock, fd, 0);
+       isc_barrier_wait(&sock->listen_barrier);
 
        if (!mgr->load_balance_sockets) {
                isc__nm_closesocket(fd);
        }
 
-       LOCK(&sock->lock);
-       result = sock->result;
-       UNLOCK(&sock->lock);
-       INSIST(result != ISC_R_UNSET);
+       for (size_t i = 1; i < sock->nchildren; i++) {
+               INSIST(result == sock->children[i].result);
+       }
 
        atomic_store(&sock->active, true);
 
@@ -332,7 +335,6 @@ isc__nm_async_udplisten(isc__networker_t *worker, isc__netievent_t *ev0) {
        isc_nm_t *mgr = NULL;
 
        REQUIRE(VALID_NMSOCK(ievent->sock));
-       REQUIRE(ievent->sock->tid == isc_tid());
        REQUIRE(VALID_NMSOCK(ievent->sock->parent));
 
        sock = ievent->sock;
@@ -340,7 +342,6 @@ isc__nm_async_udplisten(isc__networker_t *worker, isc__netievent_t *ev0) {
        mgr = sock->worker->netmgr;
 
        REQUIRE(sock->type == isc_nm_udpsocket);
-       REQUIRE(sock->parent != NULL);
        REQUIRE(sock->tid == isc_tid());
 
        (void)isc__nm_socket_min_mtu(sock->fd, sa_family);
@@ -379,27 +380,19 @@ isc__nm_async_udplisten(isc__networker_t *worker, isc__netievent_t *ev0) {
                        isc__nm_incstats(sock, STATID_BINDFAIL);
                        goto done;
                }
-       } else {
-               LOCK(&sock->parent->lock);
-               if (sock->parent->fd == -1) {
-                       /* This thread is first, bind the socket */
-                       r = isc__nm_udp_freebind(&sock->uv_handle.udp,
-                                                &sock->parent->iface.type.sa,
-                                                uv_bind_flags);
-                       if (r < 0) {
-                               isc__nm_incstats(sock, STATID_BINDFAIL);
-                               UNLOCK(&sock->parent->lock);
-                               goto done;
-                       }
-                       sock->parent->uv_handle.udp.flags =
-                               sock->uv_handle.udp.flags;
-                       sock->parent->fd = sock->fd;
-               } else {
-                       /* The socket is already bound, just copy the flags */
-                       sock->uv_handle.udp.flags =
-                               sock->parent->uv_handle.udp.flags;
+       } else if (sock->tid == 0) {
+               /* This thread is first, bind the socket */
+               r = isc__nm_udp_freebind(&sock->uv_handle.udp,
+                                        &sock->parent->iface.type.sa,
+                                        uv_bind_flags);
+               if (r < 0) {
+                       isc__nm_incstats(sock, STATID_BINDFAIL);
+                       goto done;
                }
-               UNLOCK(&sock->parent->lock);
+               sock->parent->uv_handle.udp.flags = sock->uv_handle.udp.flags;
+       } else {
+               /* The socket is already bound, just copy the flags */
+               sock->uv_handle.udp.flags = sock->parent->uv_handle.udp.flags;
        }
 
        isc__nm_set_network_buffers(mgr, &sock->uv_handle.handle);
@@ -417,16 +410,13 @@ done:
        result = isc_uverr2result(r);
        atomic_fetch_add(&sock->parent->rchildren, 1);
 
-       LOCK(&sock->parent->lock);
-       if (sock->parent->result == ISC_R_UNSET) {
-               sock->parent->result = result;
-       } else {
-               REQUIRE(sock->parent->result == result);
-       }
-       UNLOCK(&sock->parent->lock);
+       sock->result = result;
 
        REQUIRE(!worker->loop->paused);
-       isc_barrier_wait(&sock->parent->listen_barrier);
+
+       if (sock->tid != 0) {
+               isc_barrier_wait(&sock->parent->listen_barrier);
+       }
 }
 
 static void