From 77d7b6eebb411fdb2c3d1390ac779300757aa9dc Mon Sep 17 00:00:00 2001 From: Hugo Landau Date: Tue, 22 Mar 2022 10:59:36 +0000 Subject: Remove statistics tracking from LHASH Fixes #17928. Supercedes #17931. Reviewed-by: Tim Hudson Reviewed-by: Paul Dale Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/17935) --- crypto/lhash/lh_stats.c | 41 +++++++++++------------------------ crypto/lhash/lhash.c | 54 ++-------------------------------------------- crypto/lhash/lhash_local.h | 16 -------------- 3 files changed, 15 insertions(+), 96 deletions(-) (limited to 'crypto/lhash') diff --git a/crypto/lhash/lh_stats.c b/crypto/lhash/lh_stats.c index 0d4bc72608..8a49bfab34 100644 --- a/crypto/lhash/lh_stats.c +++ b/crypto/lhash/lh_stats.c @@ -61,37 +61,22 @@ 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); - BIO_printf(out, "num_expands = %lu\n", lh->num_expands); - 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); - 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); - 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 - } + BIO_printf(out, "num_expands = 0\n"); + BIO_printf(out, "num_expand_reallocs = 0\n"); + BIO_printf(out, "num_contracts = 0\n"); + BIO_printf(out, "num_contract_reallocs = 0\n"); + BIO_printf(out, "num_hash_calls = 0\n"); + BIO_printf(out, "num_comp_calls = 0\n"); + BIO_printf(out, "num_insert = 0\n"); + BIO_printf(out, "num_replace = 0\n"); + BIO_printf(out, "num_delete = 0\n"); + BIO_printf(out, "num_no_delete = 0\n"); + BIO_printf(out, "num_retrieve = 0\n"); + BIO_printf(out, "num_retrieve_miss = 0\n"); + BIO_printf(out, "num_hash_comps = 0\n"); } 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 b99b63fd6a..2a574fbfe6 100644 --- a/crypto/lhash/lhash.c +++ b/crypto/lhash/lhash.c @@ -44,22 +44,6 @@ 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; @@ -74,10 +58,6 @@ 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; @@ -99,9 +79,6 @@ 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); } @@ -147,12 +124,10 @@ void *OPENSSL_LH_insert(OPENSSL_LHASH *lh, void *data) nn->hash = hash; *rn = nn; ret = NULL; - lh->num_insert++; lh->num_items++; } else { /* replace same key */ ret = (*rn)->data; (*rn)->data = data; - lh->num_replace++; } return ret; } @@ -167,14 +142,12 @@ void *OPENSSL_LH_delete(OPENSSL_LHASH *lh, const void *data) rn = getrn(lh, data, &hash); if (*rn == NULL) { - lh->num_no_delete++; return NULL; } else { nn = *rn; *rn = nn->next; ret = nn->data; OPENSSL_free(nn); - lh->num_delete++; } lh->num_items--; @@ -190,18 +163,11 @@ void *OPENSSL_LH_retrieve(OPENSSL_LHASH *lh, const void *data) unsigned long hash; OPENSSL_LH_NODE **rn; - /*- - * 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); + if (lh->error != 0) + lh->error = 0; rn = getrn(lh, data, &hash); - if (tsan_lock(lh)) { - tsan_counter(*rn == NULL ? &lh->num_retrieve_miss : &lh->num_retrieve); - tsan_unlock(lh); - } return *rn == NULL ? NULL : (*rn)->data; } @@ -262,14 +228,12 @@ static int expand(OPENSSL_LHASH *lh) memset(n + nni, 0, sizeof(*n) * (j - nni)); lh->pmax = nni; lh->num_alloc_nodes = j; - lh->num_expand_reallocs++; lh->p = 0; } else { lh->p++; } lh->num_nodes++; - lh->num_expands++; n1 = &(lh->b[p]); n2 = &(lh->b[p + pmax]); *n2 = NULL; @@ -302,7 +266,6 @@ static void contract(OPENSSL_LHASH *lh) lh->error++; return; } - lh->num_contract_reallocs++; lh->num_alloc_nodes /= 2; lh->pmax /= 2; lh->p = lh->pmax - 1; @@ -311,7 +274,6 @@ static void contract(OPENSSL_LHASH *lh) lh->p--; lh->num_nodes--; - lh->num_contracts++; n1 = lh->b[(int)lh->p]; if (n1 == NULL) @@ -329,14 +291,8 @@ 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); - if (do_tsan) - tsan_counter(&lh->num_hash_calls); *rhash = hash; nn = hash % lh->pmax; @@ -346,20 +302,14 @@ static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh, cf = lh->comp; ret = &(lh->b[(int)nn]); for (n1 = *ret; n1 != NULL; n1 = n1->next) { - if (do_tsan) - tsan_counter(&lh->num_hash_comps); if (n1->hash != hash) { ret = &(n1->next); continue; } - 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 35c0c5b49c..5edd8e6c66 100644 --- a/crypto/lhash/lhash_local.h +++ b/crypto/lhash/lhash_local.h @@ -27,21 +27,5 @@ struct lhash_st { unsigned long up_load; /* load times 256 */ unsigned long down_load; /* load times 256 */ unsigned long num_items; - unsigned long num_expands; - unsigned long num_expand_reallocs; - unsigned long num_contracts; - unsigned long num_contract_reallocs; - TSAN_QUALIFIER unsigned long num_hash_calls; - TSAN_QUALIFIER unsigned long num_comp_calls; - unsigned long num_insert; - unsigned long num_replace; - unsigned long num_delete; - unsigned long num_no_delete; - TSAN_QUALIFIER unsigned long num_retrieve; - 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 }; -- cgit v1.2.3