[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [RFC 1/2] sem-posix: remove the posix semaphore support
From: |
Longpeng (Mike, Cloud Infrastructure Service Product Dept.) |
Subject: |
RE: [RFC 1/2] sem-posix: remove the posix semaphore support |
Date: |
Mon, 21 Feb 2022 14:35:54 +0000 |
Hi Daniel,
> -----Original Message-----
> From: Daniel P. Berrangé [mailto:berrange@redhat.com]
> Sent: Monday, February 21, 2022 7:12 PM
> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
> <longpeng2@huawei.com>
> Cc: pbonzini@redhat.com; mst@redhat.com; qemu-devel@nongnu.org; Gonglei (Arei)
> <arei.gonglei@huawei.com>
> Subject: Re: [RFC 1/2] sem-posix: remove the posix semaphore support
>
> On Mon, Feb 21, 2022 at 05:56:16PM +0800, Longpeng(Mike) via wrote:
> > 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 anymore.
>
> It doesn't appear in any man pages on my systems, but there is a
> new-ish API sem_clockwait() that accepts a choice of clock as a
> parameter.
>
> This is apparently a proposed POSIX standard API introduced in
> glibc 2.30, along with several others:
>
But the API is only supported in glibc.
https://www.gnu.org/software/gnulib/manual/html_node/sem_005fclockwait.html
> https://sourceware.org/legacy-ml/libc-announce/2019/msg00001.html
>
> [quote]
> * Add new POSIX-proposed pthread_cond_clockwait, pthread_mutex_clocklock,
> pthread_rwlock_clockrdlock, pthread_rwlock_clockwrlock and sem_clockwait
> functions. These behave similarly to their "timed" equivalents, but also
> accept a clockid_t parameter to determine which clock their timeout should
> be measured against. All functions allow waiting against CLOCK_MONOTONIC
> and CLOCK_REALTIME. The decision of which clock to be used is made at the
> time of the wait (unlike with pthread_condattr_setclock, which requires
> the clock choice at initialization time).
> [/quote]
>
It seems pthread_condattr_setclock() can meet our requirement here, it's OK
for us to choose the clock at initialization time.
> > diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h
> > index b792e6e..5466608 100644
> > --- a/include/qemu/thread-posix.h
> > +++ b/include/qemu/thread-posix.h
> > @@ -27,13 +27,9 @@ struct QemuCond {
> > };
> >
> > struct QemuSemaphore {
> > -#ifndef CONFIG_SEM_TIMEDWAIT
> > pthread_mutex_t lock;
> > pthread_cond_t cond;
> > unsigned int count;
> > -#else
> > - sem_t sem;
> > -#endif
> > bool initialized;
> > };
>
> As a point of history, the original code only used sem_t. The pthreads
> based fallback was introduced by Paolo in
>
> commit c166cb72f1676855816340666c3b618beef4b976
> Author: Paolo Bonzini <pbonzini@redhat.com>
> Date: Fri Nov 2 15:43:21 2012 +0100
>
> semaphore: implement fallback counting semaphores with mutex+condvar
>
> OpenBSD and Darwin do not have sem_timedwait. Implement a fallback
> for them.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>
> I'm going to assume this fallback impl is less efficient than the
> native sem_t impl as the reason for leaving the original impl, or
> maybe Paolo just want to risk accidental bugs by removing the
> existing usage ?
>
Paolo has replied, seems this change is acceptable, so I'll continue to
work on this solution. Thanks :)
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o-
> https://www.instagram.com/dberrange :|