From 5bcb0361013c8b65966999558c4cfb55dd7990fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 14 May 2019 00:39:07 +0700 Subject: [PATCH] Make isc_rwlock.c thread-safe --- lib/isc/include/isc/rwlock.h | 4 +- lib/isc/rwlock.c | 141 +++++++++++++++++------------------ 2 files changed, 71 insertions(+), 74 deletions(-) diff --git a/lib/isc/include/isc/rwlock.h b/lib/isc/include/isc/rwlock.h index 1b2674a335..fc71663f0d 100644 --- a/lib/isc/include/isc/rwlock.h +++ b/lib/isc/include/isc/rwlock.h @@ -58,10 +58,10 @@ struct isc_rwlock { /* Locked by lock. */ isc_condition_t readable; isc_condition_t writeable; - unsigned int readers_waiting; + unsigned int readers_waiting; /* Locked by rwlock itself. */ - unsigned int write_granted; + atomic_int_fast32_t write_granted; /* Unlocked. */ unsigned int write_quota; diff --git a/lib/isc/rwlock.c b/lib/isc/rwlock.c index 844f45d205..5f24dae1ff 100644 --- a/lib/isc/rwlock.c +++ b/lib/isc/rwlock.c @@ -81,11 +81,11 @@ print_lock(const char *operation, isc_rwlock_t *rwl, isc_rwlocktype_t type) { "write_granted=%u, write_quota=%u\n", rwl, isc_thread_self(), operation, (type == isc_rwlocktype_read ? "read" : "write"), - atomic_load_explicit(&rwl->write_requests, memory_order_relaxed), - atomic_load_explicit(&rwl->write_completions, memory_order_relaxed), - atomic_load_explicit(&rwl->cnt_and_flag, memory_order_relaxed), + atomic_load_relaxed(&rwl->write_requests), + atomic_load_relaxed(&rwl->write_completions), + atomic_load_relaxed(&rwl->cnt_and_flag), rwl->readers_waiting, - rwl->write_granted, rwl->write_quota); + atomic_load_relaxed(&rwl->write_granted), rwl->write_quota); } #endif /* ISC_RWLOCK_TRACE */ @@ -106,7 +106,7 @@ isc_rwlock_init(isc_rwlock_t *rwl, unsigned int read_quota, atomic_init(&rwl->write_completions, 0); atomic_init(&rwl->cnt_and_flag, 0); rwl->readers_waiting = 0; - rwl->write_granted = 0; + atomic_init(&rwl->write_granted, 0); if (read_quota != 0) { UNEXPECTED_ERROR(__FILE__, __LINE__, "read quota is not supported"); @@ -129,9 +129,9 @@ void isc_rwlock_destroy(isc_rwlock_t *rwl) { REQUIRE(VALID_RWLOCK(rwl)); - REQUIRE(atomic_load_explicit(&rwl->write_requests, memory_order_relaxed) == - atomic_load_explicit(&rwl->write_completions, memory_order_relaxed) && - atomic_load_explicit(&rwl->cnt_and_flag, memory_order_relaxed) == 0 && rwl->readers_waiting == 0); + REQUIRE(atomic_load_relaxed(&rwl->write_requests) == + atomic_load_relaxed(&rwl->write_completions) && + atomic_load_relaxed(&rwl->cnt_and_flag) == 0 && rwl->readers_waiting == 0); rwl->magic = 0; (void)isc_condition_destroy(&rwl->readable); @@ -215,13 +215,13 @@ isc__rwlock_lock(isc_rwlock_t *rwl, isc_rwlocktype_t type) { #endif if (type == isc_rwlocktype_read) { - if (atomic_load_explicit(&rwl->write_requests, memory_order_relaxed) != - atomic_load_explicit(&rwl->write_completions, memory_order_relaxed)) + if (atomic_load_relaxed(&rwl->write_requests) != + atomic_load_relaxed(&rwl->write_completions)) { /* there is a waiting or active writer */ LOCK(&rwl->lock); - if (atomic_load_explicit(&rwl->write_requests, memory_order_relaxed) != - atomic_load_explicit(&rwl->write_completions, memory_order_relaxed)) { + if (atomic_load_relaxed(&rwl->write_requests) != + atomic_load_relaxed(&rwl->write_completions)) { rwl->readers_waiting++; WAIT(&rwl->readable, &rwl->lock); rwl->readers_waiting--; @@ -229,18 +229,22 @@ isc__rwlock_lock(isc_rwlock_t *rwl, isc_rwlocktype_t type) { UNLOCK(&rwl->lock); } - cntflag = atomic_fetch_add_explicit(&rwl->cnt_and_flag, - READER_INCR, - memory_order_relaxed); + cntflag = atomic_fetch_add_relaxed(&rwl->cnt_and_flag, + READER_INCR); POST(cntflag); while (1) { - if ((atomic_load_explicit(&rwl->cnt_and_flag, memory_order_relaxed) & WRITER_ACTIVE) == 0) + if ((atomic_load_relaxed(&rwl->cnt_and_flag) + & WRITER_ACTIVE) == 0) + { break; + } /* A writer is still working */ LOCK(&rwl->lock); rwl->readers_waiting++; - if ((atomic_load_explicit(&rwl->cnt_and_flag, memory_order_relaxed) & WRITER_ACTIVE) != 0) { + if ((atomic_load_relaxed(&rwl->cnt_and_flag) + & WRITER_ACTIVE) != 0) + { WAIT(&rwl->readable, &rwl->lock); } rwl->readers_waiting--; @@ -277,16 +281,19 @@ isc__rwlock_lock(isc_rwlock_t *rwl, isc_rwlocktype_t type) { * quota, reset the condition (race among readers doesn't * matter). */ - rwl->write_granted = 0; + atomic_store_relaxed(&rwl->write_granted, 0); } else { int32_t prev_writer; /* enter the waiting queue, and wait for our turn */ - prev_writer = atomic_fetch_add_explicit(&rwl->write_requests, 1, - memory_order_relaxed); - while (atomic_load_explicit(&rwl->write_completions, memory_order_relaxed) != prev_writer) { + prev_writer = atomic_fetch_add_relaxed(&rwl->write_requests, 1); + while (atomic_load_relaxed(&rwl->write_completions) + != prev_writer) + { LOCK(&rwl->lock); - if (atomic_load_explicit(&rwl->write_completions, memory_order_relaxed) != prev_writer) { + if (atomic_load_relaxed(&rwl->write_completions) + != prev_writer) + { WAIT(&rwl->writeable, &rwl->lock); UNLOCK(&rwl->lock); continue; @@ -297,23 +304,23 @@ isc__rwlock_lock(isc_rwlock_t *rwl, isc_rwlocktype_t type) { while (1) { int_fast32_t zero = 0; - if (atomic_compare_exchange_strong_explicit - (&rwl->cnt_and_flag, &zero, WRITER_ACTIVE, - memory_order_relaxed, memory_order_relaxed)) + if (atomic_compare_exchange_strong_relaxed( + &rwl->cnt_and_flag, &zero, WRITER_ACTIVE)) { break; } /* Another active reader or writer is working. */ LOCK(&rwl->lock); - if (atomic_load_explicit(&rwl->cnt_and_flag, memory_order_relaxed) != 0) { + if (atomic_load_relaxed(&rwl->cnt_and_flag) != 0) { WAIT(&rwl->writeable, &rwl->lock); } UNLOCK(&rwl->lock); } - INSIST((atomic_load_explicit(&rwl->cnt_and_flag, memory_order_relaxed) & WRITER_ACTIVE)); - rwl->write_granted++; + INSIST((atomic_load_relaxed(&rwl->cnt_and_flag) + & WRITER_ACTIVE)); + atomic_fetch_add_relaxed(&rwl->write_granted, 1); } #ifdef ISC_RWLOCK_TRACE @@ -326,7 +333,7 @@ isc__rwlock_lock(isc_rwlock_t *rwl, isc_rwlocktype_t type) { isc_result_t isc_rwlock_lock(isc_rwlock_t *rwl, isc_rwlocktype_t type) { int32_t cnt = 0; - int32_t spins = atomic_load_acquire(&rwl->spins) * 2 + 10; + int32_t spins = atomic_load_relaxed(&rwl->spins) * 2 + 10; int32_t max_cnt = ISC_MAX(spins, RWLOCK_MAX_ADAPTIVE_COUNT); isc_result_t result = ISC_R_SUCCESS; @@ -355,29 +362,28 @@ isc_rwlock_trylock(isc_rwlock_t *rwl, isc_rwlocktype_t type) { if (type == isc_rwlocktype_read) { /* If a writer is waiting or working, we fail. */ - if (atomic_load_explicit(&rwl->write_requests, memory_order_relaxed) != - atomic_load_explicit(&rwl->write_completions, memory_order_relaxed)) + if (atomic_load_relaxed(&rwl->write_requests) != + atomic_load_relaxed(&rwl->write_completions)) return (ISC_R_LOCKBUSY); /* Otherwise, be ready for reading. */ - cntflag = atomic_fetch_add_explicit(&rwl->cnt_and_flag, - READER_INCR, - memory_order_relaxed); + cntflag = atomic_fetch_add_relaxed(&rwl->cnt_and_flag, + READER_INCR); if ((cntflag & WRITER_ACTIVE) != 0) { /* * A writer is working. We lose, and cancel the read * request. */ - cntflag = atomic_fetch_sub_explicit - (&rwl->cnt_and_flag, READER_INCR, - memory_order_relaxed); + cntflag = atomic_fetch_sub_relaxed( + &rwl->cnt_and_flag, READER_INCR); /* * If no other readers are waiting and we've suspended * new writers in this short period, wake them up. */ if (cntflag == READER_INCR && - atomic_load_explicit(&rwl->write_completions, memory_order_relaxed) != - atomic_load_explicit(&rwl->write_requests, memory_order_relaxed)) { + atomic_load_relaxed(&rwl->write_completions) != + atomic_load_relaxed(&rwl->write_requests)) + { LOCK(&rwl->lock); BROADCAST(&rwl->writeable); UNLOCK(&rwl->lock); @@ -388,9 +394,8 @@ isc_rwlock_trylock(isc_rwlock_t *rwl, isc_rwlocktype_t type) { } else { /* Try locking without entering the waiting queue. */ int_fast32_t zero = 0; - if (!atomic_compare_exchange_strong_explicit - (&rwl->cnt_and_flag, &zero, WRITER_ACTIVE, - memory_order_relaxed, memory_order_relaxed)) + if (!atomic_compare_exchange_strong_relaxed( + &rwl->cnt_and_flag, &zero, WRITER_ACTIVE)) { return (ISC_R_LOCKBUSY); } @@ -399,10 +404,8 @@ isc_rwlock_trylock(isc_rwlock_t *rwl, isc_rwlocktype_t type) { * XXXJT: jump into the queue, possibly breaking the writer * order. */ - atomic_fetch_sub_explicit(&rwl->write_completions, 1, - memory_order_relaxed); - - rwl->write_granted++; + atomic_fetch_sub_relaxed(&rwl->write_completions, 1); + atomic_fetch_add_relaxed(&rwl->write_granted, 1); } #ifdef ISC_RWLOCK_TRACE @@ -420,9 +423,8 @@ isc_rwlock_tryupgrade(isc_rwlock_t *rwl) { int_fast32_t reader_incr = READER_INCR; /* Try to acquire write access. */ - atomic_compare_exchange_strong_explicit - (&rwl->cnt_and_flag, &reader_incr, WRITER_ACTIVE, - memory_order_relaxed, memory_order_relaxed); + atomic_compare_exchange_strong_relaxed( + &rwl->cnt_and_flag, &reader_incr, WRITER_ACTIVE); /* * There must have been no writer, and there must have * been at least one reader. @@ -435,8 +437,7 @@ isc_rwlock_tryupgrade(isc_rwlock_t *rwl) { * We are the only reader and have been upgraded. * Now jump into the head of the writer waiting queue. */ - atomic_fetch_sub_explicit(&rwl->write_completions, 1, - memory_order_relaxed); + atomic_fetch_sub_relaxed(&rwl->write_completions, 1); } else return (ISC_R_LOCKBUSY); @@ -453,17 +454,14 @@ isc_rwlock_downgrade(isc_rwlock_t *rwl) { { /* Become an active reader. */ - prev_readers = atomic_fetch_add_explicit(&rwl->cnt_and_flag, - READER_INCR, - memory_order_relaxed); + prev_readers = atomic_fetch_add_relaxed(&rwl->cnt_and_flag, + READER_INCR); /* We must have been a writer. */ INSIST((prev_readers & WRITER_ACTIVE) != 0); /* Complete write */ - atomic_fetch_sub_explicit(&rwl->cnt_and_flag, WRITER_ACTIVE, - memory_order_relaxed); - atomic_fetch_add_explicit(&rwl->write_completions, 1, - memory_order_relaxed); + atomic_fetch_sub_relaxed(&rwl->cnt_and_flag, WRITER_ACTIVE); + atomic_fetch_add_relaxed(&rwl->write_completions, 1); } /* Resume other readers */ @@ -484,17 +482,16 @@ isc_rwlock_unlock(isc_rwlock_t *rwl, isc_rwlocktype_t type) { #endif if (type == isc_rwlocktype_read) { - prev_cnt = atomic_fetch_sub_explicit(&rwl->cnt_and_flag, - READER_INCR, - memory_order_relaxed); + prev_cnt = atomic_fetch_sub_relaxed(&rwl->cnt_and_flag, + READER_INCR); /* * If we're the last reader and any writers are waiting, wake * them up. We need to wake up all of them to ensure the * FIFO order. */ if (prev_cnt == READER_INCR && - atomic_load_explicit(&rwl->write_completions, memory_order_relaxed) != - atomic_load_explicit(&rwl->write_requests, memory_order_relaxed)) { + atomic_load_relaxed(&rwl->write_completions) != + atomic_load_relaxed(&rwl->write_requests)) { LOCK(&rwl->lock); BROADCAST(&rwl->writeable); UNLOCK(&rwl->lock); @@ -506,15 +503,15 @@ isc_rwlock_unlock(isc_rwlock_t *rwl, isc_rwlocktype_t type) { * Reset the flag, and (implicitly) tell other writers * we are done. */ - atomic_fetch_sub_explicit(&rwl->cnt_and_flag, WRITER_ACTIVE, - memory_order_relaxed); - atomic_fetch_add_explicit(&rwl->write_completions, 1, - memory_order_relaxed); + atomic_fetch_sub_relaxed(&rwl->cnt_and_flag, WRITER_ACTIVE); + atomic_fetch_add_relaxed(&rwl->write_completions, 1); if (rwl->write_granted >= rwl->write_quota || - (atomic_load_explicit(&rwl->write_requests, memory_order_relaxed) == - atomic_load_explicit(&rwl->write_completions, memory_order_relaxed)) || - (atomic_load_explicit(&rwl->cnt_and_flag, memory_order_relaxed) & ~WRITER_ACTIVE)) { + (atomic_load_relaxed(&rwl->write_requests) == + atomic_load_relaxed(&rwl->write_completions)) || + (atomic_load_relaxed(&rwl->cnt_and_flag) + & ~WRITER_ACTIVE)) + { /* * We have passed the write quota, no writer is * waiting, or some readers are almost ready, pending @@ -531,8 +528,8 @@ isc_rwlock_unlock(isc_rwlock_t *rwl, isc_rwlocktype_t type) { UNLOCK(&rwl->lock); } - if ((atomic_load_explicit(&rwl->write_requests, memory_order_relaxed) != - atomic_load_explicit(&rwl->write_completions, memory_order_relaxed)) && + if ((atomic_load_relaxed(&rwl->write_requests) != + atomic_load_relaxed(&rwl->write_completions)) && wakeup_writers) { LOCK(&rwl->lock); BROADCAST(&rwl->writeable);