diff --git a/configure.ac b/configure.ac index 6ae0314b0..699933644 100644 --- a/configure.ac +++ b/configure.ac @@ -667,21 +667,39 @@ AC_COMPILE_IFELSE( [AC_MSG_RESULT([no])] ) -AC_MSG_CHECKING([for gcc atomic operations]) +AC_MSG_CHECKING([for GCC __atomic operations]) AC_RUN_IFELSE([AC_LANG_SOURCE( [[ int main() { - volatile int ref = 1; - __sync_fetch_and_add (&ref, 1); - __sync_sub_and_fetch (&ref, 1); - /* Make sure test fails if operations are not supported */ - __sync_val_compare_and_swap(&ref, 1, 0); + int ref = 1, val; + __atomic_fetch_add(&ref, 1, __ATOMIC_RELAXED); + val = __atomic_sub_fetch(&ref, 1, __ATOMIC_RELAXED); + __atomic_compare_exchange_n(&ref, &val, 0, 0, __ATOMIC_RELAXED, + __ATOMIC_RELAXED); return ref; } ]])], [AC_MSG_RESULT([yes]); AC_DEFINE([HAVE_GCC_ATOMIC_OPERATIONS], [], - [have GCC __sync_* atomic operations])], + [have GCC __atomic_* operations])], + [AC_MSG_RESULT([no])], + [AC_MSG_RESULT([no])] +) + +AC_MSG_CHECKING([for GCC __sync operations]) +AC_RUN_IFELSE([AC_LANG_SOURCE( + [[ + int main() { + int ref = 1; + __sync_fetch_and_add (&ref, 1); + __sync_sub_and_fetch (&ref, 1); + __sync_val_compare_and_swap(&ref, 1, 0); + return ref; + } + ]])], + [AC_MSG_RESULT([yes]); + AC_DEFINE([HAVE_GCC_SYNC_OPERATIONS], [], + [have GCC __sync_* operations])], [AC_MSG_RESULT([no])], [AC_MSG_RESULT([no])] ) diff --git a/src/libcharon/bus/bus.c b/src/libcharon/bus/bus.c index b46184809..bc080d1c0 100644 --- a/src/libcharon/bus/bus.c +++ b/src/libcharon/bus/bus.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2011-2012 Tobias Brunner + * Copyright (C) 2011-2014 Tobias Brunner * Copyright (C) 2006 Martin Willi * Hochschule fuer Technik Rapperswil * @@ -23,6 +23,31 @@ #include #include +/** + * These operations allow us to speed up the log level checks on some platforms. + * In particular if acquiring the read lock is expensive even in the absence of + * any writers. + * + * Note that while holding the read/write lock the read does not have to be + * atomic as the write lock must be held to set the level. + */ +#ifdef HAVE_GCC_ATOMIC_OPERATIONS + +#define skip_level(ptr, level) (__atomic_load_n(ptr, __ATOMIC_RELAXED) < level) +#define set_level(ptr, val) __atomic_store_n(ptr, val, __ATOMIC_RELAXED) + +#elif defined(HAVE_GCC_SYNC_OPERATIONS) + +#define skip_level(ptr, level) (__sync_fetch_and_add(ptr, 0) < level) +#define set_level(ptr, val) __sync_bool_compare_and_swap(ptr, *ptr, val) + +#else + +#define skip_level(ptr, level) FALSE +#define set_level(ptr, val) ({ *ptr = val; }) + +#endif + typedef struct private_bus_t private_bus_t; /** @@ -173,11 +198,12 @@ static inline void register_logger(private_bus_t *this, debug_t group, if (entry->logger->log) { - this->max_level[group] = max(this->max_level[group], level); + set_level(&this->max_level[group], max(this->max_level[group], level)); } if (entry->logger->vlog) { - this->max_vlevel[group] = max(this->max_vlevel[group], level); + set_level(&this->max_vlevel[group], + max(this->max_vlevel[group], level)); } } @@ -205,6 +231,7 @@ static inline void unregister_logger(private_bus_t *this, logger_t *logger) if (found) { + level_t level = LEVEL_SILENT, vlevel = LEVEL_SILENT; debug_t group; for (group = 0; group < DBG_MAX; group++) @@ -214,13 +241,19 @@ static inline void unregister_logger(private_bus_t *this, logger_t *logger) loggers = this->loggers[group]; loggers->remove(loggers, found, NULL); - this->max_level[group] = LEVEL_SILENT; - this->max_vlevel[group] = LEVEL_SILENT; if (loggers->get_first(loggers, (void**)&entry) == SUCCESS) { - this->max_level[group] = entry->levels[group]; - this->max_vlevel[group] = entry->levels[group]; + if (entry->logger->log) + { + level = entry->levels[group]; + } + if (entry->logger->vlog) + { + vlevel = entry->levels[group]; + } } + set_level(&this->max_level[group], level); + set_level(&this->max_vlevel[group], vlevel); } } free(found); @@ -324,6 +357,19 @@ METHOD(bus_t, vlog, void, linked_list_t *loggers; log_data_t data; + /* NOTE: This is not 100% thread-safe and done here only because it is + * performance critical. We therefore ignore the following two issues for + * this particular case: 1) We might miss some log messages if another + * thread concurrently increases the log level or registers a new logger. + * 2) We might have to acquire the read lock below even if it wouldn't be + * necessary anymore due to another thread concurrently unregistering a + * logger or reducing the level. */ + if (skip_level(&this->max_level[group], level) && + skip_level(&this->max_vlevel[group], level)) + { + return; + } + this->log_lock->read_lock(this->log_lock); loggers = this->loggers[group]; diff --git a/src/libcharon/plugins/load_tester/load_tester_config.c b/src/libcharon/plugins/load_tester/load_tester_config.c index e133190b4..bc7c0ffbc 100644 --- a/src/libcharon/plugins/load_tester/load_tester_config.c +++ b/src/libcharon/plugins/load_tester/load_tester_config.c @@ -150,7 +150,7 @@ struct private_load_tester_config_t { /** * incremental numbering of generated configs */ - u_int num; + refcount_t num; /** * Dynamic source port, if used @@ -802,7 +802,7 @@ METHOD(backend_t, get_peer_cfg_by_name, peer_cfg_t*, { if (streq(name, "load-test")) { - return generate_config(this, this->num++); + return generate_config(this, (u_int)ref_get(&this->num)); } return NULL; } diff --git a/src/libcharon/plugins/load_tester/load_tester_ipsec.c b/src/libcharon/plugins/load_tester/load_tester_ipsec.c index 02b1d4216..5edd3b82d 100644 --- a/src/libcharon/plugins/load_tester/load_tester_ipsec.c +++ b/src/libcharon/plugins/load_tester/load_tester_ipsec.c @@ -31,14 +31,14 @@ struct private_load_tester_ipsec_t { /** * faked SPI counter */ - u_int32_t spi; + refcount_t spi; }; METHOD(kernel_ipsec_t, get_spi, status_t, private_load_tester_ipsec_t *this, host_t *src, host_t *dst, u_int8_t protocol, u_int32_t reqid, u_int32_t *spi) { - *spi = ++this->spi; + *spi = (uint32_t)ref_get(&this->spi); return SUCCESS; } diff --git a/src/libcharon/sa/ike_sa_manager.c b/src/libcharon/sa/ike_sa_manager.c index f2f81cf33..525117f3b 100644 --- a/src/libcharon/sa/ike_sa_manager.c +++ b/src/libcharon/sa/ike_sa_manager.c @@ -353,6 +353,11 @@ struct private_ike_sa_manager_t { */ shareable_segment_t *half_open_segments; + /** + * Total number of half-open IKE_SAs. + */ + refcount_t half_open_count; + /** * Hash table with connected_peers_t objects. */ @@ -764,6 +769,7 @@ static void put_half_open(private_ike_sa_manager_t *this, entry_t *entry) this->half_open_table[row] = item; } this->half_open_segments[segment].count++; + ref_get(&this->half_open_count); lock->unlock(lock); } @@ -803,6 +809,7 @@ static void remove_half_open(private_ike_sa_manager_t *this, entry_t *entry) free(item); } this->half_open_segments[segment].count--; + ignore_result(ref_put(&this->half_open_count)); break; } prev = item; @@ -1962,13 +1969,7 @@ METHOD(ike_sa_manager_t, get_half_open_count, u_int, } else { - for (segment = 0; segment < this->segment_count; segment++) - { - lock = this->half_open_segments[segment].lock; - lock->read_lock(lock); - count += this->half_open_segments[segment].count; - lock->unlock(lock); - } + count = (u_int)ref_cur(&this->half_open_count); } return count; } diff --git a/src/libstrongswan/utils/utils.c b/src/libstrongswan/utils/utils.c index fe80edb82..f2a4a065c 100644 --- a/src/libstrongswan/utils/utils.c +++ b/src/libstrongswan/utils/utils.c @@ -511,7 +511,7 @@ void nop() { } -#ifndef HAVE_GCC_ATOMIC_OPERATIONS +#if !defined(HAVE_GCC_ATOMIC_OPERATIONS) && !defined(HAVE_GCC_SYNC_OPERATIONS) /** * We use a single mutex for all refcount variables. @@ -528,7 +528,6 @@ refcount_t ref_get(refcount_t *ref) pthread_mutex_lock(&ref_mutex); current = ++(*ref); pthread_mutex_unlock(&ref_mutex); - return current; } @@ -545,6 +544,19 @@ bool ref_put(refcount_t *ref) return !more_refs; } +/** + * Current refcount + */ +refcount_t ref_cur(refcount_t *ref) +{ + refcount_t current; + + pthread_mutex_lock(&ref_mutex); + current = *ref; + pthread_mutex_unlock(&ref_mutex); + return current; +} + /** * Single mutex for all compare and swap operations. */ @@ -566,7 +578,7 @@ bool cas_##name(type *ptr, type oldval, type newval) \ _cas_impl(bool, bool) _cas_impl(ptr, void*) -#endif /* HAVE_GCC_ATOMIC_OPERATIONS */ +#endif /* !HAVE_GCC_ATOMIC_OPERATIONS && !HAVE_GCC_SYNC_OPERATIONS */ #ifdef HAVE_FMEMOPEN_FALLBACK diff --git a/src/libstrongswan/utils/utils.h b/src/libstrongswan/utils/utils.h index a55e7d831..8f91e8431 100644 --- a/src/libstrongswan/utils/utils.h +++ b/src/libstrongswan/utils/utils.h @@ -750,20 +750,40 @@ typedef u_int refcount_t; #ifdef HAVE_GCC_ATOMIC_OPERATIONS +#define ref_get(ref) __atomic_add_fetch(ref, 1, __ATOMIC_RELAXED) +/* The relaxed memory model works fine for increments as these (usually) don't + * change the state of refcounted objects. But here we have to ensure that we + * free the right stuff if ref counted objects are mutable. So we have to sync + * with other threads that call ref_put(). It would be sufficient to use + * __ATOMIC_RELEASE here and then call __atomic_thread_fence() with + * __ATOMIC_ACQUIRE if we reach 0, but since we don't have control over the use + * of ref_put() we have to make sure. */ +#define ref_put(ref) (!__atomic_sub_fetch(ref, 1, __ATOMIC_ACQ_REL)) +#define ref_cur(ref) __atomic_load_n(ref, __ATOMIC_RELAXED) + +#define _cas_impl(ptr, oldval, newval) ({ typeof(oldval) _old = oldval; \ + __atomic_compare_exchange_n(ptr, &_old, newval, FALSE, \ + __ATOMIC_SEQ_CST, __ATOMIC_RELAXED); }) +#define cas_bool(ptr, oldval, newval) _cas_impl(ptr, oldval, newval) +#define cas_ptr(ptr, oldval, newval) _cas_impl(ptr, oldval, newval) + +#elif defined(HAVE_GCC_SYNC_OPERATIONS) + #define ref_get(ref) __sync_add_and_fetch(ref, 1) #define ref_put(ref) (!__sync_sub_and_fetch(ref, 1)) +#define ref_cur(ref) __sync_fetch_and_add(ref, 0) #define cas_bool(ptr, oldval, newval) \ (__sync_bool_compare_and_swap(ptr, oldval, newval)) #define cas_ptr(ptr, oldval, newval) \ (__sync_bool_compare_and_swap(ptr, oldval, newval)) -#else /* !HAVE_GCC_ATOMIC_OPERATIONS */ +#else /* !HAVE_GCC_ATOMIC_OPERATIONS && !HAVE_GCC_SYNC_OPERATIONS */ /** * Get a new reference. * - * Increments the reference counter atomic. + * Increments the reference counter atomically. * * @param ref pointer to ref counter * @return new value of ref @@ -773,7 +793,7 @@ refcount_t ref_get(refcount_t *ref); /** * Put back a unused reference. * - * Decrements the reference counter atomic and + * Decrements the reference counter atomically and * says if more references available. * * @param ref pointer to ref counter @@ -781,6 +801,14 @@ refcount_t ref_get(refcount_t *ref); */ bool ref_put(refcount_t *ref); +/** + * Get the current value of the reference counter. + * + * @param ref pointer to ref counter + * @return current value of ref + */ +refcount_t ref_cur(refcount_t *ref); + /** * Atomically replace value of ptr with newval if it currently equals oldval. *