From 64e1a4a398736f7ab8a40414eb5c43005f8e142e Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 5 Nov 2019 13:18:37 -0800 Subject: [PATCH] temporarily move ISC_QUEUE to list.h The double-locked queue implementation is still currently in use in ns_client, but will be replaced by a fetch-and-add array queue. This commit moves it from queue.h to list.h so that queue.h can be used for the new data structure, and clean up dependencies between list.h and types.h. Later, when the ISC_QUEUE is no longer is use, it will be removed completely. --- lib/dns/include/dns/fixedname.h | 1 + lib/isc/app.c | 1 + lib/isc/include/isc/buffer.h | 1 + lib/isc/include/isc/event.h | 1 + lib/isc/include/isc/list.h | 146 +++++++++++++++++++++++++++++++- lib/isc/include/isc/queue.h | 146 -------------------------------- lib/isc/include/isc/result.h | 1 - lib/isc/include/isc/sockaddr.h | 1 + lib/isc/include/isc/types.h | 15 ++-- lib/isc/include/pk11/result.h | 1 + lib/isc/tests/queue_test.c | 2 +- lib/ns/client.c | 2 +- lib/ns/include/ns/client.h | 2 +- lib/ns/notify.c | 1 + 14 files changed, 162 insertions(+), 159 deletions(-) diff --git a/lib/dns/include/dns/fixedname.h b/lib/dns/include/dns/fixedname.h index 6a9b555105..623313439c 100644 --- a/lib/dns/include/dns/fixedname.h +++ b/lib/dns/include/dns/fixedname.h @@ -51,6 +51,7 @@ #include #include +#include #include diff --git a/lib/isc/app.c b/lib/isc/app.c index 1a013f1f01..9f228278bb 100644 --- a/lib/isc/app.c +++ b/lib/isc/app.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include diff --git a/lib/isc/include/isc/buffer.h b/lib/isc/include/isc/buffer.h index 07d259956c..6dec61ef64 100644 --- a/lib/isc/include/isc/buffer.h +++ b/lib/isc/include/isc/buffer.h @@ -105,6 +105,7 @@ #include #include #include +#include #include #include #include diff --git a/lib/isc/include/isc/event.h b/lib/isc/include/isc/event.h index 99d867cd5a..3e9fe19c8a 100644 --- a/lib/isc/include/isc/event.h +++ b/lib/isc/include/isc/event.h @@ -16,6 +16,7 @@ /*! \file isc/event.h */ #include +#include #include /***** diff --git a/lib/isc/include/isc/list.h b/lib/isc/include/isc/list.h index bc90bb974e..b7b245c1bd 100644 --- a/lib/isc/include/isc/list.h +++ b/lib/isc/include/isc/list.h @@ -9,11 +9,14 @@ * information regarding copyright ownership. */ - #ifndef ISC_LIST_H #define ISC_LIST_H 1 +#include + #include +#include +#include #ifdef ISC_LIST_CHECKINIT #define ISC_LINK_INSIST(x) ISC_INSIST(x) @@ -21,7 +24,6 @@ #define ISC_LINK_INSIST(x) #endif -#define ISC_LIST(type) struct { type *head, *tail; } #define ISC_LIST_INIT(list) \ do { (list).head = NULL; (list).tail = NULL; } while (0) @@ -189,4 +191,144 @@ #define __ISC_LIST_DEQUEUEUNSAFE_TYPE(list, elt, link, type) \ __ISC_LIST_UNLINKUNSAFE_TYPE(list, elt, link, type) +/* + * This is a generic implementation of a two-lock concurrent queue. + * There are built-in mutex locks for the head and tail of the queue, + * allowing elements to be safely added and removed at the same time. + * + * NULL is "end of list" + * -1 is "not linked" + */ + +#ifdef ISC_QUEUE_CHECKINIT +#define ISC_QLINK_INSIST(x) ISC_INSIST(x) +#else +#define ISC_QLINK_INSIST(x) (void)0 +#endif + +#define ISC_QLINK(type) struct { type *prev, *next; } + +#define ISC_QLINK_INIT(elt, link) \ + do { \ + (elt)->link.next = (elt)->link.prev = (void *)(-1); \ + } while(0) + +#define ISC_QLINK_LINKED(elt, link) ((void*)(elt)->link.next != (void*)(-1)) + +#define ISC_QUEUE(type) struct { \ + type *head, *tail; \ + isc_mutex_t headlock, taillock; \ +} + +#define ISC_QUEUE_INIT(queue, link) \ + do { \ + isc_mutex_init(&(queue).taillock); \ + isc_mutex_init(&(queue).headlock); \ + (queue).tail = (queue).head = NULL; \ + } while (0) + +#define ISC_QUEUE_EMPTY(queue) ((queue).head == NULL) + +#define ISC_QUEUE_DESTROY(queue) \ + do { \ + ISC_QLINK_INSIST(ISC_QUEUE_EMPTY(queue)); \ + isc_mutex_destroy(&(queue).taillock); \ + isc_mutex_destroy(&(queue).headlock); \ + } while (0) + +/* + * queues are meant to separate the locks at either end. For best effect, that + * means keeping the ends separate - i.e. non-empty queues work best. + * + * a push to an empty queue has to take the pop lock to update + * the pop side of the queue. + * Popping the last entry has to take the push lock to update + * the push side of the queue. + * + * The order is (pop, push), because a pop is presumably in the + * latency path and a push is when we're done. + * + * We do an MT hot test in push to see if we need both locks, so we can + * acquire them in order. Hopefully that makes the case where we get + * the push lock and find we need the pop lock (and have to release it) rare. + * + * > 1 entry - no collision, push works on one end, pop on the other + * 0 entry - headlock race + * pop wins - return(NULL), push adds new as both head/tail + * push wins - updates head/tail, becomes 1 entry case. + * 1 entry - taillock race + * pop wins - return(pop) sets head/tail NULL, becomes 0 entry case + * push wins - updates {head,tail}->link.next, pop updates head + * with new ->link.next and doesn't update tail + * + */ +#define ISC_QUEUE_PUSH(queue, elt, link) \ + do { \ + bool headlocked = false; \ + ISC_QLINK_INSIST(!ISC_QLINK_LINKED(elt, link)); \ + if ((queue).head == NULL) { \ + LOCK(&(queue).headlock); \ + headlocked = true; \ + } \ + LOCK(&(queue).taillock); \ + if ((queue).tail == NULL && !headlocked) { \ + UNLOCK(&(queue).taillock); \ + LOCK(&(queue).headlock); \ + LOCK(&(queue).taillock); \ + headlocked = true; \ + } \ + (elt)->link.prev = (queue).tail; \ + (elt)->link.next = NULL; \ + if ((queue).tail != NULL) \ + (queue).tail->link.next = (elt); \ + (queue).tail = (elt); \ + UNLOCK(&(queue).taillock); \ + if (headlocked) { \ + if ((queue).head == NULL) \ + (queue).head = (elt); \ + UNLOCK(&(queue).headlock); \ + } \ + } while (0) + +#define ISC_QUEUE_POP(queue, link, ret) \ + do { \ + LOCK(&(queue).headlock); \ + ret = (queue).head; \ + while (ret != NULL) { \ + if (ret->link.next == NULL) { \ + LOCK(&(queue).taillock); \ + if (ret->link.next == NULL) { \ + (queue).head = (queue).tail = NULL; \ + UNLOCK(&(queue).taillock); \ + break; \ + }\ + UNLOCK(&(queue).taillock); \ + } \ + (queue).head = ret->link.next; \ + (queue).head->link.prev = NULL; \ + break; \ + } \ + UNLOCK(&(queue).headlock); \ + if (ret != NULL) \ + (ret)->link.next = (ret)->link.prev = (void *)(-1); \ + } while(0) + +#define ISC_QUEUE_UNLINK(queue, elt, link) \ + do { \ + ISC_QLINK_INSIST(ISC_QLINK_LINKED(elt, link)); \ + LOCK(&(queue).headlock); \ + LOCK(&(queue).taillock); \ + if ((elt)->link.prev == NULL) \ + (queue).head = (elt)->link.next; \ + else \ + (elt)->link.prev->link.next = (elt)->link.next; \ + if ((elt)->link.next == NULL) \ + (queue).tail = (elt)->link.prev; \ + else \ + (elt)->link.next->link.prev = (elt)->link.prev; \ + UNLOCK(&(queue).taillock); \ + UNLOCK(&(queue).headlock); \ + (elt)->link.next = (elt)->link.prev = (void *)(-1); \ + } while(0) + #endif /* ISC_LIST_H */ diff --git a/lib/isc/include/isc/queue.h b/lib/isc/include/isc/queue.h index 43080a8134..a23f7c5da0 100644 --- a/lib/isc/include/isc/queue.h +++ b/lib/isc/include/isc/queue.h @@ -9,153 +9,7 @@ * information regarding copyright ownership. */ - -/* - * This is a generic implementation of a two-lock concurrent queue. - * There are built-in mutex locks for the head and tail of the queue, - * allowing elements to be safely added and removed at the same time. - * - * NULL is "end of list" - * -1 is "not linked" - */ - #ifndef ISC_QUEUE_H #define ISC_QUEUE_H 1 -#include - -#include -#include - -#ifdef ISC_QUEUE_CHECKINIT -#define ISC_QLINK_INSIST(x) ISC_INSIST(x) -#else -#define ISC_QLINK_INSIST(x) (void)0 -#endif - -#define ISC_QLINK(type) struct { type *prev, *next; } - -#define ISC_QLINK_INIT(elt, link) \ - do { \ - (elt)->link.next = (elt)->link.prev = (void *)(-1); \ - } while(0) - -#define ISC_QLINK_LINKED(elt, link) ((void*)(elt)->link.next != (void*)(-1)) - -#define ISC_QUEUE(type) struct { \ - type *head, *tail; \ - isc_mutex_t headlock, taillock; \ -} - -#define ISC_QUEUE_INIT(queue, link) \ - do { \ - isc_mutex_init(&(queue).taillock); \ - isc_mutex_init(&(queue).headlock); \ - (queue).tail = (queue).head = NULL; \ - } while (0) - -#define ISC_QUEUE_EMPTY(queue) ((queue).head == NULL) - -#define ISC_QUEUE_DESTROY(queue) \ - do { \ - ISC_QLINK_INSIST(ISC_QUEUE_EMPTY(queue)); \ - isc_mutex_destroy(&(queue).taillock); \ - isc_mutex_destroy(&(queue).headlock); \ - } while (0) - -/* - * queues are meant to separate the locks at either end. For best effect, that - * means keeping the ends separate - i.e. non-empty queues work best. - * - * a push to an empty queue has to take the pop lock to update - * the pop side of the queue. - * Popping the last entry has to take the push lock to update - * the push side of the queue. - * - * The order is (pop, push), because a pop is presumably in the - * latency path and a push is when we're done. - * - * We do an MT hot test in push to see if we need both locks, so we can - * acquire them in order. Hopefully that makes the case where we get - * the push lock and find we need the pop lock (and have to release it) rare. - * - * > 1 entry - no collision, push works on one end, pop on the other - * 0 entry - headlock race - * pop wins - return(NULL), push adds new as both head/tail - * push wins - updates head/tail, becomes 1 entry case. - * 1 entry - taillock race - * pop wins - return(pop) sets head/tail NULL, becomes 0 entry case - * push wins - updates {head,tail}->link.next, pop updates head - * with new ->link.next and doesn't update tail - * - */ -#define ISC_QUEUE_PUSH(queue, elt, link) \ - do { \ - bool headlocked = false; \ - ISC_QLINK_INSIST(!ISC_QLINK_LINKED(elt, link)); \ - if ((queue).head == NULL) { \ - LOCK(&(queue).headlock); \ - headlocked = true; \ - } \ - LOCK(&(queue).taillock); \ - if ((queue).tail == NULL && !headlocked) { \ - UNLOCK(&(queue).taillock); \ - LOCK(&(queue).headlock); \ - LOCK(&(queue).taillock); \ - headlocked = true; \ - } \ - (elt)->link.prev = (queue).tail; \ - (elt)->link.next = NULL; \ - if ((queue).tail != NULL) \ - (queue).tail->link.next = (elt); \ - (queue).tail = (elt); \ - UNLOCK(&(queue).taillock); \ - if (headlocked) { \ - if ((queue).head == NULL) \ - (queue).head = (elt); \ - UNLOCK(&(queue).headlock); \ - } \ - } while (0) - -#define ISC_QUEUE_POP(queue, link, ret) \ - do { \ - LOCK(&(queue).headlock); \ - ret = (queue).head; \ - while (ret != NULL) { \ - if (ret->link.next == NULL) { \ - LOCK(&(queue).taillock); \ - if (ret->link.next == NULL) { \ - (queue).head = (queue).tail = NULL; \ - UNLOCK(&(queue).taillock); \ - break; \ - }\ - UNLOCK(&(queue).taillock); \ - } \ - (queue).head = ret->link.next; \ - (queue).head->link.prev = NULL; \ - break; \ - } \ - UNLOCK(&(queue).headlock); \ - if (ret != NULL) \ - (ret)->link.next = (ret)->link.prev = (void *)(-1); \ - } while(0) - -#define ISC_QUEUE_UNLINK(queue, elt, link) \ - do { \ - ISC_QLINK_INSIST(ISC_QLINK_LINKED(elt, link)); \ - LOCK(&(queue).headlock); \ - LOCK(&(queue).taillock); \ - if ((elt)->link.prev == NULL) \ - (queue).head = (elt)->link.next; \ - else \ - (elt)->link.prev->link.next = (elt)->link.next; \ - if ((elt)->link.next == NULL) \ - (queue).tail = (elt)->link.prev; \ - else \ - (elt)->link.next->link.prev = (elt)->link.prev; \ - UNLOCK(&(queue).taillock); \ - UNLOCK(&(queue).headlock); \ - (elt)->link.next = (elt)->link.prev = (void *)(-1); \ - } while(0) - #endif /* ISC_QUEUE_H */ diff --git a/lib/isc/include/isc/result.h b/lib/isc/include/isc/result.h index 96f5f2bdff..a3fbeb047a 100644 --- a/lib/isc/include/isc/result.h +++ b/lib/isc/include/isc/result.h @@ -9,7 +9,6 @@ * information regarding copyright ownership. */ - #ifndef ISC_RESULT_H #define ISC_RESULT_H 1 diff --git a/lib/isc/include/isc/sockaddr.h b/lib/isc/include/isc/sockaddr.h index 1588ba9645..a1d5080881 100644 --- a/lib/isc/include/isc/sockaddr.h +++ b/lib/isc/include/isc/sockaddr.h @@ -18,6 +18,7 @@ #include #include +#include #include #include #ifdef ISC_PLATFORM_HAVESYSUNH diff --git a/lib/isc/include/isc/types.h b/lib/isc/include/isc/types.h index 8b04019877..6520d23711 100644 --- a/lib/isc/include/isc/types.h +++ b/lib/isc/include/isc/types.h @@ -20,16 +20,17 @@ */ #include #include + +#include #include -/* - * XXXDCL This is just for ISC_LIST and ISC_LINK, but gets all of the other - * list macros too. - */ -#include - /* Core Types. Alphabetized by defined type. */ +/* + * Defined here so we don't need to include list.h. + */ +#define ISC_LIST(type) struct { type *head, *tail; } + typedef struct isc_appctx isc_appctx_t; /*%< Application context */ typedef struct isc_backtrace_symmap isc_backtrace_symmap_t; /*%< Symbol Table Entry */ typedef struct isc_buffer isc_buffer_t; /*%< Buffer */ @@ -44,7 +45,7 @@ typedef unsigned int isc_eventtype_t; /*%< Event Type */ typedef uint32_t isc_fsaccess_t; /*%< FS Access */ typedef struct isc_hash isc_hash_t; /*%< Hash */ typedef struct isc_hp isc_hp_t; /*%< Hazard - pointer */ + pointer */ typedef struct isc_httpd isc_httpd_t; /*%< HTTP client */ typedef void (isc_httpdfree_t)(isc_buffer_t *, void *); /*%< HTTP free function */ typedef struct isc_httpdmgr isc_httpdmgr_t; /*%< HTTP manager */ diff --git a/lib/isc/include/pk11/result.h b/lib/isc/include/pk11/result.h index 21f4a37529..e5f8102dba 100644 --- a/lib/isc/include/pk11/result.h +++ b/lib/isc/include/pk11/result.h @@ -16,6 +16,7 @@ #include #include +#include /* * Nothing in this file truly depends on , but the diff --git a/lib/isc/tests/queue_test.c b/lib/isc/tests/queue_test.c index 89ed2d4e23..73025776cd 100644 --- a/lib/isc/tests/queue_test.c +++ b/lib/isc/tests/queue_test.c @@ -24,7 +24,7 @@ #define UNIT_TESTING #include -#include +#include #include #include "isctest.h" diff --git a/lib/ns/client.c b/lib/ns/client.c index 598e41179e..ceccec6e0e 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -16,12 +16,12 @@ #include #include #include +#include #include #include #include #include #include -#include #include #include #include diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h index 02531f2118..c75eafb655 100644 --- a/lib/ns/include/ns/client.h +++ b/lib/ns/include/ns/client.h @@ -58,10 +58,10 @@ #include #include +#include #include #include #include -#include #include #include diff --git a/lib/ns/notify.c b/lib/ns/notify.c index 0eee750ae3..714819d69b 100644 --- a/lib/ns/notify.c +++ b/lib/ns/notify.c @@ -9,6 +9,7 @@ * information regarding copyright ownership. */ +#include #include #include