ComputerSurge/bind9
3
0
Fork 0
Code Pull Requests 1 Activity

Merge branch '4451-use-only-single-method-for-overmem-condition' into 'main'

Browse Source 
Refactor the handling of isc_mem overmem condition

Closes #4451

See merge request isc-projects/bind9!8523
This commit is contained in:
Ondřej Surý
2023-11-29 13:21:09 +00:00
parent 669800342d 022f1d8573
commit 235659b95a
7 changed files with 102 additions and 212 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
CHANGES
View File

@@ -1,3 +1,7 @@
6288. [func] Refactor the isc_mem overmem handling to always use
isc_mem_isovermem and remove the water callback.
[GL #4451]
6287. [bug] Recognize escapes when reading the public key from file.
[GL !8502]

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);
/*%<

175
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;
@@ -352,11 +350,6 @@ mem_realloc(isc_mem_t *ctx, void *old_ptr, size_t old_size, size_t new_size,
return (new_ptr);
}
#define stats_bucket(ctx, size) \
((size / STATS_BUCKET_SIZE) >= STATS_BUCKETS \
? &ctx->stats[STATS_BUCKETS] \
: &ctx->stats[size / STATS_BUCKET_SIZE])
/*!
* Update internal counters after a memory get.
*/
@@ -673,68 +666,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 +677,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 +688,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 +783,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 +810,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 +845,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 +863,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 +927,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'

35
tests/isc/mem_test.c
View File

@@ -291,6 +291,40 @@ ISC_RUN_TEST_IMPL(isc_mem_reallocate) {
isc_mem_free(mctx, data);
}
ISC_RUN_TEST_IMPL(isc_mem_overmem) {
isc_mem_t *omctx = NULL;
isc_mem_create(&omctx);
assert_non_null(omctx);
isc_mem_setwater(omctx, 1024, 512);
/* inuse < lo_water */
void *data1 = isc_mem_allocate(omctx, 256);
assert_false(isc_mem_isovermem(omctx));
/* lo_water < inuse < hi_water */
void *data2 = isc_mem_allocate(omctx, 512);
assert_false(isc_mem_isovermem(omctx));
/* hi_water < inuse */
void *data3 = isc_mem_allocate(omctx, 512);
assert_true(isc_mem_isovermem(omctx));
/* lo_water < inuse < hi_water */
isc_mem_free(omctx, data2);
assert_true(isc_mem_isovermem(omctx));
/* inuse < lo_water */
isc_mem_free(omctx, data3);
assert_false(isc_mem_isovermem(omctx));
/* inuse == 0 */
isc_mem_free(omctx, data1);
assert_false(isc_mem_isovermem(omctx));
isc_mem_destroy(&omctx);
}
#if ISC_MEM_TRACKLINES
/* test mem with no flags */
@@ -486,6 +520,7 @@ ISC_TEST_ENTRY(isc_mem_inuse)
ISC_TEST_ENTRY(isc_mem_zeroget)
ISC_TEST_ENTRY(isc_mem_reget)
ISC_TEST_ENTRY(isc_mem_reallocate)
ISC_TEST_ENTRY(isc_mem_overmem)
#if ISC_MEM_TRACKLINES
ISC_TEST_ENTRY(isc_mem_noflags)