diff --git a/lib/dns/adb.c b/lib/dns/adb.c index b9aa285296..d0d30bd8da 100644 --- a/lib/dns/adb.c +++ b/lib/dns/adb.c @@ -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); } } diff --git a/lib/dns/cache.c b/lib/dns/cache.c index ed93b31c8a..c922a09284 100644 --- a/lib/dns/cache.c +++ b/lib/dns/cache.c @@ -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); } } diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 560ad74d88..6c099d8742 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -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); /*%< diff --git a/lib/isc/mem.c b/lib/isc/mem.c index fffdaade8a..b8ae369388 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -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 diff --git a/tests/dns/rbtdb_test.c b/tests/dns/rbtdb_test.c index 6f2ab3ce26..af68e21f17 100644 --- a/tests/dns/rbtdb_test.c +++ b/tests/dns/rbtdb_test.c @@ -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 * .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'