From f9fc8932b11f3bcf2a2626f567cb6fdd36a33a94 Mon Sep 17 00:00:00 2001 From: "Longpeng(Mike)" Date: Tue, 22 Feb 2022 17:05:04 +0800 Subject: thread-posix: remove the posix semaphore support POSIX specifies an absolute time for sem_timedwait(), it would be affected if the system time is changing, but there is not a relative time or monotonic clock version of sem_timedwait, so we cannot gain from POSIX semaphore any more. An alternative way is to use sem_trywait + usleep, maybe we can remove CONFIG_SEM_TIMEDWAIT in this way? No, because some systems (e.g. mac os) mark the sem_xxx API as deprecated. So maybe remove the usage of POSIX semaphore and turn to use the pthread variant for all systems looks better. Signed-off-by: Longpeng(Mike) Message-Id: <20220222090507.2028-2-longpeng2@huawei.com> Signed-off-by: Paolo Bonzini --- util/qemu-thread-posix.c | 54 ------------------------------------------------ 1 file changed, 54 deletions(-) (limited to 'util/qemu-thread-posix.c') diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index e1225b63bd..1ad2503607 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -219,7 +219,6 @@ void qemu_sem_init(QemuSemaphore *sem, int init) { int rc; -#ifndef CONFIG_SEM_TIMEDWAIT rc = pthread_mutex_init(&sem->lock, NULL); if (rc != 0) { error_exit(rc, __func__); @@ -232,12 +231,6 @@ void qemu_sem_init(QemuSemaphore *sem, int init) error_exit(EINVAL, __func__); } sem->count = init; -#else - rc = sem_init(&sem->sem, 0, init); - if (rc < 0) { - error_exit(errno, __func__); - } -#endif sem->initialized = true; } @@ -247,7 +240,6 @@ void qemu_sem_destroy(QemuSemaphore *sem) assert(sem->initialized); sem->initialized = false; -#ifndef CONFIG_SEM_TIMEDWAIT rc = pthread_cond_destroy(&sem->cond); if (rc < 0) { error_exit(rc, __func__); @@ -256,12 +248,6 @@ void qemu_sem_destroy(QemuSemaphore *sem) if (rc < 0) { error_exit(rc, __func__); } -#else - rc = sem_destroy(&sem->sem); - if (rc < 0) { - error_exit(errno, __func__); - } -#endif } void qemu_sem_post(QemuSemaphore *sem) @@ -269,7 +255,6 @@ void qemu_sem_post(QemuSemaphore *sem) int rc; assert(sem->initialized); -#ifndef CONFIG_SEM_TIMEDWAIT pthread_mutex_lock(&sem->lock); if (sem->count == UINT_MAX) { rc = EINVAL; @@ -281,12 +266,6 @@ void qemu_sem_post(QemuSemaphore *sem) if (rc != 0) { error_exit(rc, __func__); } -#else - rc = sem_post(&sem->sem); - if (rc < 0) { - error_exit(errno, __func__); - } -#endif } int qemu_sem_timedwait(QemuSemaphore *sem, int ms) @@ -295,7 +274,6 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms) struct timespec ts; assert(sem->initialized); -#ifndef CONFIG_SEM_TIMEDWAIT rc = 0; compute_abs_deadline(&ts, ms); pthread_mutex_lock(&sem->lock); @@ -313,29 +291,6 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms) } pthread_mutex_unlock(&sem->lock); return (rc == ETIMEDOUT ? -1 : 0); -#else - if (ms <= 0) { - /* This is cheaper than sem_timedwait. */ - do { - rc = sem_trywait(&sem->sem); - } while (rc == -1 && errno == EINTR); - if (rc == -1 && errno == EAGAIN) { - return -1; - } - } else { - compute_abs_deadline(&ts, ms); - do { - rc = sem_timedwait(&sem->sem, &ts); - } while (rc == -1 && errno == EINTR); - if (rc == -1 && errno == ETIMEDOUT) { - return -1; - } - } - if (rc < 0) { - error_exit(errno, __func__); - } - return 0; -#endif } void qemu_sem_wait(QemuSemaphore *sem) @@ -343,7 +298,6 @@ void qemu_sem_wait(QemuSemaphore *sem) int rc; assert(sem->initialized); -#ifndef CONFIG_SEM_TIMEDWAIT pthread_mutex_lock(&sem->lock); while (sem->count == 0) { rc = pthread_cond_wait(&sem->cond, &sem->lock); @@ -353,14 +307,6 @@ void qemu_sem_wait(QemuSemaphore *sem) } --sem->count; pthread_mutex_unlock(&sem->lock); -#else - do { - rc = sem_wait(&sem->sem); - } while (rc == -1 && errno == EINTR); - if (rc < 0) { - error_exit(errno, __func__); - } -#endif } #ifdef __linux__ -- cgit 1.4.1 From 657ac98b58cee10e99c9d402bda4555fd0ec4d1f Mon Sep 17 00:00:00 2001 From: "Longpeng(Mike)" Date: Tue, 22 Feb 2022 17:05:05 +0800 Subject: thread-posix: use monotonic clock for QemuCond and QemuSemaphore Use CLOCK_MONOTONIC, so the timeout isn't affected by changes to the system time. It depends on the pthread_condattr_setclock(), while some systems(e.g. mac os) does not support it, so the behavior won't change in these systems. Signed-off-by: Longpeng(Mike) Message-Id: <20220222090507.2028-3-longpeng2@huawei.com> Signed-off-by: Paolo Bonzini --- meson.build | 11 ++++++++++ util/qemu-thread-posix.c | 53 +++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 57 insertions(+), 7 deletions(-) (limited to 'util/qemu-thread-posix.c') diff --git a/meson.build b/meson.build index 526e9dc03b..6ba60950c8 100644 --- a/meson.build +++ b/meson.build @@ -1780,6 +1780,17 @@ config_host_data.set('CONFIG_PTHREAD_SETNAME_NP_WO_TID', cc.links(gnu_source_pre pthread_create(&thread, 0, f, 0); return 0; }''', dependencies: threads)) +config_host_data.set('CONFIG_PTHREAD_CONDATTR_SETCLOCK', cc.links(gnu_source_prefix + ''' + #include + #include + + int main(void) + { + pthread_condattr_t attr + pthread_condattr_init(&attr); + pthread_condattr_setclock(&attr, CLOCK_MONOTONIC); + return 0; + }''', dependencies: threads)) config_host_data.set('CONFIG_SIGNALFD', cc.links(gnu_source_prefix + ''' #include diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index 1ad2503607..f6a34eed2f 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -38,12 +38,20 @@ static void error_exit(int err, const char *msg) abort(); } +static inline clockid_t qemu_timedwait_clockid(void) +{ +#ifdef CONFIG_PTHREAD_CONDATTR_SETCLOCK + return CLOCK_MONOTONIC; +#else + return CLOCK_REALTIME; +#endif +} + static void compute_abs_deadline(struct timespec *ts, int ms) { - struct timeval tv; - gettimeofday(&tv, NULL); - ts->tv_nsec = tv.tv_usec * 1000 + (ms % 1000) * 1000000; - ts->tv_sec = tv.tv_sec + ms / 1000; + clock_gettime(qemu_timedwait_clockid(), ts); + ts->tv_nsec += (ms % 1000) * 1000000; + ts->tv_sec += ms / 1000; if (ts->tv_nsec >= 1000000000) { ts->tv_sec++; ts->tv_nsec -= 1000000000; @@ -147,11 +155,27 @@ void qemu_rec_mutex_unlock_impl(QemuRecMutex *mutex, const char *file, int line) void qemu_cond_init(QemuCond *cond) { + pthread_condattr_t attr; int err; - err = pthread_cond_init(&cond->cond, NULL); - if (err) + err = pthread_condattr_init(&attr); + if (err) { + error_exit(err, __func__); + } +#ifdef CONFIG_PTHREAD_CONDATTR_SETCLOCK + err = pthread_condattr_setclock(&attr, qemu_timedwait_clockid()); + if (err) { + error_exit(err, __func__); + } +#endif + err = pthread_cond_init(&cond->cond, &attr); + if (err) { error_exit(err, __func__); + } + err = pthread_condattr_destroy(&attr); + if (err) { + error_exit(err, __func__); + } cond->initialized = true; } @@ -217,16 +241,31 @@ bool qemu_cond_timedwait_impl(QemuCond *cond, QemuMutex *mutex, int ms, void qemu_sem_init(QemuSemaphore *sem, int init) { + pthread_condattr_t attr; int rc; rc = pthread_mutex_init(&sem->lock, NULL); if (rc != 0) { error_exit(rc, __func__); } - rc = pthread_cond_init(&sem->cond, NULL); + rc = pthread_condattr_init(&attr); + if (rc != 0) { + error_exit(rc, __func__); + } +#ifdef CONFIG_PTHREAD_CONDATTR_SETCLOCK + rc = pthread_condattr_setclock(&attr, qemu_timedwait_clockid()); if (rc != 0) { error_exit(rc, __func__); } +#endif + rc = pthread_cond_init(&sem->cond, &attr); + if (rc != 0) { + error_exit(rc, __func__); + } + rc = pthread_condattr_destroy(&attr); + if (rc < 0) { + error_exit(rc, __func__); + } if (init < 0) { error_exit(EINVAL, __func__); } -- cgit 1.4.1 From a0d45db85496c195ab5f3f2ced742fc93d9709c2 Mon Sep 17 00:00:00 2001 From: "Longpeng(Mike)" Date: Tue, 22 Feb 2022 17:05:06 +0800 Subject: thread-posix: implement Semaphore with QemuCond and QemuMutex Now that QemuSemaphore is implemented through pthread_cond_t only, we can use QemuCond and QemuMutex to make the code smaller. Features such as mutex tracing and CLOCK_MONOTONIC timedwait are supported in qemu-sem naturally. Signed-off-by: Longpeng(Mike) Message-Id: <20220222090507.2028-4-longpeng2@huawei.com> Signed-off-by: Paolo Bonzini --- include/qemu/thread-posix.h | 5 +-- util/qemu-thread-posix.c | 105 ++++++++++++++------------------------------ 2 files changed, 34 insertions(+), 76 deletions(-) (limited to 'util/qemu-thread-posix.c') diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h index 5466608d7c..5f2f3d1386 100644 --- a/include/qemu/thread-posix.h +++ b/include/qemu/thread-posix.h @@ -27,10 +27,9 @@ struct QemuCond { }; struct QemuSemaphore { - pthread_mutex_t lock; - pthread_cond_t cond; + QemuMutex mutex; + QemuCond cond; unsigned int count; - bool initialized; }; struct QemuEvent { diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index f6a34eed2f..8505d8c60f 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -222,16 +222,15 @@ void qemu_cond_wait_impl(QemuCond *cond, QemuMutex *mutex, const char *file, con error_exit(err, __func__); } -bool qemu_cond_timedwait_impl(QemuCond *cond, QemuMutex *mutex, int ms, - const char *file, const int line) +static bool +qemu_cond_timedwait_ts(QemuCond *cond, QemuMutex *mutex, struct timespec *ts, + const char *file, const int line) { int err; - struct timespec ts; assert(cond->initialized); trace_qemu_mutex_unlock(mutex, file, line); - compute_abs_deadline(&ts, ms); - err = pthread_cond_timedwait(&cond->cond, &mutex->lock, &ts); + err = pthread_cond_timedwait(&cond->cond, &mutex->lock, ts); trace_qemu_mutex_locked(mutex, file, line); if (err && err != ETIMEDOUT) { error_exit(err, __func__); @@ -239,113 +238,73 @@ bool qemu_cond_timedwait_impl(QemuCond *cond, QemuMutex *mutex, int ms, return err != ETIMEDOUT; } +bool qemu_cond_timedwait_impl(QemuCond *cond, QemuMutex *mutex, int ms, + const char *file, const int line) +{ + struct timespec ts; + + compute_abs_deadline(&ts, ms); + return qemu_cond_timedwait_ts(cond, mutex, &ts, file, line); +} + void qemu_sem_init(QemuSemaphore *sem, int init) { - pthread_condattr_t attr; - int rc; + qemu_mutex_init(&sem->mutex); + qemu_cond_init(&sem->cond); - rc = pthread_mutex_init(&sem->lock, NULL); - if (rc != 0) { - error_exit(rc, __func__); - } - rc = pthread_condattr_init(&attr); - if (rc != 0) { - error_exit(rc, __func__); - } -#ifdef CONFIG_PTHREAD_CONDATTR_SETCLOCK - rc = pthread_condattr_setclock(&attr, qemu_timedwait_clockid()); - if (rc != 0) { - error_exit(rc, __func__); - } -#endif - rc = pthread_cond_init(&sem->cond, &attr); - if (rc != 0) { - error_exit(rc, __func__); - } - rc = pthread_condattr_destroy(&attr); - if (rc < 0) { - error_exit(rc, __func__); - } if (init < 0) { error_exit(EINVAL, __func__); } sem->count = init; - sem->initialized = true; } void qemu_sem_destroy(QemuSemaphore *sem) { - int rc; - - assert(sem->initialized); - sem->initialized = false; - rc = pthread_cond_destroy(&sem->cond); - if (rc < 0) { - error_exit(rc, __func__); - } - rc = pthread_mutex_destroy(&sem->lock); - if (rc < 0) { - error_exit(rc, __func__); - } + qemu_cond_destroy(&sem->cond); + qemu_mutex_destroy(&sem->mutex); } void qemu_sem_post(QemuSemaphore *sem) { - int rc; - - assert(sem->initialized); - pthread_mutex_lock(&sem->lock); + qemu_mutex_lock(&sem->mutex); if (sem->count == UINT_MAX) { - rc = EINVAL; + error_exit(EINVAL, __func__); } else { sem->count++; - rc = pthread_cond_signal(&sem->cond); - } - pthread_mutex_unlock(&sem->lock); - if (rc != 0) { - error_exit(rc, __func__); + qemu_cond_signal(&sem->cond); } + qemu_mutex_unlock(&sem->mutex); } int qemu_sem_timedwait(QemuSemaphore *sem, int ms) { - int rc; + bool rc = true; struct timespec ts; - assert(sem->initialized); - rc = 0; compute_abs_deadline(&ts, ms); - pthread_mutex_lock(&sem->lock); + qemu_mutex_lock(&sem->mutex); while (sem->count == 0) { - rc = pthread_cond_timedwait(&sem->cond, &sem->lock, &ts); - if (rc == ETIMEDOUT) { + rc = qemu_cond_timedwait_ts(&sem->cond, &sem->mutex, &ts, + __FILE__, __LINE__); + if (!rc) { /* timeout */ break; } - if (rc != 0) { - error_exit(rc, __func__); - } } - if (rc != ETIMEDOUT) { + if (rc) { --sem->count; } - pthread_mutex_unlock(&sem->lock); - return (rc == ETIMEDOUT ? -1 : 0); + qemu_mutex_unlock(&sem->mutex); + return (rc ? 0 : -1); } void qemu_sem_wait(QemuSemaphore *sem) { - int rc; - - assert(sem->initialized); - pthread_mutex_lock(&sem->lock); + qemu_mutex_lock(&sem->mutex); while (sem->count == 0) { - rc = pthread_cond_wait(&sem->cond, &sem->lock); - if (rc != 0) { - error_exit(rc, __func__); - } + qemu_cond_wait(&sem->cond, &sem->mutex); } --sem->count; - pthread_mutex_unlock(&sem->lock); + qemu_mutex_unlock(&sem->mutex); } #ifdef __linux__ -- cgit 1.4.1 From 8ab3026489eafa9da07c09f1929593fe0db5e380 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 21 Feb 2022 12:46:32 +0100 Subject: thread-posix: optimize qemu_sem_timedwait with zero timeout In this case there is no need to call pthread_cond_timedwait; the function is just a trywait and waiting on the condition variable would always time out. Signed-off-by: Paolo Bonzini --- util/qemu-thread-posix.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'util/qemu-thread-posix.c') diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index 8505d8c60f..ac1d56e673 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -284,8 +284,12 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms) compute_abs_deadline(&ts, ms); qemu_mutex_lock(&sem->mutex); while (sem->count == 0) { - rc = qemu_cond_timedwait_ts(&sem->cond, &sem->mutex, &ts, - __FILE__, __LINE__); + if (ms == 0) { + rc = false; + } else { + rc = qemu_cond_timedwait_ts(&sem->cond, &sem->mutex, &ts, + __FILE__, __LINE__); + } if (!rc) { /* timeout */ break; } -- cgit 1.4.1