From 359faf2ff76c0230ccec2366ecc82017b4472c09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 4 Jan 2023 15:57:00 +0100 Subject: [PATCH 1/2] Convert isc_astack usage in netmgr to mempool and ISC_LIST Change the per-socket inactive uvreq cache (implemented as isc_astack) to per-worker memory pool. Change the per-socket inactive nmhandle cache (implemented as isc_astack) to unlocked per-socket ISC_LIST. --- lib/isc/netmgr/netmgr-int.h | 7 ++- lib/isc/netmgr/netmgr.c | 122 ++++++++++++++++-------------------- 2 files changed, 58 insertions(+), 71 deletions(-) diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index f4bb0f0871..4a722130fa 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -18,7 +18,6 @@ #include #include -#include #include #include #include @@ -201,6 +200,7 @@ typedef struct isc__networker { ISC_LIST(isc_nmsocket_t) active_sockets; + isc_mempool_t *uvreq_pool; } isc__networker_t; ISC_REFCOUNT_DECL(isc__networker); @@ -245,6 +245,7 @@ struct isc_nmhandle { int backtrace_size; #endif LINK(isc_nmhandle_t) active_link; + LINK(isc_nmhandle_t) inactive_link; void *opaque; }; @@ -323,6 +324,7 @@ struct isc__nm_uvreq { uv_fs_t fs; } uv_req; ISC_LINK(isc__nm_uvreq_t) link; + ISC_LINK(isc__nm_uvreq_t) inactive_link; }; void * @@ -987,8 +989,7 @@ struct isc_nmsocket { * 'spare' handles for that can be reused to avoid allocations, * for UDP. */ - isc_astack_t *inactivehandles; - isc_astack_t *inactivereqs; + ISC_LIST(isc_nmhandle_t) inactive_handles; /*% * Used to pass a result back from listen or connect events. diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 613112e816..d85742fe9e 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -248,6 +248,11 @@ isc_netmgr_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, isc_nm_t **netmgrp) { isc_mem_attach(loop->mctx, &worker->mctx); + isc_mempool_create(worker->mctx, sizeof(isc__nm_uvreq_t), + &worker->uvreq_pool); + isc_mempool_setfreemax(worker->uvreq_pool, + ISC_NM_REQS_STACK_SIZE); + isc_loop_attach(loop, &worker->loop); isc_loop_teardown(loop, networker_teardown, worker); isc_refcount_init(&worker->references, 1); @@ -596,7 +601,6 @@ nmsocket_cleanup(isc_nmsocket_t *sock) { REQUIRE(!isc__nmsocket_active(sock)); isc_nmhandle_t *handle = NULL; - isc__nm_uvreq_t *uvreq = NULL; isc__networker_t *worker = sock->worker; isc_refcount_destroy(&sock->references); @@ -635,7 +639,8 @@ nmsocket_cleanup(isc_nmsocket_t *sock) { isc___nmsocket_detach(&sock->outer FLARG_PASS); } - while ((handle = isc_astack_pop(sock->inactivehandles)) != NULL) { + while ((handle = ISC_LIST_HEAD(sock->inactive_handles)) != NULL) { + ISC_LIST_DEQUEUE(sock->inactive_handles, handle, inactive_link); nmhandle_free(sock, handle); } @@ -645,14 +650,6 @@ nmsocket_cleanup(isc_nmsocket_t *sock) { sock->pquota = NULL; - isc_astack_destroy(sock->inactivehandles); - - while ((uvreq = isc_astack_pop(sock->inactivereqs)) != NULL) { - isc_mem_put(sock->worker->mctx, uvreq, sizeof(*uvreq)); - } - - isc_astack_destroy(sock->inactivereqs); - isc__nm_tls_cleanup_data(sock); #if HAVE_LIBNGHTTP2 isc__nm_http_cleanup_data(sock); @@ -855,10 +852,7 @@ isc___nmsocket_init(isc_nmsocket_t *sock, isc__networker_t *worker, .type = type, .tid = worker->loop->tid, .fd = -1, - .inactivehandles = isc_astack_new(worker->mctx, - ISC_NM_HANDLES_STACK_SIZE), - .inactivereqs = isc_astack_new(worker->mctx, - ISC_NM_REQS_STACK_SIZE), + .inactive_handles = ISC_LIST_INITIALIZER, .result = ISC_R_UNSET, .active_handles = ISC_LIST_INITIALIZER, .active_link = ISC_LINK_INITIALIZER, @@ -988,27 +982,38 @@ alloc_handle(isc_nmsocket_t *sock) { *handle = (isc_nmhandle_t){ .magic = NMHANDLE_MAGIC, .active_link = ISC_LINK_INITIALIZER, + .inactive_link = ISC_LINK_INITIALIZER, }; isc_refcount_init(&handle->references, 1); return (handle); } +static isc_nmhandle_t * +dequeue_handle(isc_nmsocket_t *sock) { +#if !__SANITIZE_ADDRESS__ && !__SANITIZE_THREAD__ + isc_nmhandle_t *handle = ISC_LIST_HEAD(sock->inactive_handles); + if (handle != NULL) { + ISC_LIST_DEQUEUE(sock->inactive_handles, handle, inactive_link); + + isc_refcount_init(&handle->references, 1); + INSIST(VALID_NMHANDLE(handle)); + return (handle); + } +#else + UNUSED(sock); +#endif /* !__SANITIZE_ADDRESS__ && !__SANITIZE_THREAD__ */ + return (NULL); +} + isc_nmhandle_t * isc___nmhandle_get(isc_nmsocket_t *sock, isc_sockaddr_t const *peer, isc_sockaddr_t const *local FLARG) { - isc_nmhandle_t *handle = NULL; - REQUIRE(VALID_NMSOCK(sock)); - handle = isc_astack_pop(sock->inactivehandles); - + isc_nmhandle_t *handle = dequeue_handle(sock); if (handle == NULL) { handle = alloc_handle(sock); - } else { - isc_refcount_init(&handle->references, 1); - INSIST(VALID_NMHANDLE(handle)); - ISC_LINK_INIT(handle, active_link); } NETMGR_TRACE_LOG( @@ -1107,25 +1112,17 @@ nmhandle_free(isc_nmsocket_t *sock, isc_nmhandle_t *handle) { static void nmhandle_deactivate(isc_nmsocket_t *sock, isc_nmhandle_t *handle) { - bool reuse = false; - uint_fast32_t ah; - - /* - * We do all of this under lock to avoid races with socket - * destruction. We have to do this now, because at this point the - * socket is either unused or still attached to event->sock. - */ - ISC_LIST_UNLINK(sock->active_handles, handle, active_link); - - ah = atomic_fetch_sub(&sock->ah, 1); + uint_fast32_t ah = atomic_fetch_sub(&sock->ah, 1); INSIST(ah > 0); + ISC_LIST_UNLINK(sock->active_handles, handle, active_link); + #if !__SANITIZE_ADDRESS__ && !__SANITIZE_THREAD__ if (atomic_load(&sock->active)) { - reuse = isc_astack_trypush(sock->inactivehandles, handle); - } + ISC_LIST_APPEND(sock->inactive_handles, handle, inactive_link); + } else #endif /* !__SANITIZE_ADDRESS__ && !__SANITIZE_THREAD__ */ - if (!reuse) { + { nmhandle_free(sock, handle); } } @@ -1196,6 +1193,11 @@ nmhandle_detach_cb(isc_nmhandle_t **handlep FLARG) { } #endif + if (handle == sock->statichandle) { + /* statichandle is assigned, not attached. */ + sock->statichandle = NULL; + } + nmhandle_deactivate(sock, handle); /* @@ -1207,11 +1209,6 @@ nmhandle_detach_cb(isc_nmhandle_t **handlep FLARG) { sock->closehandle_cb(sock); } - if (handle == sock->statichandle) { - /* statichandle is assigned, not attached. */ - sock->statichandle = NULL; - } - isc___nmsocket_detach(&sock FLARG_PASS); } @@ -1761,35 +1758,31 @@ isc___nm_uvreq_get(isc__networker_t *worker, isc_nmsocket_t *sock FLARG) { REQUIRE(worker != NULL); REQUIRE(VALID_NMSOCK(sock)); + REQUIRE(sock->tid == isc_tid()); - if (sock != NULL && isc__nmsocket_active(sock)) { - /* Try to reuse one */ - req = isc_astack_pop(sock->inactivereqs); - } - - if (req == NULL) { - req = isc_mem_get(worker->mctx, sizeof(*req)); - } - + req = isc_mempool_get(worker->uvreq_pool); *req = (isc__nm_uvreq_t){ - .magic = 0, .connect_tries = 3, + .link = ISC_LINK_INITIALIZER, + .inactive_link = ISC_LINK_INITIALIZER, + .uv_req.req.data = req, + .magic = UVREQ_MAGIC, }; - ISC_LINK_INIT(req, link); - req->uv_req.req.data = req; isc___nmsocket_attach(sock, &req->sock FLARG_PASS); - req->magic = UVREQ_MAGIC; return (req); } void isc___nm_uvreq_put(isc__nm_uvreq_t **req0, isc_nmsocket_t *sock FLARG) { - isc__nm_uvreq_t *req = NULL; - isc_nmhandle_t *handle = NULL; - REQUIRE(req0 != NULL); REQUIRE(VALID_UVREQ(*req0)); + REQUIRE(VALID_NMSOCK(sock)); + REQUIRE(sock->tid == isc_tid()); + + isc__nm_uvreq_t *req = NULL; + isc_nmhandle_t *handle = NULL; + isc__networker_t *worker = sock->worker; req = *req0; *req0 = NULL; @@ -1802,18 +1795,9 @@ isc___nm_uvreq_put(isc__nm_uvreq_t **req0, isc_nmsocket_t *sock FLARG) { * We need to save this first to make sure that handle, * sock, and the netmgr won't all disappear. */ - handle = req->handle; - req->handle = NULL; + ISC_SWAP(handle, req->handle); -#if !__SANITIZE_ADDRESS__ && !__SANITIZE_THREAD__ - if (!isc__nmsocket_active(sock) || - !isc_astack_trypush(sock->inactivereqs, req)) - { - isc_mem_put(sock->worker->mctx, req, sizeof(*req)); - } -#else /* !__SANITIZE_ADDRESS__ && !__SANITIZE_THREAD__ */ - isc_mem_put(sock->worker->mctx, req, sizeof(*req)); -#endif /* !__SANITIZE_ADDRESS__ && !__SANITIZE_THREAD__ */ + isc_mempool_put(worker->uvreq_pool, req); if (handle != NULL) { isc__nmhandle_detach(&handle FLARG_PASS); @@ -2633,6 +2617,8 @@ isc__networker_destroy(isc__networker_t *worker) { isc_loop_detach(&worker->loop); + isc_mempool_destroy(&worker->uvreq_pool); + isc_mem_put(worker->mctx, worker->sendbuf, ISC_NETMGR_SENDBUF_SIZE); isc_mem_putanddetach(&worker->mctx, worker->recvbuf, ISC_NETMGR_RECVBUF_SIZE); From 10f884a5b8c7f900851fbba3c7f9f30d01fcbc1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 4 Jan 2023 16:06:48 +0100 Subject: [PATCH 2/2] Remove unused isc_astack unit The isc_astack unit is now unused, so just remove it. --- lib/isc/Makefile.am | 2 - lib/isc/astack.c | 85 ------------------------------------ lib/isc/include/isc/astack.h | 49 --------------------- lib/isc/include/isc/types.h | 5 +-- 4 files changed, 2 insertions(+), 139 deletions(-) delete mode 100644 lib/isc/astack.c delete mode 100644 lib/isc/include/isc/astack.h diff --git a/lib/isc/Makefile.am b/lib/isc/Makefile.am index fabefc6de4..7c1466c7d8 100644 --- a/lib/isc/Makefile.am +++ b/lib/isc/Makefile.am @@ -8,7 +8,6 @@ libisc_la_HEADERS = \ include/isc/align.h \ include/isc/ascii.h \ include/isc/assertions.h \ - include/isc/astack.h \ include/isc/async.h \ include/isc/atomic.h \ include/isc/attributes.h \ @@ -122,7 +121,6 @@ libisc_la_SOURCES = \ aes.c \ ascii.c \ assertions.c \ - astack.c \ async.c \ backtrace.c \ base32.c \ diff --git a/lib/isc/astack.c b/lib/isc/astack.c deleted file mode 100644 index 484ef26e87..0000000000 --- a/lib/isc/astack.c +++ /dev/null @@ -1,85 +0,0 @@ -/* - * Copyright (C) Internet Systems Consortium, Inc. ("ISC") - * - * SPDX-License-Identifier: MPL-2.0 - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, you can obtain one at https://mozilla.org/MPL/2.0/. - * - * See the COPYRIGHT file distributed with this work for additional - * information regarding copyright ownership. - */ - -#include -#include - -#include -#include -#include -#include -#include -#include - -struct isc_astack { - isc_mem_t *mctx; - size_t size; - size_t pos; - isc_mutex_t lock; - uintptr_t nodes[]; -}; - -isc_astack_t * -isc_astack_new(isc_mem_t *mctx, size_t size) { - isc_astack_t *stack = isc_mem_get( - mctx, sizeof(isc_astack_t) + size * sizeof(uintptr_t)); - - *stack = (isc_astack_t){ - .size = size, - }; - isc_mem_attach(mctx, &stack->mctx); - memset(stack->nodes, 0, size * sizeof(uintptr_t)); - isc_mutex_init(&stack->lock); - return (stack); -} - -bool -isc_astack_trypush(isc_astack_t *stack, void *obj) { - if (!isc_mutex_trylock(&stack->lock)) { - if (stack->pos >= stack->size) { - UNLOCK(&stack->lock); - return (false); - } - stack->nodes[stack->pos++] = (uintptr_t)obj; - UNLOCK(&stack->lock); - return (true); - } else { - return (false); - } -} - -void * -isc_astack_pop(isc_astack_t *stack) { - LOCK(&stack->lock); - uintptr_t rv; - if (stack->pos == 0) { - rv = 0; - } else { - rv = stack->nodes[--stack->pos]; - } - UNLOCK(&stack->lock); - return ((void *)rv); -} - -void -isc_astack_destroy(isc_astack_t *stack) { - LOCK(&stack->lock); - REQUIRE(stack->pos == 0); - UNLOCK(&stack->lock); - - isc_mutex_destroy(&stack->lock); - - isc_mem_putanddetach(&stack->mctx, stack, - sizeof(struct isc_astack) + - stack->size * sizeof(uintptr_t)); -} diff --git a/lib/isc/include/isc/astack.h b/lib/isc/include/isc/astack.h deleted file mode 100644 index a4f6762259..0000000000 --- a/lib/isc/include/isc/astack.h +++ /dev/null @@ -1,49 +0,0 @@ -/* - * Copyright (C) Internet Systems Consortium, Inc. ("ISC") - * - * SPDX-License-Identifier: MPL-2.0 - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, you can obtain one at https://mozilla.org/MPL/2.0/. - * - * See the COPYRIGHT file distributed with this work for additional - * information regarding copyright ownership. - */ - -#pragma once - -#include - -#include -#include - -isc_astack_t * -isc_astack_new(isc_mem_t *mctx, size_t size); -/*%< - * Allocate and initialize a new array stack of size 'size'. - */ - -void -isc_astack_destroy(isc_astack_t *stack); -/*%< - * Free an array stack 'stack'. - * - * Requires: - * \li 'stack' is empty. - */ - -bool -isc_astack_trypush(isc_astack_t *stack, void *obj); -/*%< - * Try to push 'obj' onto array stack 'astack'. On failure, either - * because the stack size limit has been reached or because another - * thread has already changed the stack pointer, return 'false'. - */ - -void * -isc_astack_pop(isc_astack_t *stack); -/*%< - * Pop an object off of array stack 'stack'. If the stack is empty, - * return NULL. - */ diff --git a/lib/isc/include/isc/types.h b/lib/isc/include/isc/types.h index 72cdabd7cf..475cc1f411 100644 --- a/lib/isc/include/isc/types.h +++ b/lib/isc/include/isc/types.h @@ -33,9 +33,8 @@ /* Core Types. Alphabetized by defined type. */ -typedef struct isc_astack isc_astack_t; /*%< Array-based fast stack */ -typedef struct isc_buffer isc_buffer_t; /*%< Buffer */ -typedef ISC_LIST(isc_buffer_t) isc_bufferlist_t; /*%< Buffer List */ +typedef struct isc_buffer isc_buffer_t; /*%< Buffer */ +typedef ISC_LIST(isc_buffer_t) isc_bufferlist_t; /*%< Buffer List */ typedef struct isc_constregion isc_constregion_t; /*%< Const region */ typedef struct isc_consttextregion isc_consttextregion_t; /*%< Const Text Region */