[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] qemu-thread: do not use PTHREAD_MUTEX_ERROR
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] qemu-thread: do not use PTHREAD_MUTEX_ERRORCHECK |
Date: |
Fri, 06 Mar 2015 08:54:19 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 |
On 03/06/2015 02:22 AM, Paolo Bonzini wrote:
> PTHREAD_MUTEX_ERRORCHECK is completely broken with respect to fork.
> The way to safely do fork is to bring all threads to a quiescent
> state by acquiring locks (either in callers---as we do for the
> iothread mutex---or using pthread_atfork's prepare callbacks)
> and then release them in the child.
That, and POSIX itself says that pthread_atfork is a dangerous API, and
should not be used if at all possible, because it is broken by design.
>
> The problem is that releasing error-checking locks in the child
> fails under glibc with EPERM, because the mutex stores a different
> owner tid than the duplicated thread in the child process.
Is that a bug in glibc?
> We
> could make it work for locks acquired via pthread_atfork, by
> recreating the mutex in the child instead of unlocking it
> (we know that there are no other threads that could have taken
> the mutex; but when the lock is acquired in fork's caller
> that would not be possible.
>
> The simplest solution is just to forgo error checking.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> util/qemu-thread-posix.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
I'm not sure that weakening things is always the wisest idea, but you've
provided some good arguments for why we want it here, so:
Reviewed-by: Eric Blake <address@hidden>
>
> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> index 50a29d8..ba67cec 100644
> --- a/util/qemu-thread-posix.c
> +++ b/util/qemu-thread-posix.c
> @@ -51,12 +51,8 @@ static void error_exit(int err, const char *msg)
> void qemu_mutex_init(QemuMutex *mutex)
> {
> int err;
> - pthread_mutexattr_t mutexattr;
>
> - pthread_mutexattr_init(&mutexattr);
> - pthread_mutexattr_settype(&mutexattr, PTHREAD_MUTEX_ERRORCHECK);
> - err = pthread_mutex_init(&mutex->lock, &mutexattr);
> - pthread_mutexattr_destroy(&mutexattr);
> + err = pthread_mutex_init(&mutex->lock, NULL);
> if (err)
> error_exit(err, __func__);
> }
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature