mir/bind
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-09-01 15:05:23 +00:00
Code Issues Releases Wiki Activity

Use C-RW-WP lock in the dns_resolver unit

Browse Source 
Replace the isc_mutex with isc_rwlock in the dns_resolver unit,
specifically, both fetch context and fetch counters now uses the C-RW-WP
locks.
This commit is contained in:
Ondřej Surý
2023-02-13 15:52:47 +01:00
parent 6ffda5920e
commit 3d3d3b8c58
1 changed files with 71 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

105
lib/dns/resolver.c
View File

@@ -30,6 +30,7 @@
#include <isc/random.h> #include <isc/random.h>
#include <isc/refcount.h> #include <isc/refcount.h>
#include <isc/result.h> #include <isc/result.h>
#include <isc/rwlock.h>
#include <isc/siphash.h> #include <isc/siphash.h>
#include <isc/stats.h> #include <isc/stats.h>
#include <isc/string.h> #include <isc/string.h>
@@ -325,6 +326,7 @@ struct fctxkey {
typedef struct fctxcount fctxcount_t; typedef struct fctxcount fctxcount_t;
struct fctxcount { struct fctxcount {
unsigned int magic; unsigned int magic;
isc_mutex_t lock;
dns_fixedname_t dfname; dns_fixedname_t dfname;
dns_name_t *domain; dns_name_t *domain;
uint_fast32_t count; uint_fast32_t count;
@@ -558,10 +560,10 @@ struct dns_resolver {
dns_dispatchset_t *dispatches6; dns_dispatchset_t *dispatches6;
isc_hashmap_t *fctxs; isc_hashmap_t *fctxs;
isc_mutex_t fctxs_lock; isc_rwlock_t fctxs_lock;
isc_hashmap_t *counters; isc_hashmap_t *counters;
isc_mutex_t counters_lock; isc_rwlock_t counters_lock;
unsigned int ntasks; unsigned int ntasks;
isc_task_t **tasks; isc_task_t **tasks;
@@ -1633,6 +1635,7 @@ fcount_incr(fetchctx_t *fctx, bool force) {
fctxcount_t *counter = NULL; fctxcount_t *counter = NULL;
uint32_t hashval; uint32_t hashval;
uint_fast32_t spill; uint_fast32_t spill;
isc_rwlocktype_t locktype = isc_rwlocktype_read;
REQUIRE(fctx != NULL); REQUIRE(fctx != NULL);
res = fctx->res; res = fctx->res;
@@ -1647,7 +1650,7 @@ fcount_incr(fetchctx_t *fctx, bool force) {
hashval = isc_hashmap_hash(res->counters, fctx->domain->ndata, hashval = isc_hashmap_hash(res->counters, fctx->domain->ndata,
fctx->domain->length); fctx->domain->length);
LOCK(&res->counters_lock); RWLOCK(&res->counters_lock, locktype);
result = isc_hashmap_find(res->counters, &hashval, fctx->domain->ndata, result = isc_hashmap_find(res->counters, &hashval, fctx->domain->ndata,
fctx->domain->length, (void **)&counter); fctx->domain->length, (void **)&counter);
switch (result) { switch (result) {
@@ -1660,12 +1663,24 @@ fcount_incr(fetchctx_t *fctx, bool force) {
.count = 0, .count = 0,
.allowed = 0, .allowed = 0,
}; };
isc_mutex_init(&counter->lock);
counter->domain = dns_fixedname_initname(&counter->dfname); counter->domain = dns_fixedname_initname(&counter->dfname);
dns_name_copy(fctx->domain, counter->domain); dns_name_copy(fctx->domain, counter->domain);
UPGRADELOCK(&res->counters_lock, locktype);
result = isc_hashmap_add(res->counters, &hashval, result = isc_hashmap_add(res->counters, &hashval,
counter->domain->ndata, counter->domain->ndata,
counter->domain->length, counter); counter->domain->length, counter);
if (result == ISC_R_EXISTS) {
isc_mutex_destroy(&counter->lock);
isc_mem_put(fctx->mctx, counter, sizeof(*counter));
counter = NULL;
result = isc_hashmap_find(
res->counters, &hashval, fctx->domain->ndata,
fctx->domain->length, (void **)&counter);
}
INSIST(result == ISC_R_SUCCESS); INSIST(result == ISC_R_SUCCESS);
break; break;
default: default:
@@ -1674,6 +1689,7 @@ fcount_incr(fetchctx_t *fctx, bool force) {
INSIST(VALID_FCTXCOUNT(counter)); INSIST(VALID_FCTXCOUNT(counter));
INSIST(spill > 0); INSIST(spill > 0);
LOCK(&counter->lock);
if (++counter->count > spill) { if (++counter->count > spill) {
counter->count--; counter->count--;
INSIST(counter->count > 0); INSIST(counter->count > 0);
@@ -1684,7 +1700,8 @@ fcount_incr(fetchctx_t *fctx, bool force) {
counter->allowed++; counter->allowed++;
fctx->counter = counter; fctx->counter = counter;
} }
UNLOCK(&res->counters_lock); UNLOCK(&counter->lock);
RWUNLOCK(&res->counters_lock, locktype);
return (result); return (result);
} }
@@ -1699,19 +1716,35 @@ fcount_decr(fetchctx_t *fctx) {
} }
fctx->counter = NULL; fctx->counter = NULL;
LOCK(&fctx->res->counters_lock); /*
* FIXME: This should not require a write lock, but should be
* implemented using reference counting later, otherwise we would could
* encounter ABA problem here - the count could go up and down when we
* switch from read to write lock.
*/
RWLOCK(&fctx->res->counters_lock, isc_rwlocktype_write);
LOCK(&counter->lock);
INSIST(VALID_FCTXCOUNT(counter)); INSIST(VALID_FCTXCOUNT(counter));
INSIST(counter->count > 0); INSIST(counter->count > 0);
if (--counter->count == 0) { if (--counter->count > 0) {
isc_result_t result = isc_hashmap_delete( UNLOCK(&counter->lock);
fctx->res->counters, NULL, counter->domain->ndata, RWUNLOCK(&fctx->res->counters_lock, isc_rwlocktype_write);
counter->domain->length); return;
INSIST(result == ISC_R_SUCCESS);
fcount_logspill(fctx, counter, true);
isc_mem_put(fctx->mctx, counter, sizeof(*counter));
} }
UNLOCK(&fctx->res->counters_lock);
isc_result_t result = isc_hashmap_delete(fctx->res->counters, NULL,
counter->domain->ndata,
counter->domain->length);
INSIST(result == ISC_R_SUCCESS);
fcount_logspill(fctx, counter, true);
UNLOCK(&counter->lock);
isc_mutex_destroy(&counter->lock);
isc_mem_put(fctx->mctx, counter, sizeof(*counter));
RWUNLOCK(&fctx->res->counters_lock, isc_rwlocktype_write);
} }
static void static void
@@ -7201,12 +7234,12 @@ release_fctx(fetchctx_t *fctx) {
return; return;
} }
LOCK(&res->fctxs_lock); RWLOCK(&res->fctxs_lock, isc_rwlocktype_write);
result = isc_hashmap_delete(res->fctxs, &hashval, fctx->key.key, result = isc_hashmap_delete(res->fctxs, &hashval, fctx->key.key,
fctx->key.size); fctx->key.size);
INSIST(result == ISC_R_SUCCESS); INSIST(result == ISC_R_SUCCESS);
fctx->hashed = false; fctx->hashed = false;
UNLOCK(&res->fctxs_lock); RWUNLOCK(&res->fctxs_lock, isc_rwlocktype_write);
} }
static void static void
@@ -10072,11 +10105,11 @@ dns_resolver__destroy(dns_resolver_t *res) {
INSIST(isc_hashmap_count(res->fctxs) == 0); INSIST(isc_hashmap_count(res->fctxs) == 0);
isc_hashmap_destroy(&res->fctxs); isc_hashmap_destroy(&res->fctxs);
isc_mutex_destroy(&res->fctxs_lock); isc_rwlock_destroy(&res->fctxs_lock);
INSIST(isc_hashmap_count(res->counters) == 0); INSIST(isc_hashmap_count(res->counters) == 0);
isc_hashmap_destroy(&res->counters); isc_hashmap_destroy(&res->counters);
isc_mutex_destroy(&res->counters_lock); isc_rwlock_destroy(&res->counters_lock);
if (res->dispatches4 != NULL) { if (res->dispatches4 != NULL) {
dns_dispatchset_destroy(&res->dispatches4); dns_dispatchset_destroy(&res->dispatches4);
@@ -10206,11 +10239,11 @@ dns_resolver_create(dns_view_t *view, isc_loopmgr_t *loopmgr,
/* This needs to be case sensitive to not lowercase options and type */ /* This needs to be case sensitive to not lowercase options and type */
isc_hashmap_create(view->mctx, RES_DOMAIN_HASH_BITS, isc_hashmap_create(view->mctx, RES_DOMAIN_HASH_BITS,
ISC_HASHMAP_CASE_SENSITIVE, &res->fctxs); ISC_HASHMAP_CASE_SENSITIVE, &res->fctxs);
isc_mutex_init(&res->fctxs_lock); isc_rwlock_init(&res->fctxs_lock);
isc_hashmap_create(view->mctx, RES_DOMAIN_HASH_BITS, isc_hashmap_create(view->mctx, RES_DOMAIN_HASH_BITS,
ISC_HASHMAP_CASE_INSENSITIVE, &res->counters); ISC_HASHMAP_CASE_INSENSITIVE, &res->counters);
isc_mutex_init(&res->counters_lock); isc_rwlock_init(&res->counters_lock);
if (dispatchv4 != NULL) { if (dispatchv4 != NULL) {
dns_dispatchset_create(res->mctx, dispatchv4, &res->dispatches4, dns_dispatchset_create(res->mctx, dispatchv4, &res->dispatches4,
@@ -10370,7 +10403,7 @@ dns_resolver_shutdown(dns_resolver_t *res) {
RTRACE("exiting"); RTRACE("exiting");
LOCK(&res->fctxs_lock); RWLOCK(&res->fctxs_lock, isc_rwlocktype_write);
isc_hashmap_iter_create(res->fctxs, &it); isc_hashmap_iter_create(res->fctxs, &it);
for (result = isc_hashmap_iter_first(it); for (result = isc_hashmap_iter_first(it);
result == ISC_R_SUCCESS; result == ISC_R_SUCCESS;
@@ -10386,7 +10419,7 @@ dns_resolver_shutdown(dns_resolver_t *res) {
fctx); fctx);
} }
isc_hashmap_iter_destroy(&it); isc_hashmap_iter_destroy(&it);
UNLOCK(&res->fctxs_lock); RWUNLOCK(&res->fctxs_lock, isc_rwlocktype_write);
LOCK(&res->lock); LOCK(&res->lock);
if (res->spillattimer != NULL) { if (res->spillattimer != NULL) {
@@ -10525,6 +10558,7 @@ get_attached_fctx(dns_resolver_t *res, const dns_name_t *name,
name->length, name->length,
}; };
fetchctx_t *fctx = NULL; fetchctx_t *fctx = NULL;
isc_rwlocktype_t locktype = isc_rwlocktype_read;
STATIC_ASSERT(sizeof(key.options) == sizeof(options), STATIC_ASSERT(sizeof(key.options) == sizeof(options),
"key options size mismatch"); "key options size mismatch");
@@ -10538,7 +10572,7 @@ get_attached_fctx(dns_resolver_t *res, const dns_name_t *name,
hashval = isc_hashmap_hash(res->fctxs, key.key, key.size); hashval = isc_hashmap_hash(res->fctxs, key.key, key.size);
again: again:
LOCK(&res->fctxs_lock); RWLOCK(&res->fctxs_lock, locktype);
result = isc_hashmap_find(res->fctxs, &hashval, key.key, key.size, result = isc_hashmap_find(res->fctxs, &hashval, key.key, key.size,
(void **)&fctx); (void **)&fctx);
switch (result) { switch (result) {
@@ -10552,13 +10586,17 @@ again:
goto unlock; goto unlock;
} }
*new_fctx = true; UPGRADELOCK(&res->fctxs_lock, locktype);
result = isc_hashmap_add(res->fctxs, &hashval, fctx->key.key, result = isc_hashmap_add(res->fctxs, &hashval, fctx->key.key,
fctx->key.size, fctx); fctx->key.size, fctx);
if (result == ISC_R_SUCCESS) {
fctx->hashed = true; *new_fctx = true;
fctx->hashed = true;
} else {
fctx_done_detach(&fctx, result);
result = isc_hashmap_find(res->fctxs, &hashval, key.key,
key.size, (void **)&fctx);
}
INSIST(result == ISC_R_SUCCESS); INSIST(result == ISC_R_SUCCESS);
break; break;
default: default:
@@ -10566,8 +10604,7 @@ again:
} }
fetchctx_ref(fctx); fetchctx_ref(fctx);
unlock: unlock:
UNLOCK(&res->fctxs_lock); RWUNLOCK(&res->fctxs_lock, locktype);
if (result == ISC_R_SUCCESS) { if (result == ISC_R_SUCCESS) {
LOCK(&fctx->lock); LOCK(&fctx->lock);
if (SHUTTINGDOWN(fctx) || fctx->cloned) { if (SHUTTINGDOWN(fctx) || fctx->cloned) {
@@ -11388,7 +11425,7 @@ dns_resolver_dumpfetches(dns_resolver_t *res, isc_statsformat_t format,
REQUIRE(fp != NULL); REQUIRE(fp != NULL);
REQUIRE(format == isc_statsformat_file); REQUIRE(format == isc_statsformat_file);
LOCK(&res->counters_lock); RWLOCK(&res->counters_lock, isc_rwlocktype_read);
isc_hashmap_iter_create(res->counters, &it); isc_hashmap_iter_create(res->counters, &it);
for (result = isc_hashmap_iter_first(it); result == ISC_R_SUCCESS; for (result = isc_hashmap_iter_first(it); result == ISC_R_SUCCESS;
result = isc_hashmap_iter_next(it)) result = isc_hashmap_iter_next(it))
@@ -11402,7 +11439,7 @@ dns_resolver_dumpfetches(dns_resolver_t *res, isc_statsformat_t format,
" spilled, %" PRIuFAST32 " allowed)\n", " spilled, %" PRIuFAST32 " allowed)\n",
counter->count, counter->dropped, counter->allowed); counter->count, counter->dropped, counter->allowed);
} }
UNLOCK(&res->counters_lock); RWUNLOCK(&res->counters_lock, isc_rwlocktype_read);
isc_hashmap_iter_destroy(&it); isc_hashmap_iter_destroy(&it);
} }
@@ -11419,7 +11456,7 @@ dns_resolver_dumpquota(dns_resolver_t *res, isc_buffer_t **buf) {
return (ISC_R_SUCCESS); return (ISC_R_SUCCESS);
} }
LOCK(&res->counters_lock); RWLOCK(&res->counters_lock, isc_rwlocktype_read);
isc_hashmap_iter_create(res->counters, &it); isc_hashmap_iter_create(res->counters, &it);
for (result = isc_hashmap_iter_first(it); result == ISC_R_SUCCESS; for (result = isc_hashmap_iter_first(it); result == ISC_R_SUCCESS;
result = isc_hashmap_iter_next(it)) result = isc_hashmap_iter_next(it))
@@ -11451,7 +11488,7 @@ dns_resolver_dumpquota(dns_resolver_t *res, isc_buffer_t **buf) {
} }
cleanup: cleanup:
UNLOCK(&res->counters_lock); RWUNLOCK(&res->counters_lock, isc_rwlocktype_read);
isc_hashmap_iter_destroy(&it); isc_hashmap_iter_destroy(&it);
return (result); return (result);
} }