[9.18] fix: dev: Fix memory ordering issues with atomic operations in the quota.c module
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. Also use relaxed memory ordering for `quota->max` and `quota->soft`, as done in the main branch; possible ordering issues for these variables are acceptable. Closes #5018 Merge branch '5018-quota-memory-ordering-fixes-9.18' into 'bind-9.18' See merge request isc-projects/bind9!10203
This commit is contained in:
@@ -42,25 +42,24 @@ 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_release("a->max, 0);
|
||||
atomic_store_release("a->used, 0);
|
||||
atomic_store_release("a->soft, 0);
|
||||
atomic_store_relaxed("a->max, 0);
|
||||
atomic_store_relaxed("a->soft, 0);
|
||||
isc_mutex_destroy("a->cblock);
|
||||
}
|
||||
|
||||
void
|
||||
isc_quota_soft(isc_quota_t *quota, unsigned int soft) {
|
||||
REQUIRE(VALID_QUOTA(quota));
|
||||
atomic_store_release("a->soft, soft);
|
||||
atomic_store_relaxed("a->soft, soft);
|
||||
}
|
||||
|
||||
void
|
||||
isc_quota_max(isc_quota_t *quota, unsigned int max) {
|
||||
REQUIRE(VALID_QUOTA(quota));
|
||||
atomic_store_release("a->max, max);
|
||||
atomic_store_relaxed("a->max, max);
|
||||
}
|
||||
|
||||
unsigned int
|
||||
@@ -78,14 +77,14 @@ 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
|
||||
quota_reserve(isc_quota_t *quota) {
|
||||
isc_result_t result;
|
||||
uint_fast32_t max = atomic_load_acquire("a->max);
|
||||
uint_fast32_t soft = atomic_load_acquire("a->soft);
|
||||
uint_fast32_t max = atomic_load_relaxed("a->max);
|
||||
uint_fast32_t soft = atomic_load_relaxed("a->soft);
|
||||
uint_fast32_t used = atomic_load_acquire("a->used);
|
||||
do {
|
||||
if (max != 0 && used >= max) {
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user