| Message ID | 20220221095617.1974-3-longpeng2@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qemu-sem-posix: use monotonic clock instead | expand |
On 2/21/22 10:56, Longpeng(Mike) via wrote: > + long now_nsec; > +#ifdef CONFIG_PTHREAD_CONDATTR_SETCLOCK > + struct timespec now; > + clock_gettime(CLOCK_MONOTONIC, &now); > + now_sec = now.tv_sec; > + now_nsec = now.tv_nsec; > +#else > struct timeval tv; > gettimeofday(&tv, NULL); > - ts->tv_nsec = tv.tv_usec * 1000 + (ms % 1000) * 1000000; > - ts->tv_sec = tv.tv_sec + ms / 1000; > + now_sec = tv.tv_sec; > + now_nsec = tv.tv_usec * 1000; > +#endif > + Perhaps this might minimize the amount of conditional code, too: diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index 903fa33965..4743d7b714 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -40,10 +40,14 @@ static void error_exit(int err, const char *msg) 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; +#ifdef CONFIG_PTHREAD_CONDATTR_SETCLOCK + clock_gettime(CLOCK_MONOTONIC, ts); +#else + clock_gettime(CLOCK_REALTIME, ts); +#endif + + ts->tv_nsec += (ms % 1000) * 1000000; + ts->tv_sec += ms / 1000; if (ts->tv_nsec >= 1000000000) { ts->tv_sec++; ts->tv_nsec -= 1000000000; Finally, the conditional variables initialization qemu_cond_init must also use pthread_condattr_setclock. Paolo
> -----Original Message----- > From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini > Sent: Monday, February 21, 2022 7:42 PM > To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) > <longpeng2@huawei.com>; berrange@redhat.com; mst@redhat.com > Cc: qemu-devel@nongnu.org; Gonglei (Arei) <arei.gonglei@huawei.com> > Subject: Re: [RFC 2/2] sem-posix: use monotonic clock instead > > On 2/21/22 10:56, Longpeng(Mike) via wrote: > > + long now_nsec; > > +#ifdef CONFIG_PTHREAD_CONDATTR_SETCLOCK > > + struct timespec now; > > + clock_gettime(CLOCK_MONOTONIC, &now); > > + now_sec = now.tv_sec; > > + now_nsec = now.tv_nsec; > > +#else > > struct timeval tv; > > gettimeofday(&tv, NULL); > > - ts->tv_nsec = tv.tv_usec * 1000 + (ms % 1000) * 1000000; > > - ts->tv_sec = tv.tv_sec + ms / 1000; > > + now_sec = tv.tv_sec; > > + now_nsec = tv.tv_usec * 1000; > > +#endif > > + > > Perhaps this might minimize the amount of conditional code, too: > > diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c > index 903fa33965..4743d7b714 100644 > --- a/util/qemu-thread-posix.c > +++ b/util/qemu-thread-posix.c > @@ -40,10 +40,14 @@ static void error_exit(int err, const char *msg) > > 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; > +#ifdef CONFIG_PTHREAD_CONDATTR_SETCLOCK > + clock_gettime(CLOCK_MONOTONIC, ts); > +#else > + clock_gettime(CLOCK_REALTIME, ts); > +#endif > + > + ts->tv_nsec += (ms % 1000) * 1000000; > + ts->tv_sec += ms / 1000; > if (ts->tv_nsec >= 1000000000) { > ts->tv_sec++; > ts->tv_nsec -= 1000000000; > > > Finally, the conditional variables initialization qemu_cond_init must > also use pthread_condattr_setclock. > Make sense! Will optimize the code in the next version. Thanks. > Paolo
diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h index 5466608..cc77000 100644 --- a/include/qemu/thread-posix.h +++ b/include/qemu/thread-posix.h @@ -29,6 +29,7 @@ struct QemuCond { struct QemuSemaphore { pthread_mutex_t lock; pthread_cond_t cond; + pthread_condattr_t attr; unsigned int count; bool initialized; }; diff --git a/meson.build b/meson.build index 3ccb110..2bab94f 100644 --- a/meson.build +++ b/meson.build @@ -1688,6 +1688,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 <pthread.h> + #include <time.h> + + 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 <sys/signalfd.h> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index 1ad2503..d3a7c54 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -40,10 +40,22 @@ static void error_exit(int err, const char *msg) static void compute_abs_deadline(struct timespec *ts, int ms) { + time_t now_sec; + long now_nsec; +#ifdef CONFIG_PTHREAD_CONDATTR_SETCLOCK + struct timespec now; + clock_gettime(CLOCK_MONOTONIC, &now); + now_sec = now.tv_sec; + now_nsec = now.tv_nsec; +#else struct timeval tv; gettimeofday(&tv, NULL); - ts->tv_nsec = tv.tv_usec * 1000 + (ms % 1000) * 1000000; - ts->tv_sec = tv.tv_sec + ms / 1000; + now_sec = tv.tv_sec; + now_nsec = tv.tv_usec * 1000; +#endif + + ts->tv_nsec = now_nsec + (ms % 1000) * 1000000; + ts->tv_sec = now_sec + ms / 1000; if (ts->tv_nsec >= 1000000000) { ts->tv_sec++; ts->tv_nsec -= 1000000000; @@ -223,7 +235,17 @@ void qemu_sem_init(QemuSemaphore *sem, int init) if (rc != 0) { error_exit(rc, __func__); } - rc = pthread_cond_init(&sem->cond, NULL); + rc = pthread_condattr_init(&sem->attr); + if (rc != 0) { + error_exit(rc, __func__); + } +#ifdef CONFIG_PTHREAD_CONDATTR_SETCLOCK + rc = pthread_condattr_setclock(&sem->attr, CLOCK_MONOTONIC); + if (rc != 0) { + error_exit(rc, __func__); + } +#endif + rc = pthread_cond_init(&sem->cond, &sem->attr); if (rc != 0) { error_exit(rc, __func__); } @@ -248,6 +270,10 @@ void qemu_sem_destroy(QemuSemaphore *sem) if (rc < 0) { error_exit(rc, __func__); } + rc = pthread_condattr_destroy(&sem->attr); + if (rc < 0) { + error_exit(rc, __func__); + } } void qemu_sem_post(QemuSemaphore *sem)
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) do not support it, the behavior won't change in these systems. Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com> --- include/qemu/thread-posix.h | 1 + meson.build | 11 +++++++++++ util/qemu-thread-posix.c | 32 +++++++++++++++++++++++++++++--- 3 files changed, 41 insertions(+), 3 deletions(-)