On Tue, Sep 08, 2015 at 18:34:38 +0100, Alex Bennée wrote:
> Emilio G. Cota <c...@braap.org> writes:
(snip)
> > +static void rcu_init_child(void)
> > +{
> > + qemu_mutex_init(&rcu_registry_lock);
> > +}
> > #endif
> >
> > void rcu_after_fork(void)
> > @@ -346,7 +351,7 @@ void rcu_after_fork(void)
> > static void __attribute__((__constructor__)) rcu_init(void)
> > {
> > #ifdef CONFIG_POSIX
> > - pthread_atfork(rcu_init_lock, rcu_init_unlock, rcu_init_unlock);
> > + pthread_atfork(rcu_init_lock, rcu_init_unlock, rcu_init_child);
> > #endif
>
> Hmm previously we unlocked both rcu_sync_lock and rcu_registry_lock, is
> it somehow different in it's locking rules? If I'm reading the
> pthread_atfork man page right couldn't we just do:
>
> pthread_atfork(rcu_init_lock, rcu_init_unlock, rcu_init_lock);
That'd cause the child to deadlock.
Before forking the parent locks those mutexes; then it forks. The child sees
those mutexes as 'locked', and whatever memory the parent writes to _after_
fork is not seen by the child. So trying to 'lock' those mutexes from the child
is never going to succeed, because they're seeing as already locked.
The original idea ("unlock" from the child) is OK, but doesn't work when
using PTHREAD_MUTEX_ERRORCHECK. Yes, this was removed (24fa90499f) but
it's too useful to not cherry-pick it when developing MTTCG.
What I think remains to be fixed is the possibility of corruption when
any call_rcu calls are pending while forking. The child should not mess
with these; only the parent should. See call_rcu_after_fork_{parent,child}
from liburcu for details.
Emilio
