- Make isc_quota use atomics instead of locks

- Use getters for isc_quota parameters, make fields private
- Fix a potential data race with recursion clients limits logging

diff --git a/bin/named/server.c b/bin/named/server.c
index ce048544d3b480883e6b4c03bc6b6cb9b51c2299..1bd098a22811131cff9a7f431ded3f2908f28d11 100644 (file)
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -8022,6 +8022,7 @@ load_configuration(const char *filename, named_server_t *server,
        ns_altsecretlist_t altsecrets, tmpaltsecrets;
        unsigned int maxsocks;
        uint32_t softquota = 0;
+       uint32_t max;
        unsigned int initial, idle, keepalive, advertised;
        dns_aclenv_t *env =
                ns_interfacemgr_getaclenv(named_g_server->interfacemgr);
@@ -8214,20 +8215,20 @@ load_configuration(const char *filename, named_server_t *server,
        configure_server_quota(maps, "recursive-clients",
                               &server->sctx->recursionquota);
 
-       if (server->sctx->recursionquota.max > 1000) {
-               int margin = ISC_MAX(100, named_g_cpus + 1);
-               if (margin > server->sctx->recursionquota.max - 100) {
+       max = isc_quota_getmax(&server->sctx->recursionquota);
+       if (max > 1000) {
+               unsigned margin = ISC_MAX(100, named_g_cpus + 1);
+               if (margin + 100 > max) {
                        isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
                                      NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR,
                                      "'recursive-clients %d' too low when "
                                      "running with %d worker threads",
-                                     server->sctx->recursionquota.max,
-                                     named_g_cpus);
+                                     max, named_g_cpus);
                        CHECK(ISC_R_RANGE);
                }
-               softquota = server->sctx->recursionquota.max - margin;
+               softquota = max - margin;
        } else {
-               softquota = (server->sctx->recursionquota.max * 90) / 100;
+               softquota = (max * 90) / 100;
        }
 
        isc_quota_soft(&server->sctx->recursionquota, softquota);
@@ -11452,13 +11453,14 @@ named_server_status(named_server_t *server, isc_buffer_t **text) {
        CHECK(putstr(text, line));
 
        snprintf(line, sizeof(line), "recursive clients: %d/%d/%d\n",
-                    server->sctx->recursionquota.used,
-                    server->sctx->recursionquota.soft,
-                    server->sctx->recursionquota.max);
+                    isc_quota_getused(&server->sctx->recursionquota),
+                    isc_quota_getsoft(&server->sctx->recursionquota),
+                    isc_quota_getmax(&server->sctx->recursionquota));
        CHECK(putstr(text, line));
 
        snprintf(line, sizeof(line), "tcp clients: %d/%d\n",
-                    server->sctx->tcpquota.used, server->sctx->tcpquota.max);
+                    isc_quota_getused(&server->sctx->tcpquota),
+                    isc_quota_getmax(&server->sctx->tcpquota));
        CHECK(putstr(text, line));
 
        if (server->reload_in_progress) {

diff --git a/lib/isc/include/isc/quota.h b/lib/isc/include/isc/quota.h
index e61e0e88343d0148be9474b65efa9110eff7abb6..4addbb8b2383cc3ec5b141eb78d72d1cecd7ab9f 100644 (file)
--- a/lib/isc/include/isc/quota.h
+++ b/lib/isc/include/isc/quota.h
@@ -30,6 +30,7 @@
  *** Imports.
  ***/
 
+#include <isc/atomic.h>
 #include <isc/lang.h>
 #include <isc/mutex.h>
 #include <isc/types.h>
@@ -42,14 +43,14 @@ ISC_LANG_BEGINDECLS
 
 /*% isc_quota structure */
 struct isc_quota {
-       isc_mutex_t     lock; /*%< Locked by lock. */
-       int             max;
-       int             used;
-       int             soft;
+       atomic_uint_fast32_t            __max;
+       atomic_uint_fast32_t            __used;
+       atomic_uint_fast32_t            __soft;
 };
 
+
 void
-isc_quota_init(isc_quota_t *quota, int max);
+isc_quota_init(isc_quota_t *quota, unsigned int max);
 /*%<
  * Initialize a quota object.
  */
@@ -61,17 +62,35 @@ isc_quota_destroy(isc_quota_t *quota);
  */
 
 void
-isc_quota_soft(isc_quota_t *quota, int soft);
+isc_quota_soft(isc_quota_t *quota, unsigned int soft);
 /*%<
  * Set a soft quota.
  */
 
 void
-isc_quota_max(isc_quota_t *quota, int max);
+isc_quota_max(isc_quota_t *quota, unsigned int max);
 /*%<
  * Re-set a maximum quota.
  */
 
+unsigned int
+isc_quota_getmax(isc_quota_t *quota);
+/*%<
+ * Get the maximum quota.
+ */
+
+unsigned int
+isc_quota_getsoft(isc_quota_t *quota);
+/*%<
+ * Get the soft quota.
+ */
+
+unsigned int
+isc_quota_getused(isc_quota_t *quota);
+/*%<
+ * Get the current usage of quota.
+ */
+
 isc_result_t
 isc_quota_reserve(isc_quota_t *quota);
 /*%<

diff --git a/lib/isc/quota.c b/lib/isc/quota.c
index 803bd4594ce365e7926625e865fb556abc8dc981..edc9899ecb362172b0b245b297933e17390d27aa 100644 (file)
--- a/lib/isc/quota.c
+++ b/lib/isc/quota.c
@@ -16,62 +16,73 @@
 
 #include <stddef.h>
 
+#include <isc/atomic.h>
 #include <isc/quota.h>
 #include <isc/util.h>
 
+
 void
-isc_quota_init(isc_quota_t *quota, int max) {
-       quota->max = max;
-       quota->used = 0;
-       quota->soft = 0;
-       isc_mutex_init(&quota->lock);
+isc_quota_init(isc_quota_t *quota, unsigned int max) {
+       atomic_store(&quota->__max, max);
+       atomic_store(&quota->__used, 0);
+       atomic_store(&quota->__soft, 0);
 }
 
 void
 isc_quota_destroy(isc_quota_t *quota) {
-       INSIST(quota->used == 0);
-       quota->max = 0;
-       quota->used = 0;
-       quota->soft = 0;
-       isc_mutex_destroy(&quota->lock);
+       INSIST(atomic_load(&quota->__used) == 0);
+       atomic_store(&quota->__max, 0);
+       atomic_store(&quota->__used, 0);
+       atomic_store(&quota->__soft, 0);
 }
 
 void
-isc_quota_soft(isc_quota_t *quota, int soft) {
-       LOCK(&quota->lock);
-       quota->soft = soft;
-       UNLOCK(&quota->lock);
+isc_quota_soft(isc_quota_t *quota, unsigned int soft) {
+       atomic_store(&quota->__soft, soft);
 }
 
 void
-isc_quota_max(isc_quota_t *quota, int max) {
-       LOCK(&quota->lock);
-       quota->max = max;
-       UNLOCK(&quota->lock);
+isc_quota_max(isc_quota_t *quota, unsigned int max) {
+       atomic_store(&quota->__max, max);
+}
+
+unsigned int
+isc_quota_getmax(isc_quota_t *quota) {
+       return (atomic_load_relaxed(&quota->__max));
+}
+
+unsigned int
+isc_quota_getsoft(isc_quota_t *quota) {
+       return (atomic_load_relaxed(&quota->__soft));
+}
+
+unsigned int
+isc_quota_getused(isc_quota_t *quota) {
+       return (atomic_load_relaxed(&quota->__used));
 }
 
 isc_result_t
 isc_quota_reserve(isc_quota_t *quota) {
        isc_result_t result;
-       LOCK(&quota->lock);
-       if (quota->max == 0 || quota->used < quota->max) {
-               if (quota->soft == 0 || quota->used < quota->soft)
+       uint32_t max = atomic_load(&quota->__max);
+       uint32_t soft = atomic_load(&quota->__soft);
+       uint32_t used = atomic_fetch_add(&quota->__used, 1);
+       if (max == 0 || used < max) {
+               if (soft == 0 || used < soft) {
                        result = ISC_R_SUCCESS;
-               else
+               } else {
                        result = ISC_R_SOFTQUOTA;
-               quota->used++;
-       } else
+               }
+       } else {
+               INSIST(atomic_fetch_sub(&quota->__used, 1) > 0);
                result = ISC_R_QUOTA;
-       UNLOCK(&quota->lock);
+       }
        return (result);
 }
 
 void
 isc_quota_release(isc_quota_t *quota) {
-       LOCK(&quota->lock);
-       INSIST(quota->used > 0);
-       quota->used--;
-       UNLOCK(&quota->lock);
+       INSIST(atomic_fetch_sub(&quota->__used, 1) > 0);
 }
 
 isc_result_t

diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in
index 5f78edae652da53ab4cef53e14221991d710d372..4cec81fca1574d83af88469c7617616b328588f5 100644 (file)
--- a/lib/isc/win32/libisc.def.in
+++ b/lib/isc/win32/libisc.def.in
@@ -449,6 +449,9 @@ isc_portset_removerange
 isc_quota_attach
 isc_quota_destroy
 isc_quota_detach
+isc_quota_getmax
+isc_quota_getsoft
+isc_quota_getused
 isc_quota_init
 isc_quota_max
 isc_quota_release

diff --git a/lib/ns/query.c b/lib/ns/query.c
index 1d02bd49c817a4a40807a708231d1a5072524648..0798e2c817b9f2cc60123c8a3fbac4a343623367 100644 (file)
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -5589,38 +5589,37 @@ ns_query_recurse(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qname,
                                   ns_statscounter_recursclients);
 
                if  (result == ISC_R_SOFTQUOTA) {
-                       static isc_stdtime_t last = 0;
+                       static atomic_uint_fast32_t last = 0;
                        isc_stdtime_t now;
                        isc_stdtime_get(&now);
-                       if (now != last) {
-                               last = now;
+                       if (now != atomic_load_relaxed(&last)) {
+                               atomic_store_relaxed(&last, now);
                                ns_client_log(client, NS_LOGCATEGORY_CLIENT,
-                                             NS_LOGMODULE_QUERY,
-                                             ISC_LOG_WARNING,
-                                             "recursive-clients soft limit "
-                                             "exceeded (%d/%d/%d), "
-                                             "aborting oldest query",
-                                             client->recursionquota->used,
-                                             client->recursionquota->soft,
-                                             client->recursionquota->max);
+                                     NS_LOGMODULE_QUERY, ISC_LOG_WARNING,
+                                     "recursive-clients soft limit "
+                                     "exceeded (%u/%u/%u), "
+                                     "aborting oldest query",
+                                     isc_quota_getused(client->recursionquota),
+                                     isc_quota_getsoft(client->recursionquota),
+                                     isc_quota_getmax(client->recursionquota));
                        }
                        ns_client_killoldestquery(client);
                        result = ISC_R_SUCCESS;
                } else if (result == ISC_R_QUOTA) {
-                       static isc_stdtime_t last = 0;
+                       static atomic_uint_fast32_t last = 0;
                        isc_stdtime_t now;
                        isc_stdtime_get(&now);
-                       if (now != last) {
-                               last = now;
+                       if (now != atomic_load_relaxed(&last)) {
+                               ns_server_t *sctx = client->sctx;
+                               atomic_store_relaxed(&last, now);
                                ns_client_log(client, NS_LOGCATEGORY_CLIENT,
-                                             NS_LOGMODULE_QUERY,
-                                             ISC_LOG_WARNING,
-                                             "no more recursive clients "
-                                             "(%d/%d/%d): %s",
-                                             client->sctx->recursionquota.used,
-                                             client->sctx->recursionquota.soft,
-                                             client->sctx->recursionquota.max,
-                                             isc_result_totext(result));
+                                     NS_LOGMODULE_QUERY, ISC_LOG_WARNING,
+                                     "no more recursive clients "
+                                     "(%u/%u/%u): %s",
+                                     isc_quota_getused(&sctx->recursionquota),
+                                     isc_quota_getsoft(&sctx->recursionquota),
+                                     isc_quota_getmax(&sctx->recursionquota),
+                                     isc_result_totext(result));
                        }
                        ns_client_killoldestquery(client);
                }