From b53ffa089040d587af132c6c9343659e0a8d7428 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Tue, 18 Feb 2025 18:09:15 +0000 Subject: [PATCH 1/2] com_*_lock: reduce critical section scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reduce the scope of the global com mutex, otherwise every attempt to acquire a reader lock results in contention on this global mutex. Use a per-object-table lock instead. Signed-off-by: Edwin Török --- daemon/com.c | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/daemon/com.c b/daemon/com.c index 55fb450..16a0ce7 100755 --- a/daemon/com.c +++ b/daemon/com.c @@ -115,6 +115,7 @@ typedef struct ha_common_object MTC_U32 in_use; MTC_U32 ref_count; MTC_U32 checksum; // to detect modification by reader + pthread_mutex_t thread_id_record_table_mutex; THREAD_ID_RECORD thread_id_record_table[THREAD_ID_RECORD_NUM]; } HA_COMMON_OBJECT; @@ -537,6 +538,7 @@ set_thread_id_record( clock_gettime(CLOCK_MONOTONIC, &ts); now = tstoms(ts); + pthread_mutex_lock(&object->thread_id_record_table_mutex); switch (lock_state) { case LOCK_STATE_READER_ACQUIREING: case LOCK_STATE_WRITER_ACQUIREING: @@ -551,6 +553,7 @@ set_thread_id_record( object->thread_id_record_table[i].lock_state = lock_state; object->thread_id_record_table[i].thread_id = self; object->thread_id_record_table[i].changed_time = now; + pthread_mutex_unlock(&object->thread_id_record_table_mutex); return; } } @@ -571,12 +574,14 @@ set_thread_id_record( // object->thread_id_record_table[i].lock_state = lock_state; object->thread_id_record_table[i].changed_time = now; + pthread_mutex_unlock(&object->thread_id_record_table_mutex); return; } } log_message(MTC_LOG_WARNING, "COM: thread_id %lu not found in thread_id_record_table.\n", self); break; } + pthread_mutex_unlock(&object->thread_id_record_table_mutex); assert(FALSE); return ; } @@ -755,6 +760,13 @@ com_create( handle = (HA_COMMON_OBJECT_HANDLE_INTERNAL *) *object_handle; object->ref_count ++; handle->object->in_use++; + pthread_ret = pthread_mutex_init(&handle->object->thread_id_record_table_mutex, NULL); + if (pthread_ret != 0) + { + log_internal(MTC_LOG_ERR, "COM: (%s) pthread_mutex_init failed (sys %d).\n", __func__, pthread_ret); + ret = MTC_ERROR_COM_PTHREAD; + goto error_return; + } set_thread_id_record(handle->object, LOCK_STATE_WRITER_ACQUIREING); LEAVE_CS; @@ -770,6 +782,7 @@ com_create( pthread_ret = pthread_rwlock_wrlock(&handle->object->rwlock); if (fist_on("com.pthread")) pthread_ret = FIST_PTHREAD_ERRCODE; + ENTER_CS; set_thread_id_record(handle->object, LOCK_STATE_WRITER_ACQUIRED); @@ -938,6 +951,11 @@ com_close( { log_message(MTC_LOG_WARNING, "COM: pthread_rwlock_destroy failed (sys %d).\n", pthread_ret); } + pthread_ret = pthread_mutex_destroy(&object->thread_id_record_table_mutex); + if (pthread_ret != 0) + { + log_message(MTC_LOG_WARNING, "COM: pthread_destroy failed (sys %d).\n", pthread_ret); + } free_object(object); } error_return: @@ -1098,7 +1116,6 @@ com_writer_lock( HA_COMMON_OBJECT_HANDLE_INTERNAL *handle = object_handle; int pthread_ret; - ENTER_CS; if (!valid_object_handle(handle)) { log_internal(MTC_LOG_ERR, "COM: (%s) invalid handle.\n", __func__); @@ -1106,12 +1123,12 @@ com_writer_lock( ret = MTC_ERROR_COM_INVALID_HANDLE; goto error_return; } + ENTER_CS; handle->object->in_use++; - set_thread_id_record(handle->object, LOCK_STATE_WRITER_ACQUIREING); LEAVE_CS; + set_thread_id_record(handle->object, LOCK_STATE_WRITER_ACQUIREING); pthread_ret = pthread_rwlock_wrlock(&handle->object->rwlock); if (fist_on("com.pthread")) pthread_ret = FIST_PTHREAD_ERRCODE; - ENTER_CS; set_thread_id_record(handle->object, LOCK_STATE_WRITER_ACQUIRED); if (pthread_ret != 0) { @@ -1123,7 +1140,6 @@ com_writer_lock( *buffer = handle->object->buffer; error_return: - LEAVE_CS; if (ret != MTC_SUCCESS) { log_status(ret, NULL); @@ -1158,7 +1174,6 @@ com_writer_unlock( int pthread_ret; HA_COMMON_OBJECT_CALLBACK_LIST_ITEM *c; - ENTER_CS; if (!valid_object_handle(handle)) { log_internal(MTC_LOG_ERR, "COM: (%s) invalid handle.\n", __func__); @@ -1167,6 +1182,7 @@ com_writer_unlock( goto error_return; } + ENTER_CS; for (c = handle->object->callback_list_head; c != NULL; c = c->next) { c->func(c->object_handle, handle->object->buffer); @@ -1177,6 +1193,8 @@ com_writer_unlock( handle->object->in_use--; + LEAVE_CS; + set_thread_id_record(handle->object, LOCK_STATE_NONE); pthread_ret = pthread_rwlock_unlock(&handle->object->rwlock); if (fist_on("com.pthread")) pthread_ret = FIST_PTHREAD_ERRCODE; @@ -1188,7 +1206,6 @@ com_writer_unlock( } error_return: - LEAVE_CS; if (ret != MTC_SUCCESS) { log_status(ret, NULL); @@ -1225,7 +1242,6 @@ com_reader_lock( HA_COMMON_OBJECT_HANDLE_INTERNAL *handle = object_handle; int pthread_ret; - ENTER_CS; if (!valid_object_handle(handle)) { log_internal(MTC_LOG_ERR, "COM: (%s) invalid handle.\n", __func__); @@ -1233,12 +1249,12 @@ com_reader_lock( ret = MTC_ERROR_COM_INVALID_HANDLE; goto error_return; } + ENTER_CS; handle->object->in_use++; - set_thread_id_record(handle->object, LOCK_STATE_READER_ACQUIREING); LEAVE_CS; + set_thread_id_record(handle->object, LOCK_STATE_READER_ACQUIREING); pthread_ret = pthread_rwlock_rdlock(&handle->object->rwlock); if (fist_on("com.pthread")) pthread_ret = FIST_PTHREAD_ERRCODE; - ENTER_CS; set_thread_id_record(handle->object, LOCK_STATE_READER_ACQUIRED); if (pthread_ret != 0) { @@ -1249,7 +1265,6 @@ com_reader_lock( *buffer = handle->object->buffer; error_return: - LEAVE_CS; if (ret != MTC_SUCCESS) { log_status(ret, NULL); @@ -1282,7 +1297,6 @@ com_reader_unlock( HA_COMMON_OBJECT_HANDLE_INTERNAL *handle = object_handle; int pthread_ret; - ENTER_CS; if (!valid_object_handle(handle)) { log_internal(MTC_LOG_ERR, "COM: (%s) invalid handle.\n", __func__); @@ -1291,10 +1305,12 @@ com_reader_unlock( goto error_return; } + ENTER_CS; #ifndef NDEBUG assert(handle->object->checksum == calc_checksum_object_buffer(handle->object)); #endif //NDEBUG handle->object->in_use--; + LEAVE_CS; set_thread_id_record(handle->object, LOCK_STATE_NONE); pthread_ret = pthread_rwlock_unlock(&handle->object->rwlock); if (fist_on("com.pthread")) pthread_ret = FIST_PTHREAD_ERRCODE; @@ -1306,7 +1322,6 @@ com_reader_unlock( } error_return: - LEAVE_CS; if (ret != MTC_SUCCESS) { log_status(ret, NULL); From c5777e7a33c3512b5456bf4f71cb6d6b490c4245 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Tue, 18 Feb 2025 17:57:38 +0000 Subject: [PATCH 2/2] Use pthread_cond_timewait instead of waking up every 100ms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TODO: use pthread_condattr_setclock and switch to a monotonic clock instead Signed-off-by: Edwin Török --- daemon/sm.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/daemon/sm.c b/daemon/sm.c index 0d5c33c..2cd9e46 100755 --- a/daemon/sm.c +++ b/daemon/sm.c @@ -37,6 +37,7 @@ #define _GNU_SOURCE #include +#include #include #include #include @@ -2819,6 +2820,8 @@ check_sigs( return signaled; } +#define NSEC_PER_SEC 1000000000 + MTC_STATIC MTC_BOOLEAN sm_wait_signals_sm_hb_sf( MTC_BOOLEAN sm_sig, @@ -2827,16 +2830,26 @@ sm_wait_signals_sm_hb_sf( MTC_CLOCK timeout) { MTC_BOOLEAN signaled; - MTC_CLOCK start = _getms(); + + struct timespec deadline; if (timeout == 0) { return FALSE; } + struct timespec timeout_ts = mstots(timeout); + if (clock_gettime(CLOCK_REALTIME, &deadline) < 0) { + log_message(MTC_LOG_WARNING, "clock_gettime failed (sys %d)", errno); + timeout = -1; + } + deadline.tv_nsec += timeout_ts.tv_nsec; + deadline.tv_sec += timeout_ts.tv_sec; + deadline.tv_sec += deadline.tv_nsec / NSEC_PER_SEC; + deadline.tv_nsec %= NSEC_PER_SEC; + pthread_mutex_lock(&smvar.mutex); - while (!(signaled = check_sigs(sm_sig, hb_sig, sf_sig)) && - ((timeout < 0)? TRUE: (_getms() - start < timeout))) + while (!(signaled = check_sigs(sm_sig, hb_sig, sf_sig))) { if (timeout < 0) { @@ -2844,9 +2857,8 @@ sm_wait_signals_sm_hb_sf( } else { - pthread_mutex_unlock(&smvar.mutex); - mssleep(100); - pthread_mutex_lock(&smvar.mutex); + if (pthread_cond_timedwait(&smvar.cond, &smvar.mutex, &deadline) == ETIMEDOUT) + break; } } pthread_mutex_unlock(&smvar.mutex);