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

Refactor the handling of isc_mem overmem condition

Browse Source 
Previously, there were two methods of working with the overmem
condition:

1. hi/lo water callback - when the overmem condition was reached
   for the first time, the water callback was called with HIWATER
   mark and .is_overmem boolean was set internally.  Similarly,
   when the used memory went below the lo water mark, the water
   callback would be called with LOWATER mark and .is_overmem
   was reset.  This check would be called **every** time memory
   was allocated or freed.

2. isc_mem_isovermem() - a simple getter for the internal
   .is_overmem flag

This commit refactors removes the first method and move the hi/lo water
checks to the isc_mem_isovermem() function, thus we now have only a
single method of checking overmem condition and the check for hi/lo
water is removed from the hot path for memory contexts that doesn't use
overmem checks.
This commit is contained in:
Ondřej Surý
2023-11-29 09:01:56 +01:00
parent 669800342d
commit 14bdd21e0a
5 changed files with 63 additions and 207 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
lib/dns/adb.c
View File

@@ -116,7 +116,6 @@ struct dns_adb {
isc_stats_t *stats;
atomic_bool exiting;
atomic_bool is_overmem;
uint32_t quota;
uint32_t atr_freq;
@@ -346,8 +345,6 @@ shutdown_names(dns_adb_t *);
static void
shutdown_entries(dns_adb_t *);
static void
water(void *, int);
static void
dump_entry(FILE *, dns_adb_t *, dns_adbentry_t *, bool, isc_stdtime_t);
static void
adjustsrtt(dns_adbaddrinfo_t *addr, unsigned int rtt, unsigned int factor,
@@ -1290,7 +1287,7 @@ get_attached_and_locked_name(dns_adb_t *adb, const dns_name_t *name,
last_update = adb->names_last_update;
if (last_update + ADB_STALE_MARGIN >= now ||
atomic_load_relaxed(&adb->is_overmem))
isc_mem_isovermem(adb->mctx))
{
last_update = now;
UPGRADELOCK(&adb->names_lock, locktype);
@@ -1384,7 +1381,7 @@ get_attached_and_locked_entry(dns_adb_t *adb, isc_stdtime_t now,
last_update = adb->entries_last_update;
if (now - last_update > ADB_STALE_MARGIN ||
atomic_load_relaxed(&adb->is_overmem))
isc_mem_isovermem(adb->mctx))
{
last_update = now;
@@ -1617,7 +1614,7 @@ maybe_expire_entry(dns_adbentry_t *adbentry, isc_stdtime_t now) {
*/
static void
purge_stale_names(dns_adb_t *adb, isc_stdtime_t now) {
bool overmem = atomic_load_relaxed(&adb->is_overmem);
bool overmem = isc_mem_isovermem(adb->mctx);
int max_removed = overmem ? 2 : 1;
int scans = 0, removed = 0;
dns_adbname_t *prev = NULL;
@@ -1722,7 +1719,7 @@ cleanup_names(dns_adb_t *adb, isc_stdtime_t now) {
*/
static void
purge_stale_entries(dns_adb_t *adb, isc_stdtime_t now) {
bool overmem = atomic_load_relaxed(&adb->is_overmem);
bool overmem = isc_mem_isovermem(adb->mctx);
int max_removed = overmem ? 2 : 1;
int scans = 0, removed = 0;
dns_adbentry_t *prev = NULL;
@@ -3478,18 +3475,6 @@ dns_adb_flushnames(dns_adb_t *adb, const dns_name_t *name) {
RWUNLOCK(&adb->names_lock, isc_rwlocktype_write);
}
static void
water(void *arg, int mark) {
dns_adb_t *adb = arg;
REQUIRE(DNS_ADB_VALID(adb));
atomic_store_release(&adb->is_overmem, (mark == ISC_MEM_HIWATER));
DP(ISC_LOG_DEBUG(1), "adb reached %s water mark",
(mark == ISC_MEM_HIWATER) ? "high" : "low");
}
void
dns_adb_setadbsize(dns_adb_t *adb, size_t size) {
size_t hiwater, lowater;
@@ -3506,7 +3491,7 @@ dns_adb_setadbsize(dns_adb_t *adb, size_t size) {
if (size == 0U || hiwater == 0U || lowater == 0U) {
isc_mem_clearwater(adb->mctx);
} else {
isc_mem_setwater(adb->mctx, water, adb, hiwater, lowater);
isc_mem_setwater(adb->mctx, hiwater, lowater);
}
}

9
lib/dns/cache.c
View File

@@ -239,13 +239,6 @@ dns_cache_getname(dns_cache_t *cache) {
return (cache->name);
}
/* This is a no-op, but has to exist for isc_mem_setwater(). */
static void
water(void *arg, int mark) {
UNUSED(arg);
UNUSED(mark);
}
void
dns_cache_setcachesize(dns_cache_t *cache, size_t size) {
REQUIRE(VALID_CACHE(cache));
@@ -267,7 +260,7 @@ dns_cache_setcachesize(dns_cache_t *cache, size_t size) {
if (size == 0U || hi == 0U || lo == 0U) {
isc_mem_clearwater(cache->mctx);
} else {
isc_mem_setwater(cache->mctx, water, cache, hi, lo);
isc_mem_setwater(cache->mctx, hi, lo);
}
}

52
lib/isc/include/isc/mem.h
View File

@@ -26,10 +26,6 @@
ISC_LANG_BEGINDECLS
#define ISC_MEM_LOWATER 0
#define ISC_MEM_HIWATER 1
typedef void (*isc_mem_water_t)(void *, int);
/*%
* Define ISC_MEM_TRACKLINES=1 to turn on detailed tracing of memory
* allocation and freeing by file and line number.
@@ -45,7 +41,8 @@ extern unsigned int isc_mem_defaultflags;
#define ISC_MEM_DEBUGTRACE 0x00000001U
#define ISC_MEM_DEBUGRECORD 0x00000002U
#define ISC_MEM_DEBUGUSAGE 0x00000004U
#define ISC_MEM_DEBUGALL 0x0000001FU
#define ISC_MEM_DEBUGALL \
(ISC_MEM_DEBUGTRACE | ISC_MEM_DEBUGRECORD | ISC_MEM_DEBUGUSAGE)
/*!<
* The variable isc_mem_debugging holds a set of flags for
* turning certain memory debugging options on or off at
@@ -61,8 +58,8 @@ extern unsigned int isc_mem_defaultflags;
* Crash if a free doesn't match an allocation.
*
* \li #ISC_MEM_DEBUGUSAGE
* If a hi_water mark is set, print the maximum inuse memory
* every time it is raised once it exceeds the hi_water mark.
* Every time the memory usage is greater (lower) than hi_water
* (lo_water) mark, print the current inuse memory.
*/
/*@}*/
@@ -292,51 +289,22 @@ isc_mem_isovermem(isc_mem_t *mctx);
void
isc_mem_clearwater(isc_mem_t *mctx);
void
isc_mem_setwater(isc_mem_t *mctx, isc_mem_water_t water, void *water_arg,
size_t hiwater, size_t lowater);
isc_mem_setwater(isc_mem_t *mctx, size_t hiwater, size_t lowater);
/*%<
* Set high and low water marks for this memory context.
*
* When the memory usage of 'mctx' exceeds 'hiwater',
* '(water)(water_arg, #ISC_MEM_HIWATER)' will be called. 'water' needs
* to call isc_mem_waterack() with #ISC_MEM_HIWATER to acknowledge the
* state change. 'water' may be called multiple times.
* When the memory usage of 'mctx' exceeds 'hiwater', the overmem condition
* will be met and isc_mem_isovermem() will return true.
*
* When the usage drops below 'lowater', 'water' will again be called,
* this time with #ISC_MEM_LOWATER. 'water' need to calls
* isc_mem_waterack() with #ISC_MEM_LOWATER to acknowledge the change.
* If the 'hiwater' and 'lowater' is set to 0, the high- and low-water
* processing are disabled for this memory context.
*
* static void
* water(void *arg, int mark) {
* struct foo *foo = arg;
*
* LOCK(&foo->marklock);
* if (foo->mark != mark) {
* foo->mark = mark;
* ....
* isc_mem_waterack(foo->mctx, mark);
* }
* UNLOCK(&foo->marklock);
* }
*
* if 'water' is set to NULL, the 'hiwater' and 'lowater' must set to 0, and
* high- and low-water processing are disabled for this memory context. There's
* a convenient function isc_mem_clearwater().
* There's a convenient function isc_mem_clearwater().
*
* Requires:
*
*\li If 'water' is NULL, 'hiwater' and 'lowater' must be set to 0.
*\li If 'water' and 'water_arg' have previously been set, they are
unchanged.
*\li 'hiwater' >= 'lowater'
*/
void
isc_mem_waterack(isc_mem_t *ctx, int mark);
/*%<
* Called to acknowledge changes in signaled by calls to 'water'.
*/
void
isc_mem_checkdestroyed(FILE *file);
/*%<

170
lib/isc/mem.c
View File

@@ -129,8 +129,6 @@ struct isc_mem {
atomic_size_t inuse;
atomic_bool hi_called;
atomic_bool is_overmem;
isc_mem_water_t water;
void *water_arg;
atomic_size_t hi_water;
atomic_size_t lo_water;
ISC_LIST(isc_mempool_t) pools;
@@ -673,68 +671,6 @@ isc__mem_destroy(isc_mem_t **ctxp FLARG) {
*ctxp = NULL;
}
#define CALL_HI_WATER(ctx) \
{ \
if (ctx->water != NULL && hi_water(ctx)) { \
(ctx->water)(ctx->water_arg, ISC_MEM_HIWATER); \
} \
}
#define CALL_LO_WATER(ctx) \
{ \
if ((ctx->water != NULL) && lo_water(ctx)) { \
(ctx->water)(ctx->water_arg, ISC_MEM_LOWATER); \
} \
}
static bool
hi_water(isc_mem_t *ctx) {
size_t inuse;
size_t hiwater = atomic_load_relaxed(&ctx->hi_water);
if (hiwater == 0) {
return (false);
}
inuse = atomic_load_relaxed(&ctx->inuse);
if (inuse <= hiwater) {
return (false);
}
if (atomic_load_acquire(&ctx->hi_called)) {
return (false);
}
/* We are over water (for the first time) */
atomic_store_release(&ctx->is_overmem, true);
return (true);
}
static bool
lo_water(isc_mem_t *ctx) {
size_t inuse;
size_t lowater = atomic_load_relaxed(&ctx->lo_water);
if (lowater == 0) {
return (false);
}
inuse = atomic_load_relaxed(&ctx->inuse);
if (inuse >= lowater) {
return (false);
}
if (!atomic_load_acquire(&ctx->hi_called)) {
return (false);
}
/* We are no longer overmem */
atomic_store_release(&ctx->is_overmem, false);
return (true);
}
void *
isc__mem_get(isc_mem_t *ctx, size_t size, int flags FLARG) {
void *ptr = NULL;
@@ -746,8 +682,6 @@ isc__mem_get(isc_mem_t *ctx, size_t size, int flags FLARG) {
mem_getstats(ctx, size);
ADD_TRACE(ctx, ptr, size, file, line);
CALL_HI_WATER(ctx);
return (ptr);
}
@@ -759,19 +693,6 @@ isc__mem_put(isc_mem_t *ctx, void *ptr, size_t size, int flags FLARG) {
mem_putstats(ctx, size);
mem_put(ctx, ptr, size, flags);
CALL_LO_WATER(ctx);
}
void
isc_mem_waterack(isc_mem_t *ctx, int flag) {
REQUIRE(VALID_CONTEXT(ctx));
if (flag == ISC_MEM_LOWATER) {
atomic_store_release(&ctx->hi_called, false);
} else if (flag == ISC_MEM_HIWATER) {
atomic_store_release(&ctx->hi_called, true);
}
}
#if ISC_MEM_TRACKLINES
@@ -867,8 +788,6 @@ isc__mem_allocate(isc_mem_t *ctx, size_t size, int flags FLARG) {
mem_getstats(ctx, size);
ADD_TRACE(ctx, ptr, size, file, line);
CALL_HI_WATER(ctx);
return (ptr);
}
@@ -896,8 +815,6 @@ isc__mem_reget(isc_mem_t *ctx, void *old_ptr, size_t old_size, size_t new_size,
* where the realloc will exactly hit on the boundary of
* the water and we would call water twice.
*/
CALL_LO_WATER(ctx);
CALL_HI_WATER(ctx);
}
return (new_ptr);
@@ -933,8 +850,6 @@ isc__mem_reallocate(isc_mem_t *ctx, void *old_ptr, size_t new_size,
* where the realloc will exactly hit on the boundary of
* the water and we would call water twice.
*/
CALL_LO_WATER(ctx);
CALL_HI_WATER(ctx);
}
return (new_ptr);
@@ -953,8 +868,6 @@ isc__mem_free(isc_mem_t *ctx, void *ptr, int flags FLARG) {
mem_putstats(ctx, size);
mem_put(ctx, ptr, size, flags);
CALL_LO_WATER(ctx);
}
/*
@@ -1019,59 +932,66 @@ isc_mem_inuse(isc_mem_t *ctx) {
void
isc_mem_clearwater(isc_mem_t *mctx) {
isc_mem_setwater(mctx, NULL, NULL, 0, 0);
isc_mem_setwater(mctx, 0, 0);
}
void
isc_mem_setwater(isc_mem_t *ctx, isc_mem_water_t water, void *water_arg,
size_t hiwater, size_t lowater) {
isc_mem_water_t oldwater;
void *oldwater_arg;
isc_mem_setwater(isc_mem_t *ctx, size_t hiwater, size_t lowater) {
REQUIRE(VALID_CONTEXT(ctx));
REQUIRE(hiwater >= lowater);
oldwater = ctx->water;
oldwater_arg = ctx->water_arg;
/* No water was set and new water is also NULL */
if (oldwater == NULL && water == NULL) {
return;
}
/* The water function is being set for the first time */
if (oldwater == NULL) {
REQUIRE(water != NULL && lowater > 0);
INSIST(atomic_load_acquire(&ctx->hi_water) == 0);
INSIST(atomic_load_acquire(&ctx->lo_water) == 0);
ctx->water = water;
ctx->water_arg = water_arg;
atomic_store_release(&ctx->hi_water, hiwater);
atomic_store_release(&ctx->lo_water, lowater);
return;
}
REQUIRE((water == oldwater && water_arg == oldwater_arg) ||
(water == NULL && water_arg == NULL && hiwater == 0));
atomic_store_release(&ctx->hi_water, hiwater);
atomic_store_release(&ctx->lo_water, lowater);
if (atomic_load_acquire(&ctx->hi_called) &&
(atomic_load_acquire(&ctx->inuse) < lowater || lowater == 0U))
{
(oldwater)(oldwater_arg, ISC_MEM_LOWATER);
}
return;
}
bool
isc_mem_isovermem(isc_mem_t *ctx) {
REQUIRE(VALID_CONTEXT(ctx));
return (atomic_load_relaxed(&ctx->is_overmem));
bool is_overmem = atomic_load_relaxed(&ctx->is_overmem);
if (!is_overmem) {
/* We are not overmem, check whether we should be? */
size_t hiwater = atomic_load_relaxed(&ctx->hi_water);
if (hiwater == 0) {
return (false);
}
size_t inuse = atomic_load_relaxed(&ctx->inuse);
if (inuse <= hiwater) {
return (false);
}
if ((isc_mem_debugging & ISC_MEM_DEBUGUSAGE) != 0) {
fprintf(stderr,
"overmem mctx %p inuse %zu hi_water %zu\n", ctx,
inuse, hiwater);
}
atomic_store_relaxed(&ctx->is_overmem, true);
return (true);
} else {
/* We are overmem, check whether we should not be? */
size_t lowater = atomic_load_relaxed(&ctx->lo_water);
if (lowater == 0) {
return (false);
}
size_t inuse = atomic_load_relaxed(&ctx->inuse);
if (inuse >= lowater) {
return (true);
}
if ((isc_mem_debugging & ISC_MEM_DEBUGUSAGE) != 0) {
fprintf(stderr,
"overmem mctx %p inuse %zu lo_water %zu\n", ctx,
inuse, lowater);
}
atomic_store_relaxed(&ctx->is_overmem, false);
return (false);
}
}
void

14
tests/dns/rbtdb_test.c
View File

@@ -215,16 +215,6 @@ ISC_RUN_TEST_IMPL(setownercase) {
assert_true(dns_name_caseequal(name1, name2));
}
/*
* No operation water() callback. We need it to cause overmem condition, but
* nothing has to be done in the callback.
*/
static void
overmempurge_water(void *arg, int mark) {
UNUSED(arg);
UNUSED(mark);
}
/*
* Add to a cache DB 'db' an rdataset of type 'rtype' at a name
* <idx>.example.com. The rdataset would contain one data, and rdata_len is
@@ -307,7 +297,7 @@ ISC_RUN_TEST_IMPL(overmempurge_bigrdata) {
dns_rdataclass_in, 0, NULL, &db);
assert_int_equal(result, ISC_R_SUCCESS);
isc_mem_setwater(mctx2, overmempurge_water, NULL, hiwater, lowater);
isc_mem_setwater(mctx2, hiwater, lowater);
/*
* Add cache entries with minimum size of data until 'overmem'
@@ -351,7 +341,7 @@ ISC_RUN_TEST_IMPL(overmempurge_longname) {
dns_rdataclass_in, 0, NULL, &db);
assert_int_equal(result, ISC_R_SUCCESS);
isc_mem_setwater(mctx2, overmempurge_water, NULL, hiwater, lowater);
isc_mem_setwater(mctx2, hiwater, lowater);
/*
* Add cache entries with minimum size of data until 'overmem'