diff options
author | Pauli <pauli@openssl.org> | 2022-01-12 14:45:07 +1100 |
---|---|---|
committer | Pauli <pauli@openssl.org> | 2022-01-13 21:46:34 +1100 |
commit | 43f132778b138870120d965f2fb61aa7411b78b2 (patch) | |
tree | e6004ea6de1f485f7c32b8f79cf0c89244a1bb4f /crypto/lhash | |
parent | 5c41cee225094e6298799b709278b0431643fb1f (diff) |
lhash: use lock when TSAN not available for statistics gathering
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/17479)
Diffstat (limited to 'crypto/lhash')
-rw-r--r-- | crypto/lhash/lh_stats.c | 25 | ||||
-rw-r--r-- | crypto/lhash/lhash.c | 55 | ||||
-rw-r--r-- | crypto/lhash/lhash_local.h | 3 |
3 files changed, 66 insertions, 17 deletions
diff --git a/crypto/lhash/lh_stats.c b/crypto/lhash/lh_stats.c index 5e38c42580..0d4bc72608 100644 --- a/crypto/lhash/lh_stats.c +++ b/crypto/lhash/lh_stats.c @@ -61,6 +61,14 @@ void OPENSSL_LH_node_usage_stats(const OPENSSL_LHASH *lh, FILE *fp) void OPENSSL_LH_stats_bio(const OPENSSL_LHASH *lh, BIO *out) { + int omit_tsan = 0; + +#ifdef TSAN_REQUIRES_LOCKING + if (!CRYPTO_THREAD_read_lock(lh->tsan_lock)) { + BIO_printf(out, "unable to lock table, omitting TSAN counters\n"); + omit_tsan = 1; + } +#endif BIO_printf(out, "num_items = %lu\n", lh->num_items); BIO_printf(out, "num_nodes = %u\n", lh->num_nodes); BIO_printf(out, "num_alloc_nodes = %u\n", lh->num_alloc_nodes); @@ -68,15 +76,22 @@ void OPENSSL_LH_stats_bio(const OPENSSL_LHASH *lh, BIO *out) BIO_printf(out, "num_expand_reallocs = %lu\n", lh->num_expand_reallocs); BIO_printf(out, "num_contracts = %lu\n", lh->num_contracts); BIO_printf(out, "num_contract_reallocs = %lu\n", lh->num_contract_reallocs); - BIO_printf(out, "num_hash_calls = %lu\n", lh->num_hash_calls); - BIO_printf(out, "num_comp_calls = %lu\n", lh->num_comp_calls); + if (!omit_tsan) { + BIO_printf(out, "num_hash_calls = %lu\n", lh->num_hash_calls); + BIO_printf(out, "num_comp_calls = %lu\n", lh->num_comp_calls); + } BIO_printf(out, "num_insert = %lu\n", lh->num_insert); BIO_printf(out, "num_replace = %lu\n", lh->num_replace); BIO_printf(out, "num_delete = %lu\n", lh->num_delete); BIO_printf(out, "num_no_delete = %lu\n", lh->num_no_delete); - BIO_printf(out, "num_retrieve = %lu\n", lh->num_retrieve); - BIO_printf(out, "num_retrieve_miss = %lu\n", lh->num_retrieve_miss); - BIO_printf(out, "num_hash_comps = %lu\n", lh->num_hash_comps); + if (!omit_tsan) { + BIO_printf(out, "num_retrieve = %lu\n", lh->num_retrieve); + BIO_printf(out, "num_retrieve_miss = %lu\n", lh->num_retrieve_miss); + BIO_printf(out, "num_hash_comps = %lu\n", lh->num_hash_comps); +#ifdef TSAN_REQUIRES_LOCKING + CRYPTO_THREAD_unlock(lh->tsan_lock); +#endif + } } void OPENSSL_LH_node_stats_bio(const OPENSSL_LHASH *lh, BIO *out) diff --git a/crypto/lhash/lhash.c b/crypto/lhash/lhash.c index b2fa4c518b..62e2be4c18 100644 --- a/crypto/lhash/lhash.c +++ b/crypto/lhash/lhash.c @@ -44,6 +44,22 @@ static int expand(OPENSSL_LHASH *lh); static void contract(OPENSSL_LHASH *lh); static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh, const void *data, unsigned long *rhash); +static ossl_inline int tsan_lock(const OPENSSL_LHASH *lh) +{ +#ifdef TSAN_REQUIRES_LOCKING + if (!CRYPTO_THREAD_write_lock(lh->tsan_lock)) + return 0; +#endif + return 1; +} + +static ossl_inline void tsan_unlock(const OPENSSL_LHASH *lh) +{ +#ifdef TSAN_REQUIRES_LOCKING + CRYPTO_THREAD_unlock(lh->tsan_lock); +#endif +} + OPENSSL_LHASH *OPENSSL_LH_new(OPENSSL_LH_HASHFUNC h, OPENSSL_LH_COMPFUNC c) { OPENSSL_LHASH *ret; @@ -58,6 +74,10 @@ OPENSSL_LHASH *OPENSSL_LH_new(OPENSSL_LH_HASHFUNC h, OPENSSL_LH_COMPFUNC c) } if ((ret->b = OPENSSL_zalloc(sizeof(*ret->b) * MIN_NODES)) == NULL) goto err; +#ifdef TSAN_REQUIRES_LOCKING + if ((ret->tsan_lock = CRYPTO_THREAD_lock_new()) == NULL) + goto err; +#endif ret->comp = ((c == NULL) ? (OPENSSL_LH_COMPFUNC)strcmp : c); ret->hash = ((h == NULL) ? (OPENSSL_LH_HASHFUNC)OPENSSL_LH_strhash : h); ret->num_nodes = MIN_NODES / 2; @@ -79,6 +99,9 @@ void OPENSSL_LH_free(OPENSSL_LHASH *lh) return; OPENSSL_LH_flush(lh); +#ifdef TSAN_REQUIRES_LOCKING + CRYPTO_THREAD_lock_free(lh->tsan_lock); +#endif OPENSSL_free(lh->b); OPENSSL_free(lh); } @@ -166,21 +189,20 @@ void *OPENSSL_LH_retrieve(OPENSSL_LHASH *lh, const void *data) { unsigned long hash; OPENSSL_LH_NODE **rn; - void *ret; + /*- + * This should be atomic without tsan. + * It's not clear why it was done this way and not elsewhere. + */ tsan_store((TSAN_QUALIFIER int *)&lh->error, 0); rn = getrn(lh, data, &hash); - if (*rn == NULL) { - tsan_counter(&lh->num_retrieve_miss); - return NULL; - } else { - ret = (*rn)->data; - tsan_counter(&lh->num_retrieve); + if (tsan_lock(lh)) { + tsan_counter(*rn == NULL ? &lh->num_retrieve_miss : &lh->num_retrieve); + tsan_unlock(lh); } - - return ret; + return *rn == NULL ? NULL : (*rn)->data; } static void doall_util_fn(OPENSSL_LHASH *lh, int use_arg, @@ -307,9 +329,14 @@ static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh, OPENSSL_LH_NODE **ret, *n1; unsigned long hash, nn; OPENSSL_LH_COMPFUNC cf; + int do_tsan = 1; +#ifdef TSAN_REQUIRES_LOCKING + do_tsan = tsan_lock(lh); +#endif hash = (*(lh->hash)) (data); - tsan_counter(&lh->num_hash_calls); + if (do_tsan) + tsan_counter(&lh->num_hash_calls); *rhash = hash; nn = hash % lh->pmax; @@ -319,16 +346,20 @@ static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh, cf = lh->comp; ret = &(lh->b[(int)nn]); for (n1 = *ret; n1 != NULL; n1 = n1->next) { - tsan_counter(&lh->num_hash_comps); + if (do_tsan) + tsan_counter(&lh->num_hash_comps); if (n1->hash != hash) { ret = &(n1->next); continue; } - tsan_counter(&lh->num_comp_calls); + if (do_tsan) + tsan_counter(&lh->num_comp_calls); if (cf(n1->data, data) == 0) break; ret = &(n1->next); } + if (do_tsan) + tsan_unlock(lh); return ret; } diff --git a/crypto/lhash/lhash_local.h b/crypto/lhash/lhash_local.h index ad9dd4d346..35c0c5b49c 100644 --- a/crypto/lhash/lhash_local.h +++ b/crypto/lhash/lhash_local.h @@ -41,4 +41,7 @@ struct lhash_st { TSAN_QUALIFIER unsigned long num_retrieve_miss; TSAN_QUALIFIER unsigned long num_hash_comps; int error; +#ifdef TSAN_REQUIRES_LOCKING + CRYPTO_RWLOCK *tsan_lock; +#endif }; |