From df373d7d991aedd17ae98c65051768675e1e836e Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Thu, 27 Feb 2025 16:48:52 +0000 Subject: [PATCH] Fix memory ordering for operations with quota->used and quota->waiting Change all the non-locked operations on 'quota->used' and 'quota->waiting' to "acq/rel" for inter-thread synchronization. Some loads are left as "relaxed", because they are under a locked mutex which also provides protection. --- lib/isc/quota.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/isc/quota.c b/lib/isc/quota.c index ccafd240ea..fa2d562c9c 100644 --- a/lib/isc/quota.c +++ b/lib/isc/quota.c @@ -42,11 +42,10 @@ isc_quota_destroy(isc_quota_t *quota) { REQUIRE(VALID_QUOTA(quota)); quota->magic = 0; - INSIST(atomic_load("a->used) == 0); - INSIST(atomic_load("a->waiting) == 0); + INSIST(atomic_load_acquire("a->used) == 0); + INSIST(atomic_load_acquire("a->waiting) == 0); INSIST(ISC_LIST_EMPTY(quota->cbs)); atomic_store_relaxed("a->max, 0); - atomic_store_release("a->used, 0); atomic_store_relaxed("a->soft, 0); isc_mutex_destroy("a->cblock); } @@ -78,7 +77,7 @@ isc_quota_getsoft(isc_quota_t *quota) { unsigned int isc_quota_getused(isc_quota_t *quota) { REQUIRE(VALID_QUOTA(quota)); - return atomic_load_relaxed("a->used); + return atomic_load_acquire("a->used); } static isc_result_t @@ -106,6 +105,7 @@ static void enqueue(isc_quota_t *quota, isc_quota_cb_t *cb) { REQUIRE(cb != NULL); ISC_LIST_ENQUEUE(quota->cbs, cb, link); + /* No need for acquire; the lock protects from other writers. */ atomic_fetch_add_release("a->waiting, 1); } @@ -115,7 +115,8 @@ dequeue(isc_quota_t *quota) { isc_quota_cb_t *cb = ISC_LIST_HEAD(quota->cbs); INSIST(cb != NULL); ISC_LIST_DEQUEUE(quota->cbs, cb, link); - atomic_fetch_sub_relaxed("a->waiting, 1); + /* No need for acquire; the lock protects from other writers. */ + atomic_fetch_sub_release("a->waiting, 1); return cb; } @@ -133,6 +134,7 @@ quota_release(isc_quota_t *quota) { if (atomic_load_acquire("a->waiting) > 0) { isc_quota_cb_t *cb = NULL; LOCK("a->cblock); + /* No need for acquire; the lock protects from other writers. */ if (atomic_load_relaxed("a->waiting) > 0) { cb = dequeue(quota); } @@ -143,7 +145,7 @@ quota_release(isc_quota_t *quota) { } } - used = atomic_fetch_sub_release("a->used, 1); + used = atomic_fetch_sub_acq_rel("a->used, 1); INSIST(used > 0); }