From: Michał Kępień Date: Wed, 13 Jul 2022 11:19:32 +0000 (+0200) Subject: Improve reporting for read-write lock errors X-Git-Tag: v9.19.4~31^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7009f9d270964824893bdbd2503b6ad2f6fb6154;p=thirdparty%2Fbind9.git Improve reporting for read-write lock errors Replace direct uses of implementation-specific rwlock functions in lib/isc/include/isc/rwlock.h with preprocessor macros that use ERRNO_CHECK(), in order to augment rwlock-related error messages with file/line/caller information and the error string corresponding to errno. Adjust the implementation-specific functions for pthreads-based rwlocks so that they return any errors encountered to the caller instead of aborting execution immediately using RUNTIME_CHECK(). To keep code modifications simple, make the non-pthreads-based implementation-specific rwlock functions always return 0; these functions continue to handle errors using less verbose run-time assertions as they do not set errno anyway. --- diff --git a/lib/isc/include/isc/rwlock.h b/lib/isc/include/isc/rwlock.h index b9682e051a6..b6d5232f523 100644 --- a/lib/isc/include/isc/rwlock.h +++ b/lib/isc/include/isc/rwlock.h @@ -50,11 +50,11 @@ typedef struct isc_rwlock isc__rwlock_t; *rwl = malloc(sizeof(**rwl)); \ isc__rwlock_init(*rwl, rq, wq); \ } -#define isc_rwlock_lock(rwl, type) isc___rwlock_lock(*rwl, type) +#define isc_rwlock_lock(rwl, type) isc__rwlock_lock(*rwl, type) #define isc_rwlock_trylock(rwl, type) isc___rwlock_trylock(*rwl, type) -#define isc_rwlock_unlock(rwl, type) isc___rwlock_unlock(*rwl, type) +#define isc_rwlock_unlock(rwl, type) isc__rwlock_unlock(*rwl, type) #define isc_rwlock_tryupgrade(rwl) isc___rwlock_tryupgrade(*rwl) -#define isc_rwlock_downgrade(rwl) isc___rwlock_downgrade(*rwl) +#define isc_rwlock_downgrade(rwl) isc__rwlock_downgrade(*rwl) #define isc_rwlock_destroy(rwl) \ { \ isc___rwlock_destroy(*rwl); \ @@ -67,12 +67,12 @@ typedef struct isc_rwlock isc_rwlock_t; typedef struct isc_rwlock isc__rwlock_t; #define isc_rwlock_init(rwl, rq, wq) isc__rwlock_init(rwl, rq, wq) -#define isc_rwlock_lock(rwl, type) isc___rwlock_lock(rwl, type) +#define isc_rwlock_lock(rwl, type) isc__rwlock_lock(rwl, type) #define isc_rwlock_trylock(rwl, type) isc___rwlock_trylock(rwl, type) -#define isc_rwlock_unlock(rwl, type) isc___rwlock_unlock(rwl, type) +#define isc_rwlock_unlock(rwl, type) isc__rwlock_unlock(rwl, type) #define isc_rwlock_tryupgrade(rwl) isc___rwlock_tryupgrade(rwl) -#define isc_rwlock_downgrade(rwl) isc___rwlock_downgrade(rwl) -#define isc_rwlock_destroy(rwl) isc___rwlock_destroy(rwl) +#define isc_rwlock_downgrade(rwl) isc__rwlock_downgrade(rwl) +#define isc_rwlock_destroy(rwl) isc__rwlock_destroy(rwl) #endif /* ISC_TRACK_PTHREADS_OBJECTS */ @@ -118,12 +118,12 @@ typedef struct isc_rwlock isc_rwlock_t; typedef struct isc_rwlock isc__rwlock_t; #define isc_rwlock_init(rwl, rq, wq) isc__rwlock_init(rwl, rq, wq) -#define isc_rwlock_lock(rwl, type) isc___rwlock_lock(rwl, type) +#define isc_rwlock_lock(rwl, type) isc__rwlock_lock(rwl, type) #define isc_rwlock_trylock(rwl, type) isc___rwlock_trylock(rwl, type) -#define isc_rwlock_unlock(rwl, type) isc___rwlock_unlock(rwl, type) +#define isc_rwlock_unlock(rwl, type) isc__rwlock_unlock(rwl, type) #define isc_rwlock_tryupgrade(rwl) isc___rwlock_tryupgrade(rwl) -#define isc_rwlock_downgrade(rwl) isc___rwlock_downgrade(rwl) -#define isc_rwlock_destroy(rwl) isc___rwlock_destroy(rwl) +#define isc_rwlock_downgrade(rwl) isc__rwlock_downgrade(rwl) +#define isc_rwlock_destroy(rwl) isc__rwlock_destroy(rwl) #endif /* USE_PTHREAD_RWLOCK */ @@ -133,26 +133,50 @@ typedef struct isc_rwlock isc__rwlock_t; ERRNO_CHECK(isc___rwlock_init, _ret); \ } +#define isc__rwlock_lock(rwl, type) \ + { \ + int _ret = isc___rwlock_lock(rwl, type); \ + ERRNO_CHECK(isc___rwlock_lock, _ret); \ + } + +#define isc__rwlock_unlock(rwl, type) \ + { \ + int _ret = isc___rwlock_unlock(rwl, type); \ + ERRNO_CHECK(isc___rwlock_unlock, _ret); \ + } + +#define isc__rwlock_downgrade(rwl) \ + { \ + int _ret = isc___rwlock_downgrade(rwl); \ + ERRNO_CHECK(isc___rwlock_downgrade, _ret); \ + } + +#define isc__rwlock_destroy(rwl) \ + { \ + int _ret = isc___rwlock_destroy(rwl); \ + ERRNO_CHECK(isc___rwlock_destroy, _ret); \ + } + int isc___rwlock_init(isc__rwlock_t *rwl, unsigned int read_quota, unsigned int write_quota); -void +int isc___rwlock_lock(isc__rwlock_t *rwl, isc_rwlocktype_t type); isc_result_t isc___rwlock_trylock(isc__rwlock_t *rwl, isc_rwlocktype_t type); -void +int isc___rwlock_unlock(isc__rwlock_t *rwl, isc_rwlocktype_t type); isc_result_t isc___rwlock_tryupgrade(isc__rwlock_t *rwl); -void +int isc___rwlock_downgrade(isc__rwlock_t *rwl); -void +int isc___rwlock_destroy(isc__rwlock_t *rwl); ISC_LANG_ENDDECLS diff --git a/lib/isc/rwlock.c b/lib/isc/rwlock.c index 511c88c4f57..ec3afa74d0e 100644 --- a/lib/isc/rwlock.c +++ b/lib/isc/rwlock.c @@ -46,26 +46,32 @@ isc___rwlock_init(isc__rwlock_t *rwl, unsigned int read_quota, return (ret); } -void +int isc___rwlock_lock(isc__rwlock_t *rwl, isc_rwlocktype_t type) { + int ret; + switch (type) { case isc_rwlocktype_read: - RUNTIME_CHECK(pthread_rwlock_rdlock(&rwl->rwlock) == 0); - break; + return (pthread_rwlock_rdlock(&rwl->rwlock)); case isc_rwlocktype_write: while (true) { - RUNTIME_CHECK(pthread_rwlock_wrlock(&rwl->rwlock) == 0); + ret = pthread_rwlock_wrlock(&rwl->rwlock); + if (ret != 0) { + return (ret); + } /* Unlock if in middle of downgrade operation */ if (atomic_load_acquire(&rwl->downgrade)) { - RUNTIME_CHECK(pthread_rwlock_unlock( - &rwl->rwlock) == 0); + ret = pthread_rwlock_unlock(&rwl->rwlock); + if (ret != 0) { + return (ret); + } while (atomic_load_acquire(&rwl->downgrade)) { } continue; } break; } - break; + return (0); default: UNREACHABLE(); } @@ -101,10 +107,10 @@ isc___rwlock_trylock(isc__rwlock_t *rwl, isc_rwlocktype_t type) { } } -void +int isc___rwlock_unlock(isc__rwlock_t *rwl, isc_rwlocktype_t type) { UNUSED(type); - RUNTIME_CHECK(pthread_rwlock_unlock(&rwl->rwlock) == 0); + return (pthread_rwlock_unlock(&rwl->rwlock)); } isc_result_t @@ -113,17 +119,30 @@ isc___rwlock_tryupgrade(isc__rwlock_t *rwl) { return (ISC_R_LOCKBUSY); } -void +int isc___rwlock_downgrade(isc__rwlock_t *rwl) { + int ret; + atomic_store_release(&rwl->downgrade, true); - RUNTIME_CHECK(pthread_rwlock_unlock(&rwl->rwlock) == 0); - RUNTIME_CHECK(pthread_rwlock_rdlock(&rwl->rwlock) == 0); + + ret = pthread_rwlock_unlock(&rwl->rwlock); + if (ret != 0) { + return (ret); + } + + ret = pthread_rwlock_rdlock(&rwl->rwlock); + if (ret != 0) { + return (ret); + } + atomic_store_release(&rwl->downgrade, false); + + return (0); } -void +int isc___rwlock_destroy(isc__rwlock_t *rwl) { - pthread_rwlock_destroy(&rwl->rwlock); + return (pthread_rwlock_destroy(&rwl->rwlock)); } #else /* if USE_PTHREAD_RWLOCK */ @@ -223,7 +242,7 @@ isc___rwlock_init(isc__rwlock_t *rwl, unsigned int read_quota, return (0); } -void +int isc___rwlock_destroy(isc__rwlock_t *rwl) { REQUIRE(VALID_RWLOCK(rwl)); @@ -236,6 +255,8 @@ isc___rwlock_destroy(isc__rwlock_t *rwl) { isc_condition_destroy(&rwl->readable); isc_condition_destroy(&rwl->writeable); isc_mutex_destroy(&rwl->lock); + + return (0); } /* @@ -420,7 +441,7 @@ rwlock_lock(isc__rwlock_t *rwl, isc_rwlocktype_t type) { #endif /* ifdef ISC_RWLOCK_TRACE */ } -void +int 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; @@ -435,6 +456,8 @@ isc___rwlock_lock(isc__rwlock_t *rwl, isc_rwlocktype_t type) { } while (isc_rwlock_trylock(rwl, type) != ISC_R_SUCCESS); atomic_fetch_add_release(&rwl->spins, (cnt - spins) / 8); + + return (0); } isc_result_t @@ -533,7 +556,7 @@ isc___rwlock_tryupgrade(isc__rwlock_t *rwl) { return (ISC_R_SUCCESS); } -void +int isc___rwlock_downgrade(isc__rwlock_t *rwl) { int32_t prev_readers; @@ -555,9 +578,11 @@ isc___rwlock_downgrade(isc__rwlock_t *rwl) { BROADCAST(&rwl->readable); } UNLOCK(&rwl->lock); + + return (0); } -void +int isc___rwlock_unlock(isc__rwlock_t *rwl, isc_rwlocktype_t type) { int32_t prev_cnt; @@ -628,6 +653,8 @@ isc___rwlock_unlock(isc__rwlock_t *rwl, isc_rwlocktype_t type) { #ifdef ISC_RWLOCK_TRACE print_lock("postunlock", rwl, type); #endif /* ifdef ISC_RWLOCK_TRACE */ + + return (0); } #endif /* USE_PTHREAD_RWLOCK */