Re: [PATCH v8 13/21] main: keep rcu_atfork callback enabled for qtest

[Alexander Bulekov]

On 200130 1824, Paolo Bonzini wrote:
> On 30/01/20 15:42, Stefan Hajnoczi wrote:
> >> +
> >> +    /*
> >> +     * If QTest is enabled, keep the rcu_atfork enabled, since system 
> >> processes
> >> +     * may be forked testing purposes (e.g. fork-server based fuzzing)
> >> +     */
> >> +    if (!qtest_enabled()) {
> >> +        rcu_disable_atfork();
> >> +    }
> > I haven't reviewed the details of whether resources are leaked across
> > fork but in general it makes sense that we want an RCU thread in the
> > fork child:
> 
> Note that there is a possible deadlock between fork and synchronize_rcu
> (see commit 73c6e40, "rcu: completely disable pthread_atfork callbacks
> as soon as possible", 2016-01-27):
> 
> - the CPU thread is inside a RCU critical section and wants to take the
> BQL in order to do MMIO
> 
> - the I/O thread, which is owning the BQL, forks and calls
> rcu_init_lock, which tries to take the rcu_sync_lock
> 
> - the call_rcu thread has taken rcu_sync_lock in synchronize_rcu, but
> synchronize_rcu needs the CPU thread to end the critical section before
> returning.
> 
> Therefore it would be best if the fork server could fork before a single
> CPU instruction is executed, and then rcu_disable_atfork could be moved
> right after the fork server is started (just like right now we do it
> right after os_daemonize).  We probably talked about this before, but
> how do you ensure that the fork server is started before threads are
> created (apart from the RCU thread)?

With QTest, is this still a concern, since there are no CPU instructions
involved? Sometimes the fork-server starts after some I/O has already
occured (eg mapping BARs and setting up VQs for virtio-net). I know we
briefly talked about threads at some point, and it seems that iothreads
may be a concern, if any are started before fork. Other than that, since
there is no TCG/CPU thread, are there any other threads that could be
a concern?
-Alex

> Paolo
>