mir/bind
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-29 13:38:26 +00:00
Code Issues Releases Wiki Activity

Replace some ADB entry locking with atomics to reduce ADB contention

Browse Source 
Use atomics on couple of ADB entry members (.srtt, .flags, .expires, and
.lastage) to remove ADB entry locking from couple of hot spots.  The
most prominent place is copy_namehook_lists() that gets called under ADB
name lock and if the namehook list is long it acquires-releases quite a
few ADB entry locks.  Changing those ADB entry members to atomics
allowed us to new_adbaddrinfo() not require locked ADB entry and since
adbentry_overquota() already used atomics and handling lame information
was dropped in the previous commit, we could not make the
copy_namehook_lists() lockless.

The other hotspot is dns_adb_adjustsrtt() and dns_adb_agesrtt() that can
now use atomics because .srtt is already atomic_uint.

And the last place that could now use atomics is dns_adb_changeflags().
This commit is contained in:
Ondřej Surý 2023-09-21 11:59:01 +02:00
parent 2b20db05e3
commit cb0db600e7
No known key found for this signature in database
GPG Key ID: 2820F37E873DEA41
1 changed files with 46 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

98
lib/dns/adb.c
View File

@ -220,8 +220,8 @@ struct dns_adbentry {
isc_refcount_t references; isc_refcount_t references;
dns_adbnamehooklist_t nhs; dns_adbnamehooklist_t nhs;
unsigned int flags; atomic_uint flags;
unsigned int srtt; atomic_uint srtt;
unsigned int completed; unsigned int completed;
unsigned int timeouts; unsigned int timeouts;
unsigned char plain; unsigned char plain;
@ -239,8 +239,8 @@ struct dns_adbentry {
unsigned char *cookie; unsigned char *cookie;
uint16_t cookielen; uint16_t cookielen;
isc_stdtime_t expires; _Atomic(isc_stdtime_t) expires;
isc_stdtime_t lastage; _Atomic(isc_stdtime_t) lastage;
/*%< /*%<
* A nonzero 'expires' field indicates that the entry should * A nonzero 'expires' field indicates that the entry should
* persist until that time. This allows entries found * persist until that time. This allows entries found
@ -391,7 +391,7 @@ enum {
enum { enum {
ENTRY_IS_DEAD = 1 << 31, ENTRY_IS_DEAD = 1 << 31,
}; };
#define ENTRY_DEAD(e) (((e)->flags & ENTRY_IS_DEAD) != 0) #define ENTRY_DEAD(e) ((atomic_load(&(e)->flags) & ENTRY_IS_DEAD) != 0)
/* /*
* To the name, address classes are all that really exist. If it has a * To the name, address classes are all that really exist. If it has a
@ -1068,6 +1068,9 @@ new_adbentry(dns_adb_t *adb, const isc_sockaddr_t *addr) {
.srtt = isc_random_uniform(0x1f) + 1, .srtt = isc_random_uniform(0x1f) + 1,
.sockaddr = *addr, .sockaddr = *addr,
.link = ISC_LINK_INITIALIZER, .link = ISC_LINK_INITIALIZER,
.quota = adb->quota,
.references = ISC_REFCOUNT_INITIALIZER(1),
.adb = dns_adb_ref(adb),
.magic = DNS_ADBENTRY_MAGIC, .magic = DNS_ADBENTRY_MAGIC,
}; };
@ -1075,14 +1078,8 @@ new_adbentry(dns_adb_t *adb, const isc_sockaddr_t *addr) {
fprintf(stderr, "dns_adbentry__init:%s:%s:%d:%p->references = 1\n", fprintf(stderr, "dns_adbentry__init:%s:%s:%d:%p->references = 1\n",
__func__, __FILE__, __LINE__ + 1, entry); __func__, __FILE__, __LINE__ + 1, entry);
#endif #endif
isc_refcount_init(&entry->references, 1);
isc_mutex_init(&entry->lock); isc_mutex_init(&entry->lock);
atomic_init(&entry->active, 0);
atomic_init(&entry->quota, adb->quota);
dns_adb_attach(adb, &entry->adb);
inc_adbstats(adb, dns_adbstats_entriescnt); inc_adbstats(adb, dns_adbstats_entriescnt);
return (entry); return (entry);
@ -1210,8 +1207,8 @@ new_adbaddrinfo(dns_adb_t *adb, dns_adbentry_t *entry, in_port_t port) {
ai = isc_mem_get(adb->mctx, sizeof(*ai)); ai = isc_mem_get(adb->mctx, sizeof(*ai));
*ai = (dns_adbaddrinfo_t){ *ai = (dns_adbaddrinfo_t){
.srtt = entry->srtt, .srtt = atomic_load(&entry->srtt),
.flags = entry->flags, .flags = atomic_load(&entry->flags),
.publink = ISC_LINK_INITIALIZER, .publink = ISC_LINK_INITIALIZER,
.sockaddr = entry->sockaddr, .sockaddr = entry->sockaddr,
.entry = dns_adbentry_ref(entry), .entry = dns_adbentry_ref(entry),
@ -1492,7 +1489,6 @@ copy_namehook_lists(dns_adb_t *adb, dns_adbfind_t *find, dns_adbname_t *name) {
while (namehook != NULL) { while (namehook != NULL) {
dns_adbaddrinfo_t *addrinfo = NULL; dns_adbaddrinfo_t *addrinfo = NULL;
entry = namehook->entry; entry = namehook->entry;
LOCK(&entry->lock);
if (adbentry_overquota(entry)) { if (adbentry_overquota(entry)) {
find->options |= DNS_ADBFIND_OVERQUOTA; find->options |= DNS_ADBFIND_OVERQUOTA;
@ -1506,7 +1502,6 @@ copy_namehook_lists(dns_adb_t *adb, dns_adbfind_t *find, dns_adbname_t *name) {
*/ */
ISC_LIST_APPEND(find->list, addrinfo, publink); ISC_LIST_APPEND(find->list, addrinfo, publink);
nextv4: nextv4:
UNLOCK(&entry->lock);
namehook = ISC_LIST_NEXT(namehook, name_link); namehook = ISC_LIST_NEXT(namehook, name_link);
} }
} }
@ -1516,7 +1511,6 @@ copy_namehook_lists(dns_adb_t *adb, dns_adbfind_t *find, dns_adbname_t *name) {
while (namehook != NULL) { while (namehook != NULL) {
dns_adbaddrinfo_t *addrinfo = NULL; dns_adbaddrinfo_t *addrinfo = NULL;
entry = namehook->entry; entry = namehook->entry;
LOCK(&entry->lock);
if (adbentry_overquota(entry)) { if (adbentry_overquota(entry)) {
find->options |= DNS_ADBFIND_OVERQUOTA; find->options |= DNS_ADBFIND_OVERQUOTA;
@ -1530,7 +1524,6 @@ copy_namehook_lists(dns_adb_t *adb, dns_adbfind_t *find, dns_adbname_t *name) {
*/ */
ISC_LIST_APPEND(find->list, addrinfo, publink); ISC_LIST_APPEND(find->list, addrinfo, publink);
nextv6: nextv6:
UNLOCK(&entry->lock);
namehook = ISC_LIST_NEXT(namehook, name_link); namehook = ISC_LIST_NEXT(namehook, name_link);
} }
} }
@ -1572,7 +1565,7 @@ expire_entry(dns_adbentry_t *adbentry) {
dns_adb_t *adb = adbentry->adb; dns_adb_t *adb = adbentry->adb;
if (!ENTRY_DEAD(adbentry)) { if (!ENTRY_DEAD(adbentry)) {
adbentry->flags |= ENTRY_IS_DEAD; (void)atomic_fetch_or(&adbentry->flags, ENTRY_IS_DEAD);
result = isc_hashmap_delete( result = isc_hashmap_delete(
adb->entries, adb->entries,
@ -1591,7 +1584,9 @@ entry_expired(dns_adbentry_t *adbentry, isc_stdtime_t now) {
return (false); return (false);
} }
if (adbentry->expires == 0 || adbentry->expires > now) { if (atomic_load(&adbentry->expires) == 0 ||
atomic_load(&adbentry->expires) > now)
{
return (false); return (false);
} }
@ -2452,8 +2447,8 @@ dump_entry(FILE *f, dns_adb_t *adb, dns_adbentry_t *entry, bool debug,
fprintf(f, fprintf(f,
";\t%s [srtt %u] [flags %08x] [edns %u/%u] " ";\t%s [srtt %u] [flags %08x] [edns %u/%u] "
"[plain %u/%u]", "[plain %u/%u]",
addrbuf, entry->srtt, entry->flags, entry->edns, entry->ednsto, addrbuf, atomic_load(&entry->srtt), atomic_load(&entry->flags),
entry->plain, entry->plainto); entry->edns, entry->ednsto, entry->plain, entry->plainto);
if (entry->udpsize != 0U) { if (entry->udpsize != 0U) {
fprintf(f, " [udpsize %u]", entry->udpsize); fprintf(f, " [udpsize %u]", entry->udpsize);
} }
@ -2465,8 +2460,9 @@ dump_entry(FILE *f, dns_adb_t *adb, dns_adbentry_t *entry, bool debug,
} }
fprintf(f, "]"); fprintf(f, "]");
} }
if (entry->expires != 0) { if (atomic_load(&entry->expires) != 0) {
fprintf(f, " [ttl %d]", (int)(entry->expires - now)); fprintf(f, " [ttl %d]",
(int)(atomic_load(&entry->expires) - now));
} }
if (adb != NULL && adb->quota != 0 && adb->atr_freq != 0) { if (adb != NULL && adb->quota != 0 && adb->atr_freq != 0) {
@ -3031,16 +3027,14 @@ dns_adb_adjustsrtt(dns_adb_t *adb, dns_adbaddrinfo_t *addr, unsigned int rtt,
REQUIRE(DNS_ADBADDRINFO_VALID(addr)); REQUIRE(DNS_ADBADDRINFO_VALID(addr));
REQUIRE(factor <= 10); REQUIRE(factor <= 10);
isc_stdtime_t now = 0;
dns_adbentry_t *entry = addr->entry; dns_adbentry_t *entry = addr->entry;
LOCK(&entry->lock);
if (entry->expires == 0 || factor == DNS_ADB_RTTADJAGE) { isc_stdtime_t now = 0;
if (atomic_load(&entry->expires) == 0 || factor == DNS_ADB_RTTADJAGE) {
now = isc_stdtime_now(); now = isc_stdtime_now();
} }
adjustsrtt(addr, rtt, factor, now);
UNLOCK(&entry->lock); adjustsrtt(addr, rtt, factor, now);
} }
void void
@ -3048,40 +3042,36 @@ dns_adb_agesrtt(dns_adb_t *adb, dns_adbaddrinfo_t *addr, isc_stdtime_t now) {
REQUIRE(DNS_ADB_VALID(adb)); REQUIRE(DNS_ADB_VALID(adb));
REQUIRE(DNS_ADBADDRINFO_VALID(addr)); REQUIRE(DNS_ADBADDRINFO_VALID(addr));
dns_adbentry_t *entry = addr->entry;
LOCK(&entry->lock);
adjustsrtt(addr, 0, DNS_ADB_RTTADJAGE, now); adjustsrtt(addr, 0, DNS_ADB_RTTADJAGE, now);
UNLOCK(&entry->lock);
} }
static void static void
adjustsrtt(dns_adbaddrinfo_t *addr, unsigned int rtt, unsigned int factor, adjustsrtt(dns_adbaddrinfo_t *addr, unsigned int rtt, unsigned int factor,
isc_stdtime_t now) { isc_stdtime_t now) {
uint64_t new_srtt; unsigned int new_srtt;
if (factor == DNS_ADB_RTTADJAGE) { if (factor == DNS_ADB_RTTADJAGE) {
if (addr->entry->lastage != now) { if (atomic_load(&addr->entry->lastage) != now) {
new_srtt = addr->entry->srtt; new_srtt = addr->entry->srtt;
new_srtt <<= 9; new_srtt <<= 9;
new_srtt -= addr->entry->srtt; new_srtt -= addr->entry->srtt;
new_srtt >>= 9; new_srtt >>= 9;
addr->entry->lastage = now; atomic_store(&addr->entry->lastage, now);
} else { } else {
new_srtt = addr->entry->srtt; new_srtt = atomic_load(&addr->entry->srtt)
} }
} else { } else {
new_srtt = ((uint64_t)addr->entry->srtt / 10 * factor) + new_srtt = ((uint64_t)atomic_load(&addr->entry->srtt) / 10 *
factor) +
((uint64_t)rtt / 10 * (10 - factor)); ((uint64_t)rtt / 10 * (10 - factor));
} }
addr->entry->srtt = (unsigned int)new_srtt; atomic_store(&addr->entry->srtt, new_srtt);
addr->srtt = (unsigned int)new_srtt; addr->srtt = new_srtt;
if (addr->entry->expires == 0) { (void)atomic_compare_exchange_strong(&addr->entry->expires,
addr->entry->expires = now + ADB_ENTRY_WINDOW; &(isc_stdtime_t){ 0 },
} now + ADB_ENTRY_WINDOW);
} }
void void
@ -3093,12 +3083,16 @@ dns_adb_changeflags(dns_adb_t *adb, dns_adbaddrinfo_t *addr, unsigned int bits,
isc_stdtime_t now; isc_stdtime_t now;
dns_adbentry_t *entry = addr->entry; dns_adbentry_t *entry = addr->entry;
LOCK(&entry->lock); unsigned int flags = atomic_load(&entry->flags);
while (!atomic_compare_exchange_strong(&entry->flags, &flags,
(flags & ~mask) | (bits & mask)))
{
/* repeat */
}
entry->flags = (entry->flags & ~mask) | (bits & mask); if (atomic_load(&entry->expires) == 0) {
if (entry->expires == 0) {
now = isc_stdtime_now(); now = isc_stdtime_now();
entry->expires = now + ADB_ENTRY_WINDOW; atomic_store(&entry->expires, now + ADB_ENTRY_WINDOW);
} }
/* /*
@ -3106,8 +3100,6 @@ dns_adb_changeflags(dns_adb_t *adb, dns_adbaddrinfo_t *addr, unsigned int bits,
* the most recent values from addr->entry->flags. * the most recent values from addr->entry->flags.
*/ */
addr->flags = (addr->flags & ~mask) | (bits & mask); addr->flags = (addr->flags & ~mask) | (bits & mask);
UNLOCK(&entry->lock);
} }
/* /*
@ -3365,11 +3357,12 @@ dns_adb_findaddrinfo(dns_adb_t *adb, const isc_sockaddr_t *sa,
entry = get_attached_and_locked_entry(adb, now, sa); entry = get_attached_and_locked_entry(adb, now, sa);
INSIST(entry != NULL); INSIST(entry != NULL);
UNLOCK(&entry->lock);
port = isc_sockaddr_getport(sa); port = isc_sockaddr_getport(sa);
addr = new_adbaddrinfo(adb, entry, port); addr = new_adbaddrinfo(adb, entry, port);
*addrp = addr; *addrp = addr;
UNLOCK(&entry->lock);
dns_adbentry_detach(&entry); dns_adbentry_detach(&entry);
return (result); return (result);
@ -3393,10 +3386,9 @@ dns_adb_freeaddrinfo(dns_adb_t *adb, dns_adbaddrinfo_t **addrp) {
REQUIRE(DNS_ADBENTRY_VALID(entry)); REQUIRE(DNS_ADBENTRY_VALID(entry));
if (entry->expires == 0) {
now = isc_stdtime_now(); now = isc_stdtime_now();
entry->expires = now + ADB_ENTRY_WINDOW; (void)atomic_compare_exchange_strong(
} &entry->expires, &(isc_stdtime_t){ 0 }, now + ADB_ENTRY_WINDOW);
free_adbaddrinfo(adb, &addr); free_adbaddrinfo(adb, &addr);
} }