From: Peter Xu <peterx@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, Eric Blake <eblake@redhat.com>
Subject: Re: [Qemu-devel] [PATCH for-2.11] rcu: init globals only once
Date: Tue, 8 Aug 2017 15:49:15 +0800 [thread overview]
Message-ID: <20170808074915.GB13486@pxdev.xzpeter.org> (raw)
In-Reply-To: <3f0e8fe6-babd-df82-c690-de0f1ffdd460@redhat.com>
On Tue, Aug 08, 2017 at 09:26:43AM +0200, Paolo Bonzini wrote:
> On 08/08/2017 09:00, Peter Xu wrote:
> > We were calling rcu_init_complete() twice in the child processes when
> > fork happened. However the pthread library does not really suggest to do
> > it that way:
> >
> > http://pubs.opengroup.org/onlinepubs/7908799/xsh/pthread_mutex_init.html
> >
> > "Attempting to initialise an already initialised mutex results in
> > undefined behaviour."
> >
> > Actually, IMHO we can do it in a more natural way: Firstly, we only init
> > the RCU globals once in rcu_init(). Then, in rcu_init_child(), we unlock
> > all the locks held in rcu_init_lock() just like what we do in the parent
> > process, then do the rest of RCU re-init (e.g., create the RCU thread).
>
> This doesn't work for error-checking mutexes: rcu_init_child has a
> different PID than the parent, so the mutexes aren't unlocked. It's
> also true that right now we don't use error-checking mutexes (commit
> 24fa90499f, "qemu-thread: do not use PTHREAD_MUTEX_ERRORCHECK",
> 2015-03-10); however, that's also a bit sad.
>
> The reason for the undefined behavior is probably that some operating
> systems allocate memory in pthread_mutex_init, and initializing twice
> causes a memory leak. One such operating system is OpenBSD. :(
Good to know. :)
I thought pthread_atfork() was designed to solve such a locking
problem (in child hanlder, we unlock all the held locks). If
PTHREAD_MUTEX_ERRORCHECK cannot coop well with it, not sure whether
that means we should just avoid using PTHREAD_MUTEX_ERRORCHECK in such
a use case (but we should be able to use the error checks in other
mutexes that do not need extra fork handling)?
Another idea is: can we just destroy the mutex first then re-init it
in subprocess? A quick glance in libpthread code shows that at least
pthread_mutex_destroy() won't check PTHREAD_MUTEX_ERRORCHECK.
Thanks,
>
> Eric, you chimed in on the patch that became commit 24fa90499f, what do
> you suggest?
>
> Paolo
--
Peter Xu
next prev parent reply other threads:[~2017-08-08 7:49 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-08 7:00 [Qemu-devel] [PATCH for-2.11] rcu: init globals only once Peter Xu
2017-08-08 7:26 ` Paolo Bonzini
2017-08-08 7:49 ` Peter Xu [this message]
2017-08-08 8:15 ` Paolo Bonzini
2017-08-08 14:09 ` Eric Blake
2017-08-09 3:25 ` Peter Xu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170808074915.GB13486@pxdev.xzpeter.org \
--to=peterx@redhat.com \
--cc=eblake@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.