From b9f4ba19a647e83b18c17568cdf0c6932777c020 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 30 Sep 2019 13:45:40 +0200 Subject: [PATCH 1/3] Use integer fast type for mutexatomic shim type --- lib/isc/include/isc/mutexatomic.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/isc/include/isc/mutexatomic.h b/lib/isc/include/isc/mutexatomic.h index 94d8998822..a1ccf7c9fc 100644 --- a/lib/isc/include/isc/mutexatomic.h +++ b/lib/isc/include/isc/mutexatomic.h @@ -75,22 +75,22 @@ typedef enum memory_order memory_order; typedef struct atomic_int_fast32 { isc_mutex_t m; - int32_t v; + int_fast32_t v; } atomic_int_fast32_t; typedef struct atomic_int_fast64 { isc_mutex_t m; - int64_t v; + int_fast64_t v; } atomic_int_fast64_t; typedef struct atomic_uint_fast32 { isc_mutex_t m; - uint32_t v; + uint_fast32_t v; } atomic_uint_fast32_t; typedef struct atomic_uint_fast64 { isc_mutex_t m; - uint64_t v; + uint_fast64_t v; } atomic_uint_fast64_t; typedef struct atomic_uintptr { From bad5a523c2e7b90562b959fe9391bb1b296b3b45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Witold=20Kr=C4=99cicki?= Date: Tue, 22 Jan 2019 10:47:18 +0100 Subject: [PATCH 2/3] lib/dns/adb.c: Use atomics for adb quota values and reference counting --- lib/dns/adb.c | 77 +++++++++++++++++++++++---------------------------- 1 file changed, 35 insertions(+), 42 deletions(-) diff --git a/lib/dns/adb.c b/lib/dns/adb.c index ce879f09d5..d07dcd36ad 100644 --- a/lib/dns/adb.c +++ b/lib/dns/adb.c @@ -252,8 +252,8 @@ struct dns_adbentry { unsigned char to4096; /* Our max. */ uint8_t mode; - uint32_t quota; - uint32_t active; + atomic_uint_fast32_t quota; + atomic_uint_fast32_t active; double atr; /* @@ -1832,9 +1832,9 @@ new_adbentry(dns_adb_t *adb) { e->srtt = (isc_random_uniform(0x1f)) + 1; e->lastage = 0; e->expires = 0; - e->active = 0; + atomic_init(&e->active, 0); e->mode = 0; - e->quota = adb->quota; + atomic_init(&e->quota, adb->quota); e->atr = 0.0; ISC_LIST_INIT(e->lameinfo); ISC_LINK_INIT(e, plink); @@ -2161,8 +2161,10 @@ log_quota(dns_adbentry_t *entry, const char *fmt, ...) { isc_netaddr_format(&netaddr, addrbuf, sizeof(addrbuf)); isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_ADB, - ISC_LOG_INFO, "adb: quota %s (%u/%u): %s", - addrbuf, entry->active, entry->quota, msgbuf); + ISC_LOG_INFO, + "adb: quota %s (%" PRIuFAST32 "/%" PRIuFAST32 "): %s", + addrbuf, atomic_load_relaxed(&entry->active), + atomic_load_relaxed(&entry->quota), msgbuf); } static void @@ -2185,9 +2187,7 @@ copy_namehook_lists(dns_adb_t *adb, dns_adbfind_t *find, INSIST(bucket != DNS_ADB_INVALIDBUCKET); LOCK(&adb->entrylocks[bucket]); - if (entry->quota != 0 && - entry->active >= entry->quota) - { + if (dns_adbentry_overquota(entry)) { find->options |= (DNS_ADBFIND_LAMEPRUNED| DNS_ADBFIND_OVERQUOTA); @@ -2225,9 +2225,7 @@ copy_namehook_lists(dns_adb_t *adb, dns_adbfind_t *find, INSIST(bucket != DNS_ADB_INVALIDBUCKET); LOCK(&adb->entrylocks[bucket]); - if (entry->quota != 0 && - entry->active >= entry->quota) - { + if (dns_adbentry_overquota(entry)) { find->options |= (DNS_ADBFIND_LAMEPRUNED| DNS_ADBFIND_OVERQUOTA); @@ -3534,8 +3532,8 @@ dump_entry(FILE *f, dns_adb_t *adb, dns_adbentry_t *entry, fprintf(f, " [ttl %d]", (int)(entry->expires - now)); if (adb != NULL && adb->quota != 0 && adb->atr_freq != 0) { - fprintf(f, " [atr %0.2f] [quota %u]", - entry->atr, entry->quota); + fprintf(f, " [atr %0.2f] [quota %" PRIuFAST32 "]", + entry->atr, atomic_load_relaxed(&entry->quota)); } fprintf(f, "\n"); @@ -4257,21 +4255,25 @@ maybe_adjust_quota(dns_adb_t *adb, dns_adbaddrinfo_t *addr, addr->entry->atr = ISC_CLAMP(addr->entry->atr, 0.0, 1.0); if (addr->entry->atr < adb->atr_low && addr->entry->mode > 0) { - addr->entry->quota = adb->quota * - quota_adj[--addr->entry->mode] / 10000; - log_quota(addr->entry, "atr %0.2f, quota increased to %u", - addr->entry->atr, addr->entry->quota); + uint_fast32_t new_quota = + adb->quota * quota_adj[--addr->entry->mode] / 10000; + atomic_store_release(&addr->entry->quota, + ISC_MIN(1, new_quota)); + log_quota(addr->entry, "atr %0.2f, quota increased to %" + PRIuFAST32, + addr->entry->atr, + new_quota); } else if (addr->entry->atr > adb->atr_high && addr->entry->mode < (QUOTA_ADJ_SIZE - 1)) { - addr->entry->quota = adb->quota * - quota_adj[++addr->entry->mode] / 10000; - log_quota(addr->entry, "atr %0.2f, quota decreased to %u", - addr->entry->atr, addr->entry->quota); + uint_fast32_t new_quota = + adb->quota * quota_adj[++addr->entry->mode] / 10000; + atomic_store_release(&addr->entry->quota, + ISC_MIN(1, new_quota)); + log_quota(addr->entry, "atr %0.2f, quota decreased to %" + PRIuFAST32, + addr->entry->atr, + new_quota); } - - /* Ensure we don't drop to zero */ - if (addr->entry->quota == 0) - addr->entry->quota = 1; } #define EDNSTOS 3U @@ -4773,34 +4775,25 @@ dns_adb_setquota(dns_adb_t *adb, uint32_t quota, uint32_t freq, bool dns_adbentry_overquota(dns_adbentry_t *entry) { REQUIRE(DNS_ADBENTRY_VALID(entry)); - return (entry->quota != 0 && entry->active >= entry->quota); + + uint_fast32_t quota = atomic_load_relaxed(&entry->quota); + uint_fast32_t active = atomic_load_acquire(&entry->active); + + return (quota != 0 && active >= quota); } void dns_adb_beginudpfetch(dns_adb_t *adb, dns_adbaddrinfo_t *addr) { - int bucket; - REQUIRE(DNS_ADB_VALID(adb)); REQUIRE(DNS_ADBADDRINFO_VALID(addr)); - bucket = addr->entry->lock_bucket; - - LOCK(&adb->entrylocks[bucket]); - addr->entry->active++; - UNLOCK(&adb->entrylocks[bucket]); + INSIST(atomic_fetch_add_relaxed(&addr->entry->active, 1) != UINT32_MAX); } void dns_adb_endudpfetch(dns_adb_t *adb, dns_adbaddrinfo_t *addr) { - int bucket; - REQUIRE(DNS_ADB_VALID(adb)); REQUIRE(DNS_ADBADDRINFO_VALID(addr)); - bucket = addr->entry->lock_bucket; - - LOCK(&adb->entrylocks[bucket]); - if (addr->entry->active > 0) - addr->entry->active--; - UNLOCK(&adb->entrylocks[bucket]); + INSIST(atomic_fetch_sub_release(&addr->entry->active, 1) != 0); } From 21eab267dfdcbfc46a65025697837aa214a75972 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 19 Nov 2019 13:49:28 +0800 Subject: [PATCH 3/3] Fix missing adb->{e,i}refcnt locking --- lib/dns/adb.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/dns/adb.c b/lib/dns/adb.c index d07dcd36ad..94ce4d418f 100644 --- a/lib/dns/adb.c +++ b/lib/dns/adb.c @@ -2785,9 +2785,8 @@ dns_adb_detach(dns_adb_t **adbx) { adb = *adbx; *adbx = NULL; - INSIST(adb->erefcnt > 0); - LOCK(&adb->reflock); + INSIST(adb->erefcnt > 0); adb->erefcnt--; need_exit_check = (adb->erefcnt == 0 && adb->irefcnt == 0); UNLOCK(&adb->reflock); @@ -3393,10 +3392,13 @@ dump_adb(dns_adb_t *adb, FILE *f, bool debug, isc_stdtime_t now) { fprintf(f, "; [edns success/4096 timeout/1432 timeout/1232 timeout/" "512 timeout]\n"); fprintf(f, "; [plain success/timeout]\n;\n"); - if (debug) + if (debug) { + LOCK(&adb->reflock); fprintf(f, "; addr %p, erefcnt %u, irefcnt %u, finds out %u\n", adb, adb->erefcnt, adb->irefcnt, isc_mempool_getallocated(adb->nhmp)); + UNLOCK(&adb->reflock); + } /* * In TSAN mode we need to lock the locks individually, as TSAN