Use C-RW-WP lock in the dns_adb unit

Replace the isc_mutex in the dns_adb unit with isc_rwlock for better
performance.  Both ADB names and ADB entries hashtables and LRU are now
using isc_rwlock.

diff --git a/lib/dns/adb.c b/lib/dns/adb.c
index 146c5cecca143c1fda4a0d647af113fc0a648caa..6b32df2a0f360cd1efb88fcdd0549a133dde8a4a 100644 (file)
--- a/lib/dns/adb.c
+++ b/lib/dns/adb.c
@@ -33,6 +33,7 @@
 #include <isc/print.h>
 #include <isc/random.h>
 #include <isc/result.h>
+#include <isc/rwlock.h>
 #include <isc/stats.h>
 #include <isc/string.h>
 #include <isc/task.h>
@@ -118,12 +119,12 @@ struct dns_adb {
        dns_adbnamelist_t names_lru;
        isc_stdtime_t names_last_update;
        isc_hashmap_t *names;
-       isc_mutex_t names_lock;
+       isc_rwlock_t names_lock;
 
        dns_adbentrylist_t entries_lru;
        isc_stdtime_t entries_last_update;
        isc_hashmap_t *entries;
-       isc_mutex_t entries_lock;
+       isc_rwlock_t entries_lock;
 
        isc_stats_t *stats;
 
@@ -372,6 +373,8 @@ maybe_expire_name(dns_adbname_t *adbname, isc_stdtime_t now);
 static void
 expire_name(dns_adbname_t *adbname, isc_eventtype_t evtype);
 static bool
+entry_expired(dns_adbentry_t *adbentry, isc_stdtime_t now);
+static bool
 maybe_expire_entry(dns_adbentry_t *adbentry, isc_stdtime_t now);
 static void
 expire_entry(dns_adbentry_t *adbentry);
@@ -762,7 +765,7 @@ static void
 shutdown_names(dns_adb_t *adb) {
        dns_adbname_t *next = NULL;
 
-       LOCK(&adb->names_lock);
+       RWLOCK(&adb->names_lock, isc_rwlocktype_write);
        for (dns_adbname_t *name = ISC_LIST_HEAD(adb->names_lru); name != NULL;
             name = next)
        {
@@ -779,20 +782,20 @@ shutdown_names(dns_adb_t *adb) {
                UNLOCK(&name->lock);
                dns_adbname_detach(&name);
        }
-       UNLOCK(&adb->names_lock);
+       RWUNLOCK(&adb->names_lock, isc_rwlocktype_write);
 }
 
 static void
 shutdown_entries(dns_adb_t *adb) {
        dns_adbentry_t *next = NULL;
-       LOCK(&adb->entries_lock);
+       RWLOCK(&adb->entries_lock, isc_rwlocktype_write);
        for (dns_adbentry_t *adbentry = ISC_LIST_HEAD(adb->entries_lru);
             adbentry != NULL; adbentry = next)
        {
                next = ISC_LIST_NEXT(adbentry, link);
                expire_entry(adbentry);
        }
-       UNLOCK(&adb->entries_lock);
+       RWUNLOCK(&adb->entries_lock, isc_rwlocktype_write);
 }
 
 /*
@@ -1342,6 +1345,7 @@ get_attached_and_locked_name(dns_adb_t *adb, const dns_name_t *name,
        isc_time_t timenow;
        isc_stdtime_t last_update;
        adbnamekey_t key;
+       isc_rwlocktype_t locktype = isc_rwlocktype_read;
 
        isc_time_set(&timenow, now, 0);
 
@@ -1351,54 +1355,65 @@ get_attached_and_locked_name(dns_adb_t *adb, const dns_name_t *name,
 
        hashval = isc_hashmap_hash(adb->names, &key.key, key.size);
 
-       LOCK(&adb->names_lock);
+       RWLOCK(&adb->names_lock, locktype);
        last_update = adb->names_last_update;
-       if (last_update + ADB_STALE_MARGIN < now ||
+
+       if (last_update + ADB_STALE_MARGIN >= now ||
            atomic_load_relaxed(&adb->is_overmem))
        {
-               last_update = adb->names_last_update = now;
-
+               last_update = now;
+               UPGRADELOCK(&adb->names_lock, locktype);
                purge_stale_names(adb, now);
+               adb->names_last_update = last_update;
        }
 
        result = isc_hashmap_find(adb->names, &hashval, key.key, key.size,
                                  (void **)&adbname);
        switch (result) {
        case ISC_R_NOTFOUND:
+               UPGRADELOCK(&adb->names_lock, locktype);
+
                /* Allocate a new name and add it to the hash table. */
                adbname = new_adbname(adb, name, start_at_zone);
 
                result = isc_hashmap_add(adb->names, &hashval,
                                         &adbname->key.key, adbname->key.size,
                                         adbname);
+               if (result == ISC_R_EXISTS) {
+                       destroy_adbname(adbname);
+                       adbname = NULL;
+                       result = isc_hashmap_find(adb->names, &hashval, key.key,
+                                                 key.size, (void **)&adbname);
+                       ISC_LIST_UNLINK(adb->names_lru, adbname, link);
+               }
                INSIST(result == ISC_R_SUCCESS);
 
-               dns_adbname_ref(adbname);
-               LOCK(&adbname->lock); /* Must be unlocked by the caller */
-               adbname->last_used = now;
-
-               ISC_LIST_PREPEND(adb->names_lru, adbname, link);
                break;
        case ISC_R_SUCCESS:
-               dns_adbname_ref(adbname);
-               LOCK(&adbname->lock); /* Must be unlocked by the caller */
-               if (adbname->last_used + ADB_CACHE_MINIMUM <= last_update) {
-                       adbname->last_used = now;
-
+               if (locktype == isc_rwlocktype_write) {
                        ISC_LIST_UNLINK(adb->names_lru, adbname, link);
-                       ISC_LIST_PREPEND(adb->names_lru, adbname, link);
                }
                break;
        default:
                UNREACHABLE();
        }
 
+       dns_adbname_ref(adbname);
+
+       LOCK(&adbname->lock); /* Must be unlocked by the caller */
+       if (adbname->last_used + ADB_CACHE_MINIMUM <= last_update) {
+               adbname->last_used = now;
+       }
+       if (locktype == isc_rwlocktype_write) {
+               ISC_LIST_PREPEND(adb->names_lru, adbname, link);
+       }
+
        /*
         * The refcount is now 2 and the final detach will happen in
         * expire_name() - the unused adbname stored in the hashtable and lru
         * has always refcount == 1
         */
-       UNLOCK(&adb->names_lock);
+       RWUNLOCK(&adb->names_lock, locktype);
 
        return (adbname);
 }
@@ -1415,64 +1430,93 @@ get_attached_and_locked_entry(dns_adb_t *adb, isc_stdtime_t now,
        isc_stdtime_t last_update;
        uint32_t hashval = isc_hashmap_hash(
                adb->entries, (const unsigned char *)addr, sizeof(*addr));
+       isc_rwlocktype_t locktype = isc_rwlocktype_read;
 
        isc_time_set(&timenow, now, 0);
 
-       LOCK(&adb->entries_lock);
+       RWLOCK(&adb->entries_lock, locktype);
        last_update = adb->entries_last_update;
+
        if (now - last_update > ADB_STALE_MARGIN ||
            atomic_load_relaxed(&adb->is_overmem))
        {
-               last_update = adb->entries_last_update = now;
+               last_update = now;
+
+               UPGRADELOCK(&adb->entries_lock, locktype);
 
                purge_stale_entries(adb, now);
+               adb->entries_last_update = now;
        }
 
+again:
        result = isc_hashmap_find(adb->entries, &hashval,
                                  (const unsigned char *)addr, sizeof(*addr),
                                  (void **)&adbentry);
        switch (result) {
        case ISC_R_NOTFOUND: {
        create:
+               UPGRADELOCK(&adb->entries_lock, locktype);
+
                /* Allocate a new entry and add it to the hash table. */
                adbentry = new_adbentry(adb, addr);
 
                result = isc_hashmap_add(adb->entries, &hashval,
                                         &adbentry->sockaddr,
                                         sizeof(adbentry->sockaddr), adbentry);
-               INSIST(result == ISC_R_SUCCESS);
-
-               dns_adbentry_ref(adbentry);
-               LOCK(&adbentry->lock); /* Must be unlocked by the caller */
-               adbentry->last_used = now;
+               if (result == ISC_R_EXISTS) {
+                       dns_adbentry_detach(&adbentry);
 
-               ISC_LIST_PREPEND(adb->entries_lru, adbentry, link);
+                       result = isc_hashmap_find(adb->entries, &hashval,
+                                                 (const unsigned char *)addr,
+                                                 sizeof(*addr),
+                                                 (void **)&adbentry);
+                       ISC_LIST_UNLINK(adb->entries_lru, adbentry, link);
+               }
+               INSIST(result == ISC_R_SUCCESS);
                break;
        }
        case ISC_R_SUCCESS:
-               /*
-                * The dns_adbentry_ref() must stay here before trying to expire
-                * the ADB entry, so it is not destroyed under the lock.
-                */
-               dns_adbentry_ref(adbentry);
-               LOCK(&adbentry->lock); /* Must be unlocked by the caller */
+               if (locktype == isc_rwlocktype_write) {
+                       ISC_LIST_UNLINK(adb->entries_lru, adbentry, link);
+               }
+               break;
+       default:
+               UNREACHABLE();
+       }
+
+       /*
+        * The dns_adbentry_ref() must stay here before trying to expire
+        * the ADB entry, so it is not destroyed under the lock.
+        */
+       dns_adbentry_ref(adbentry);
+       LOCK(&adbentry->lock); /* Must be unlocked by the caller */
+       switch (locktype) {
+       case isc_rwlocktype_read:
+               if (entry_expired(adbentry, now)) {
+                       UNLOCK(&adbentry->lock);
+                       dns_adbentry_detach(&adbentry);
+                       goto again;
+               }
+               break;
+       case isc_rwlocktype_write:
                if (maybe_expire_entry(adbentry, now)) {
                        UNLOCK(&adbentry->lock);
                        dns_adbentry_detach(&adbentry);
                        goto create;
                }
-               if (adbentry->last_used + ADB_CACHE_MINIMUM <= last_update) {
-                       adbentry->last_used = now;
-
-                       ISC_LIST_UNLINK(adb->entries_lru, adbentry, link);
-                       ISC_LIST_PREPEND(adb->entries_lru, adbentry, link);
-               }
                break;
        default:
                UNREACHABLE();
        }
 
-       UNLOCK(&adb->entries_lock);
+       if (adbentry->last_used + ADB_CACHE_MINIMUM <= last_update) {
+               adbentry->last_used = now;
+       }
+       if (locktype == isc_rwlocktype_write) {
+               ISC_LIST_PREPEND(adb->entries_lru, adbentry, link);
+       }
+
+       RWUNLOCK(&adb->entries_lock, locktype);
 
        return (adbentry);
 }
@@ -1657,9 +1701,7 @@ expire_entry(dns_adbentry_t *adbentry) {
 }
 
 static bool
-maybe_expire_entry(dns_adbentry_t *adbentry, isc_stdtime_t now) {
-       REQUIRE(DNS_ADBENTRY_VALID(adbentry));
-
+entry_expired(dns_adbentry_t *adbentry, isc_stdtime_t now) {
        if (!ISC_LIST_EMPTY(adbentry->nhs)) {
                return (false);
        }
@@ -1668,11 +1710,21 @@ maybe_expire_entry(dns_adbentry_t *adbentry, isc_stdtime_t now) {
                return (false);
        }
 
-       expire_entry(adbentry);
-
        return (true);
 }
 
+static bool
+maybe_expire_entry(dns_adbentry_t *adbentry, isc_stdtime_t now) {
+       REQUIRE(DNS_ADBENTRY_VALID(adbentry));
+
+       if (entry_expired(adbentry, now)) {
+               expire_entry(adbentry);
+               return (true);
+       }
+
+       return (false);
+}
+
 /*%
  * Examine the tail entry of the LRU list to see if it expires or is stale
  * (unused for some period); if so, the name entry will be freed.  If the ADB
@@ -1756,7 +1808,7 @@ static void
 cleanup_names(dns_adb_t *adb, isc_stdtime_t now) {
        dns_adbname_t *next = NULL;
 
-       LOCK(&adb->names_lock);
+       RWLOCK(&adb->names_lock, isc_rwlocktype_write);
        for (dns_adbname_t *adbname = ISC_LIST_HEAD(adb->names_lru);
             adbname != NULL; adbname = next)
        {
@@ -1775,7 +1827,7 @@ cleanup_names(dns_adb_t *adb, isc_stdtime_t now) {
                UNLOCK(&adbname->lock);
                dns_adbname_detach(&adbname);
        }
-       UNLOCK(&adb->names_lock);
+       RWUNLOCK(&adb->names_lock, isc_rwlocktype_write);
 }
 
 /*%
@@ -1859,7 +1911,7 @@ static void
 cleanup_entries(dns_adb_t *adb, isc_stdtime_t now) {
        dns_adbentry_t *next = NULL;
 
-       LOCK(&adb->entries_lock);
+       RWLOCK(&adb->entries_lock, isc_rwlocktype_write);
        for (dns_adbentry_t *adbentry = ISC_LIST_HEAD(adb->entries_lru);
             adbentry != NULL; adbentry = next)
        {
@@ -1871,7 +1923,7 @@ cleanup_entries(dns_adb_t *adb, isc_stdtime_t now) {
                UNLOCK(&adbentry->lock);
                dns_adbentry_detach(&adbentry);
        }
-       UNLOCK(&adb->entries_lock);
+       RWUNLOCK(&adb->entries_lock, isc_rwlocktype_write);
 }
 
 static void
@@ -1880,18 +1932,18 @@ destroy(dns_adb_t *adb) {
 
        adb->magic = 0;
 
-       LOCK(&adb->names_lock);
+       RWLOCK(&adb->names_lock, isc_rwlocktype_write);
        INSIST(isc_hashmap_count(adb->names) == 0);
        isc_hashmap_destroy(&adb->names);
-       UNLOCK(&adb->names_lock);
-       isc_mutex_destroy(&adb->names_lock);
+       RWUNLOCK(&adb->names_lock, isc_rwlocktype_write);
+       isc_rwlock_destroy(&adb->names_lock);
 
-       LOCK(&adb->entries_lock);
+       RWLOCK(&adb->entries_lock, isc_rwlocktype_write);
        /* There are no unassociated entries */
        INSIST(isc_hashmap_count(adb->entries) == 0);
        isc_hashmap_destroy(&adb->entries);
-       UNLOCK(&adb->entries_lock);
-       isc_mutex_destroy(&adb->entries_lock);
+       RWUNLOCK(&adb->entries_lock, isc_rwlocktype_write);
+       isc_rwlock_destroy(&adb->entries_lock);
 
        isc_mem_destroy(&adb->hmctx);
 
@@ -1956,11 +2008,11 @@ dns_adb_create(isc_mem_t *mem, dns_view_t *view, isc_loopmgr_t *loopmgr,
 
        isc_hashmap_create(adb->hmctx, ADB_HASH_BITS,
                           ISC_HASHMAP_CASE_INSENSITIVE, &adb->names);
-       isc_mutex_init(&adb->names_lock);
+       isc_rwlock_init(&adb->names_lock);
 
        isc_hashmap_create(adb->hmctx, ADB_HASH_BITS,
                           ISC_HASHMAP_CASE_SENSITIVE, &adb->entries);
-       isc_mutex_init(&adb->entries_lock);
+       isc_rwlock_init(&adb->entries_lock);
 
        isc_mutex_init(&adb->lock);
 
@@ -2002,11 +2054,11 @@ free_tasks:
 
        isc_mutex_destroy(&adb->lock);
 
-       isc_mutex_destroy(&adb->entries_lock);
+       isc_rwlock_destroy(&adb->entries_lock);
        isc_hashmap_destroy(&adb->entries);
        INSIST(ISC_LIST_EMPTY(adb->entries_lru));
 
-       isc_mutex_destroy(&adb->names_lock);
+       isc_rwlock_destroy(&adb->names_lock);
        isc_hashmap_destroy(&adb->names);
        INSIST(ISC_LIST_EMPTY(adb->names_lru));
 
@@ -2507,7 +2559,7 @@ dump_adb(dns_adb_t *adb, FILE *f, bool debug, isc_stdtime_t now) {
        /*
         * Ensure this operation is applied to both hash tables at once.
         */
-       LOCK(&adb->names_lock);
+       RWLOCK(&adb->names_lock, isc_rwlocktype_write);
 
        for (dns_adbname_t *name = ISC_LIST_HEAD(adb->names_lru); name != NULL;
             name = ISC_LIST_NEXT(name, link))
@@ -2546,7 +2598,7 @@ dump_adb(dns_adb_t *adb, FILE *f, bool debug, isc_stdtime_t now) {
                UNLOCK(&name->lock);
        }
 
-       LOCK(&adb->entries_lock);
+       RWLOCK(&adb->entries_lock, isc_rwlocktype_write);
        fprintf(f, ";\n; Unassociated entries\n;\n");
        for (dns_adbentry_t *adbentry = ISC_LIST_HEAD(adb->entries_lru);
             adbentry != NULL; adbentry = ISC_LIST_NEXT(adbentry, link))
@@ -2558,8 +2610,8 @@ dump_adb(dns_adb_t *adb, FILE *f, bool debug, isc_stdtime_t now) {
                UNLOCK(&adbentry->lock);
        }
 
-       UNLOCK(&adb->entries_lock);
-       UNLOCK(&adb->names_lock);
+       RWUNLOCK(&adb->entries_lock, isc_rwlocktype_write);
+       RWUNLOCK(&adb->names_lock, isc_rwlocktype_write);
 }
 
 static void
@@ -2735,7 +2787,7 @@ dns_adb_dumpquota(dns_adb_t *adb, isc_buffer_t **buf) {
        isc_hashmap_iter_t *it = NULL;
        isc_result_t result;
 
-       LOCK(&adb->entries_lock);
+       RWLOCK(&adb->entries_lock, isc_rwlocktype_read);
        isc_hashmap_iter_create(adb->entries, &it);
        for (result = isc_hashmap_iter_first(it); result == ISC_R_SUCCESS;
             result = isc_hashmap_iter_next(it))
@@ -2764,7 +2816,7 @@ dns_adb_dumpquota(dns_adb_t *adb, isc_buffer_t **buf) {
                UNLOCK(&entry->lock);
        }
        isc_hashmap_iter_destroy(&it);
-       UNLOCK(&adb->entries_lock);
+       RWUNLOCK(&adb->entries_lock, isc_rwlocktype_read);
 
        return (ISC_R_SUCCESS);
 }
@@ -3612,7 +3664,7 @@ dns_adb_flushname(dns_adb_t *adb, const dns_name_t *name) {
                return;
        }
 
-       LOCK(&adb->names_lock);
+       RWLOCK(&adb->names_lock, isc_rwlocktype_write);
 again:
        /*
         * Delete both entries - without and with NAME_STARTATZONE set.
@@ -3636,7 +3688,7 @@ again:
                start_at_zone = true;
                goto again;
        }
-       UNLOCK(&adb->names_lock);
+       RWUNLOCK(&adb->names_lock, isc_rwlocktype_write);
 }
 
 void
@@ -3650,7 +3702,7 @@ dns_adb_flushnames(dns_adb_t *adb, const dns_name_t *name) {
                return;
        }
 
-       LOCK(&adb->names_lock);
+       RWLOCK(&adb->names_lock, isc_rwlocktype_write);
        for (dns_adbname_t *adbname = ISC_LIST_HEAD(adb->names_lru);
             adbname != NULL; adbname = next)
        {
@@ -3663,7 +3715,7 @@ dns_adb_flushnames(dns_adb_t *adb, const dns_name_t *name) {
                UNLOCK(&adbname->lock);
                dns_adbname_detach(&adbname);
        }
-       UNLOCK(&adb->names_lock);
+       RWUNLOCK(&adb->names_lock, isc_rwlocktype_write);
 }
 
 static void