Emilio G. Cota <c...@braap.org> writes:
> 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.
Doh apologies I misread the code. What caught my eye is the old code
locks/unlocks both rcu_registry_lock and rcu_sync_lock so shouldn't the
child function reinit both?
static void rcu_init_child(void)
{
qemu_mutex_init(&rcu_sync_lock);
qemu_mutex_init(&rcu_registry_lock);
}
>
> 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
--
Alex Bennée
