From ee4347d7db9f66bff11d7a47d2c1dab5809b6509 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= Date: Tue, 1 Mar 2022 16:43:39 +0100 Subject: [PATCH] Replace downstream change with upstream proposal bind-dyndb-ldap requires sending from custom spawned thread to main named threads. Change queue type to locked variant, which would not crash when isc_send_task() is called from dyndb worker thread. Related: rhbz#2048235 --- bind-9.16-hp-workers-rh2048235-dyndb.patch | 116 ----------- bind-9.16-hp-workers-rh2048235.patch | 180 ----------------- bind-9.16-locked-isc-queue.patch | 214 +++++++++++++++++++++ bind.spec | 11 +- 4 files changed, 221 insertions(+), 300 deletions(-) delete mode 100644 bind-9.16-hp-workers-rh2048235-dyndb.patch delete mode 100644 bind-9.16-hp-workers-rh2048235.patch create mode 100644 bind-9.16-locked-isc-queue.patch diff --git a/bind-9.16-hp-workers-rh2048235-dyndb.patch b/bind-9.16-hp-workers-rh2048235-dyndb.patch deleted file mode 100644 index 12ae5ad..0000000 --- a/bind-9.16-hp-workers-rh2048235-dyndb.patch +++ /dev/null @@ -1,116 +0,0 @@ -From b6320643194a3b435f2a6ae4569ace19db6ac795 Mon Sep 17 00:00:00 2001 -From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= -Date: Fri, 11 Feb 2022 14:19:10 +0100 -Subject: [PATCH] Propagate extra hp threads to dyndb plugins - -Plugin may require extra threads for processing its own source data. If -that plugin needs to call isc_task_send() from those threads, it needs -extra hp threads initialized. Allow plugin to read how many such threads -it can use. Allows plugin to fail initialization if not enough workers -were initialized. ---- - bin/named/server.c | 9 +++++---- - lib/dns/dyndb.c | 4 +++- - lib/dns/include/dns/dyndb.h | 22 +++++++++++++--------- - 3 files changed, 21 insertions(+), 14 deletions(-) - -diff --git a/bin/named/server.c b/bin/named/server.c -index 721464db8e..27e23fd52e 100644 ---- a/bin/named/server.c -+++ b/bin/named/server.c -@@ -5581,10 +5581,11 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, - - if (dctx == NULL) { - const void *hashinit = isc_hash_get_initializer(); -- CHECK(dns_dyndb_createctx(mctx, hashinit, named_g_lctx, -- view, named_g_server->zonemgr, -- named_g_server->task, -- named_g_timermgr, &dctx)); -+ CHECK(dns_dyndb_createctx( -+ mctx, hashinit, named_g_lctx, view, -+ named_g_server->zonemgr, named_g_server->task, -+ named_g_timermgr, named_g_hp_extra_workers, -+ &dctx)); - } - - CHECK(configure_dyndb(dyndb, mctx, dctx)); -diff --git a/lib/dns/dyndb.c b/lib/dns/dyndb.c -index 1caf90bab7..7d2784c5fa 100644 ---- a/lib/dns/dyndb.c -+++ b/lib/dns/dyndb.c -@@ -408,7 +408,8 @@ dns_dyndb_cleanup(bool exiting) { - isc_result_t - dns_dyndb_createctx(isc_mem_t *mctx, const void *hashinit, isc_log_t *lctx, - dns_view_t *view, dns_zonemgr_t *zmgr, isc_task_t *task, -- isc_timermgr_t *tmgr, dns_dyndbctx_t **dctxp) { -+ isc_timermgr_t *tmgr, unsigned int extra_workers, -+ dns_dyndbctx_t **dctxp) { - dns_dyndbctx_t *dctx; - - REQUIRE(dctxp != NULL && *dctxp == NULL); -@@ -429,6 +430,7 @@ dns_dyndb_createctx(isc_mem_t *mctx, const void *hashinit, isc_log_t *lctx, - dctx->hashinit = hashinit; - dctx->lctx = lctx; - dctx->memdebug = &isc_mem_debugging; -+ dctx->extra_workers = extra_workers; - - isc_mem_attach(mctx, &dctx->mctx); - dctx->magic = DNS_DYNDBCTX_MAGIC; -diff --git a/lib/dns/include/dns/dyndb.h b/lib/dns/include/dns/dyndb.h -index bda744a8d9..a93947d340 100644 ---- a/lib/dns/include/dns/dyndb.h -+++ b/lib/dns/include/dns/dyndb.h -@@ -35,14 +35,15 @@ ISC_LANG_BEGINDECLS - */ - struct dns_dyndbctx { - unsigned int magic; -- const void *hashinit; -- isc_mem_t *mctx; -- isc_log_t *lctx; -- dns_view_t *view; -- dns_zonemgr_t *zmgr; -- isc_task_t *task; -+ const void * hashinit; -+ isc_mem_t * mctx; -+ isc_log_t * lctx; -+ dns_view_t * view; -+ dns_zonemgr_t * zmgr; -+ isc_task_t * task; - isc_timermgr_t *timermgr; -- unsigned int *memdebug; -+ unsigned int * memdebug; -+ unsigned int extra_workers; - }; - - #define DNS_DYNDBCTX_MAGIC ISC_MAGIC('D', 'd', 'b', 'c') -@@ -58,7 +59,7 @@ struct dns_dyndbctx { - */ - #ifndef DNS_DYNDB_VERSION - #define DNS_DYNDB_VERSION 1 --#define DNS_DYNDB_AGE 0 -+#define DNS_DYNDB_AGE 1 - #endif /* ifndef DNS_DYNDB_VERSION */ - - typedef isc_result_t -@@ -135,7 +136,8 @@ dns_dyndb_cleanup(bool exiting); - isc_result_t - dns_dyndb_createctx(isc_mem_t *mctx, const void *hashinit, isc_log_t *lctx, - dns_view_t *view, dns_zonemgr_t *zmgr, isc_task_t *task, -- isc_timermgr_t *tmgr, dns_dyndbctx_t **dctxp); -+ isc_timermgr_t *tmgr, unsigned int extra_workers, -+ dns_dyndbctx_t **dctxp); - /*% - * Create a dyndb initialization context structure, with - * pointers to structures in the server that the dyndb module will -@@ -143,6 +145,8 @@ dns_dyndb_createctx(isc_mem_t *mctx, const void *hashinit, isc_log_t *lctx, - * etc). This structure is expected to last only until all dyndb - * modules have been loaded and initialized; after that it will be - * destroyed with dns_dyndb_destroyctx(). -+ * extra workers specifies how many extra worker threads can plugin -+ * use to send tasks to named threads by isc_task_send() - * - * Returns: - *\li #ISC_R_SUCCESS --- -2.34.1 - diff --git a/bind-9.16-hp-workers-rh2048235.patch b/bind-9.16-hp-workers-rh2048235.patch deleted file mode 100644 index 019274b..0000000 --- a/bind-9.16-hp-workers-rh2048235.patch +++ /dev/null @@ -1,180 +0,0 @@ -From 140ae14a4d6374530c9ab19a29c246abbf6d60b8 Mon Sep 17 00:00:00 2001 -From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= -Date: Fri, 11 Feb 2022 13:29:03 +0100 -Subject: [PATCH] Add extra hp workers parameter - -May be needed by dyndb plugins to prepare extra worker threads. If they -use isc_task_send, their threads need to have initialized enough thread -identifiers in isc_hp_init. New parameters allow that. - -Allows specification of both relative and absolute number of extra -workers. - -Adds new function isc_managers_create2() with extra parameter, call -original function from many tests and tools unchanged. ---- - bin/named/include/named/globals.h | 2 ++ - bin/named/include/named/main.h | 2 +- - bin/named/main.c | 29 +++++++++++++++++++++++++---- - bin/named/named.rst | 9 +++++++++ - lib/isc/include/isc/managers.h | 5 +++++ - lib/isc/managers.c | 11 ++++++++++- - 6 files changed, 52 insertions(+), 6 deletions(-) - -diff --git a/bin/named/include/named/globals.h b/bin/named/include/named/globals.h -index 82b632ef04..d46e62b94d 100644 ---- a/bin/named/include/named/globals.h -+++ b/bin/named/include/named/globals.h -@@ -53,6 +53,8 @@ EXTERN unsigned int named_g_udpdisp INIT(0); - EXTERN isc_taskmgr_t *named_g_taskmgr INIT(NULL); - EXTERN dns_dispatchmgr_t *named_g_dispatchmgr INIT(NULL); - EXTERN unsigned int named_g_cpus_detected INIT(1); -+EXTERN unsigned int named_g_hp_extra_workers INIT(0); -+EXTERN unsigned int named_g_hp_extra_percent INIT(100); - - #ifdef ENABLE_AFL - EXTERN bool named_g_run_done INIT(false); -diff --git a/bin/named/include/named/main.h b/bin/named/include/named/main.h -index 816aeae7a0..9618251f4d 100644 ---- a/bin/named/include/named/main.h -+++ b/bin/named/include/named/main.h -@@ -23,7 +23,7 @@ - /* - * Commandline arguments for named; also referenced in win32/ntservice.c - */ --#define NAMED_MAIN_ARGS "46A:c:d:D:E:fFgL:M:m:n:N:p:sS:t:T:U:u:vVx:X:" -+#define NAMED_MAIN_ARGS "46A:c:d:D:E:fFgh:H:L:M:m:n:N:p:sS:t:T:U:u:vVx:X:" - - ISC_PLATFORM_NORETURN_PRE void - named_main_earlyfatal(const char *format, ...) -diff --git a/bin/named/main.c b/bin/named/main.c -index 9ad2d0e277..fbe4c8d011 100644 ---- a/bin/named/main.c -+++ b/bin/named/main.c -@@ -351,7 +351,8 @@ usage(void) { - "username] [-U listeners]\n" - " [-X lockfile] [-m " - "{usage|trace|record|size|mctx}]\n" -- " [-M fill|nofill]\n" -+ " [-M fill|nofill] [-h extra_workers] " -+ "[-H extra_percent]\n" - "usage: named [-v|-V]\n"); - } - -@@ -792,6 +793,21 @@ parse_command_line(int argc, char *argv[]) { - named_g_foreground = true; - named_g_logstderr = true; - break; -+ case 'h': -+ named_g_hp_extra_workers = -+ parse_int(isc_commandline_argument, -+ "number of extra workers"); -+ break; -+ case 'H': -+ named_g_hp_extra_percent = -+ parse_int(isc_commandline_argument, -+ "percent of extra workers"); -+ if (named_g_hp_extra_percent < 100) { -+ named_main_earlyfatal( -+ "percent of extra workers " -+ "cannot be less than 100"); -+ } -+ break; - case 'L': - named_g_logfile = isc_commandline_argument; - break; -@@ -904,6 +920,10 @@ create_managers(void) { - if (named_g_cpus == 0) { - named_g_cpus = named_g_cpus_detected; - } -+ if (named_g_hp_extra_percent > 100) { -+ named_g_hp_extra_workers += -+ named_g_cpus * (named_g_hp_extra_percent - 100) / 100; -+ } - isc_log_write( - named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, - ISC_LOG_INFO, "found %u CPU%s, using %u worker thread%s", -@@ -924,11 +944,12 @@ create_managers(void) { - "using %u UDP listener%s per interface", named_g_udpdisp, - named_g_udpdisp == 1 ? "" : "s"); - -- result = isc_managers_create(named_g_mctx, named_g_cpus, 0, &named_g_nm, -- &named_g_taskmgr); -+ result = isc_managers_create2(named_g_mctx, named_g_cpus, 0, -+ named_g_hp_extra_workers, &named_g_nm, -+ &named_g_taskmgr); - if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR(__FILE__, __LINE__, -- "isc_managers_create() failed: %s", -+ "isc_managers_create2() failed: %s", - isc_result_totext(result)); - return (ISC_R_UNEXPECTED); - } -diff --git a/bin/named/named.rst b/bin/named/named.rst -index 39c445f5f9..b5bf95398c 100644 ---- a/bin/named/named.rst -+++ b/bin/named/named.rst -@@ -75,6 +75,15 @@ Options - ``-g`` - This option runs the server in the foreground and forces all logging to ``stderr``. - -+``-h #workers`` -+ This option requests extra worker threads initialized by hazard pointers. Used only for -+ dyndb plugins. -+ -+``-H workers%`` -+ This option requests extra worker threads initialized by hazard pointers. Value is -+ computed as percent of number of cpus set by ``-n`` parameter. Minimal value is 100%. -+ Used only for dyndb plugins. -+ - ``-L logfile`` - This option sets the log to the file ``logfile`` by default, instead of the system log. - -diff --git a/lib/isc/include/isc/managers.h b/lib/isc/include/isc/managers.h -index 0ed17ff31d..712ea3e3ed 100644 ---- a/lib/isc/include/isc/managers.h -+++ b/lib/isc/include/isc/managers.h -@@ -25,5 +25,10 @@ isc_result_t - isc_managers_create(isc_mem_t *mctx, size_t workers, size_t quantum, - isc_nm_t **netmgrp, isc_taskmgr_t **taskmgrp); - -+isc_result_t -+isc_managers_create2(isc_mem_t *mctx, size_t workers, size_t quantum, -+ size_t hp_extra, isc_nm_t **netmgrp, -+ isc_taskmgr_t **taskmgrp); -+ - void - isc_managers_destroy(isc_nm_t **netmgrp, isc_taskmgr_t **taskmgrp); -diff --git a/lib/isc/managers.c b/lib/isc/managers.c -index a285e80d8a..daf19c5596 100644 ---- a/lib/isc/managers.c -+++ b/lib/isc/managers.c -@@ -21,15 +21,24 @@ - isc_result_t - isc_managers_create(isc_mem_t *mctx, size_t workers, size_t quantum, - isc_nm_t **netmgrp, isc_taskmgr_t **taskmgrp) { -+ return isc_managers_create2(mctx, workers, quantum, 0, netmgrp, -+ taskmgrp); -+} -+ -+isc_result_t -+isc_managers_create2(isc_mem_t *mctx, size_t workers, size_t quantum, -+ size_t hp_extra, isc_nm_t **netmgrp, -+ isc_taskmgr_t **taskmgrp) { - isc_result_t result; - isc_taskmgr_t *taskmgr = NULL; - isc_nm_t *netmgr = NULL; -+ size_t hp_workers = 4 * (workers + hp_extra); - - /* - * We have ncpus network threads, ncpus old network threads - make - * it 4x just to be on the safe side. - */ -- isc_hp_init(4 * workers); -+ isc_hp_init(hp_workers); - - REQUIRE(netmgrp != NULL && *netmgrp == NULL); - isc__netmgr_create(mctx, workers, &netmgr); --- -2.34.1 - diff --git a/bind-9.16-locked-isc-queue.patch b/bind-9.16-locked-isc-queue.patch new file mode 100644 index 0000000..816d960 --- /dev/null +++ b/bind-9.16-locked-isc-queue.patch @@ -0,0 +1,214 @@ +From 0df59049fe13ef89d362fa7f109f289b297441dc Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= +Date: Tue, 22 Feb 2022 23:40:39 +0100 +Subject: [PATCH] Provide alternative isc_queue implementation based on locked + list + +The current implementation of isc_queue uses Michael-Scott lock-free +queue that in turn uses hazard pointers. It was discovered that the way +we use the isc_queue, such complicated mechanism isn't really needed, +because most of the time, we either execute the work directly when on +nmthread (in case of UDP) or schedule the work from the matching +nmthreads. + +Provide alternative implementation for the isc_queue based on locked +ISC_LIST. +PatchNumber: 11 +PatchNumber: 11 +--- + bin/named/main.c | 1 - + configure.ac | 12 ++++ + lib/isc/include/isc/queue.h | 3 +- + lib/isc/queue.c | 121 ++++++++++++++++++++++++++++++++++++ + 4 files changed, 134 insertions(+), 3 deletions(-) + +diff --git a/bin/named/main.c b/bin/named/main.c +index 9ad2d0e..8870933 100644 +--- a/bin/named/main.c ++++ b/bin/named/main.c +@@ -34,7 +34,6 @@ + #include + #include + #include +-#include + #include + #include + #include +diff --git a/configure.ac b/configure.ac +index 79d33d1..26241a0 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -2263,8 +2263,20 @@ AS_CASE([$with_cmocka], + AC_SUBST([CMOCKA_CFLAGS]) + AC_SUBST([CMOCKA_LIBS]) + ++# ++# Use lock-free Michael-Scott's queue implementation or locked-list queue ++# ++# [pairwise: --enable-lock-free-queue, --disable-lock-free-queue] ++AC_ARG_ENABLE([lock-free-queue], ++ [AS_HELP_STRING([--enable-lock-free-queue],[enable lock-free queue implementation (default is enabled)])], ++ [],[enable_lock_free_queue=yes]) ++AS_CASE([$enable_lock_free_queue], ++ [no],[], ++ [yes],[AC_DEFINE([USE_LOCK_FREE_QUEUE],[1],[Define to 1 to enable lock-free queue])]) ++ + AC_DEFINE([SKIPPED_TEST_EXIT_CODE], [0], [Exit code for skipped tests]) + ++ + # + # Check for kyua execution engine if CMocka was requested + # and bail out if execution engine was not found +diff --git a/lib/isc/include/isc/queue.h b/lib/isc/include/isc/queue.h +index 0927075..568bf18 100644 +--- a/lib/isc/include/isc/queue.h ++++ b/lib/isc/include/isc/queue.h +@@ -39,8 +39,7 @@ uintptr_t + isc_queue_dequeue(isc_queue_t *queue); + /*%< + * Remove an object pointer from the head of the queue and return the +- * pointer. If the queue is empty, return `nulluintptr` (the uintptr_t +- * representation of NULL). ++ * pointer. If the queue is empty, return `NULL`. + * + * Requires: + * \li 'queue' is not null. +diff --git a/lib/isc/queue.c b/lib/isc/queue.c +index d7ea824..c4cb404 100644 +--- a/lib/isc/queue.c ++++ b/lib/isc/queue.c +@@ -28,6 +28,10 @@ + + static uintptr_t nulluintptr = (uintptr_t)NULL; + ++#if USE_LOCK_FREE_QUEUE ++ ++#define BUFFER_SIZE 1024 ++ + typedef struct node { + atomic_uint_fast32_t deqidx; + atomic_uintptr_t items[BUFFER_SIZE]; +@@ -232,3 +236,120 @@ isc_queue_destroy(isc_queue_t *queue) { + alloced = queue->alloced_ptr; + isc_mem_putanddetach(&queue->mctx, alloced, sizeof(*queue) + ALIGNMENT); + } ++ ++#else /* USE_LOCK_FREE_QUEUE */ ++ ++typedef struct node node_t; ++ ++struct node { ++ uintptr_t item; ++ ISC_LINK(node_t) link; ++}; ++ ++struct isc_queue { ++ isc_mem_t *mctx; ++ isc_mutex_t lock; ++ int max_threads; ++ ISC_LIST(node_t) nodes; ++ void *alloced_ptr; ++}; ++ ++static node_t * ++node_new(isc_mem_t *mctx, uintptr_t item) { ++ node_t *node = isc_mem_get(mctx, sizeof(*node)); ++ *node = (node_t){ ++ .item = item, ++ }; ++ ++ ISC_LINK_INIT(node, link); ++ ++ return (node); ++} ++ ++static void ++node_destroy(isc_mem_t *mctx, node_t *node) { ++ isc_mem_put(mctx, node, sizeof(*node)); ++} ++ ++isc_queue_t * ++isc_queue_new(isc_mem_t *mctx, int max_threads) { ++ isc_queue_t *queue = NULL; ++ void *qbuf = NULL; ++ uintptr_t qptr; ++ ++ qbuf = isc_mem_get(mctx, sizeof(*queue) + ALIGNMENT); ++ qptr = (uintptr_t)qbuf; ++ queue = (isc_queue_t *)(qptr + (ALIGNMENT - (qptr % ALIGNMENT))); ++ ++ if (max_threads == 0) { ++ max_threads = MAX_THREADS; ++ } ++ ++ *queue = (isc_queue_t){ ++ .max_threads = max_threads, ++ .alloced_ptr = qbuf, ++ }; ++ ++ ISC_LIST_INIT(queue->nodes); ++ ++ isc_mutex_init(&queue->lock); ++ isc_mem_attach(mctx, &queue->mctx); ++ ++ return (queue); ++} ++ ++void ++isc_queue_enqueue(isc_queue_t *queue, uintptr_t item) { ++ node_t *node = node_new(queue->mctx, item); ++ REQUIRE(item != nulluintptr); ++ ++ LOCK(&queue->lock); ++ ISC_LIST_ENQUEUE(queue->nodes, node, link); ++ UNLOCK(&queue->lock); ++} ++ ++uintptr_t ++isc_queue_dequeue(isc_queue_t *queue) { ++ node_t *node = NULL; ++ uintptr_t item = nulluintptr; ++ REQUIRE(queue != NULL); ++ ++ LOCK(&queue->lock); ++ node = ISC_LIST_HEAD(queue->nodes); ++ if (node != NULL) { ++ ISC_LIST_DEQUEUE(queue->nodes, node, link); ++ item = node->item; ++ } ++ UNLOCK(&queue->lock); ++ ++ if (node != NULL) { ++ node_destroy(queue->mctx, node); ++ } ++ ++ return (item); ++} ++ ++void ++isc_queue_destroy(isc_queue_t *queue) { ++ node_t *node = NULL; ++ void *alloced = NULL; ++ ++ REQUIRE(queue != NULL); ++ ++ LOCK(&queue->lock); ++ node = ISC_LIST_HEAD(queue->nodes); ++ while (node != NULL) { ++ node_t *next = ISC_LIST_NEXT(node, link); ++ ISC_LIST_DEQUEUE(queue->nodes, node, link); ++ node_destroy(queue->mctx, node); ++ node = next; ++ } ++ UNLOCK(&queue->lock); ++ ++ isc_mutex_destroy(&queue->lock); ++ ++ alloced = queue->alloced_ptr; ++ isc_mem_putanddetach(&queue->mctx, alloced, sizeof(*queue) + ALIGNMENT); ++} ++ ++#endif /* USE_LOCK_FREE_QUEUE */ +-- +2.34.1 + diff --git a/bind.spec b/bind.spec index 105cc5d..ccfbe36 100644 --- a/bind.spec +++ b/bind.spec @@ -53,7 +53,7 @@ Summary: The Berkeley Internet Name Domain (BIND) DNS (Domain Name System) serv Name: bind License: MPLv2.0 Version: 9.16.26 -Release: 1%{?dist} +Release: 2%{?dist} Epoch: 32 Url: https://www.isc.org/downloads/bind/ # @@ -103,9 +103,8 @@ Patch24: bind-9.9.1-P2-dlz-libdb.patch # https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/2689 Patch25:bind-9.11-rh1666814.patch -# https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/5795 -Patch26:bind-9.16-hp-workers-rh2048235.patch -Patch27:bind-9.16-hp-workers-rh2048235-dyndb.patch +# https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/5905 +Patch26: bind-9.16-locked-isc-queue.patch %{?systemd_ordering} Requires: coreutils @@ -465,6 +464,7 @@ export LIBDIR_SUFFIX --includedir=%{_includedir}/bind9 \ --with-tuning=large \ --with-libidn2 \ + --disable-lock-free-queue \ %if %{with GEOIP2} --with-maxminddb \ %endif @@ -1115,6 +1115,9 @@ fi; %endif %changelog +* Tue Mar 01 2022 Petr Menšík - 32:9.16.26-2 +- Switch to locked queue (#2048235) + * Thu Feb 17 2022 Petr Menšík - 32:9.16.26-1 - Update to 9.16.26 (#2055120)