Re: [Qemu-devel] [PATCH for-2.5] rcu: Allow calling rcu_(un)register_thread() during synchronize_rcu()

[Wen Congyang <wency@cn.fujitsu.com>]

On 07/27/2015 06:17 PM, Paolo Bonzini wrote:
> 
> 
> On 27/07/2015 04:24, Wen Congyang wrote:
>> +        /* Wait for one thread to report a quiescent state and try again.
>> +         * Release rcu_registry_lock, so rcu_(un)register_thread() doesn't
>> +         * wait too much time. Note: rcu_unregister_thread() may remove
>> +         * the node from qsreaders. That's a bit tricky, but it should work.
> 
> "It should work" is a bit optimistic. :D
> 
> Does this description look okay?
> 
>         /* Wait for one thread to report a quiescent state and try again.
>          * Release rcu_registry_lock, so rcu_(un)register_thread() doesn't
>          * wait too much time.
>          *
>          * rcu_register_thread() may add nodes to &registry; it will not
>          * wake up synchronize_rcu, but that is okay because at least another
>          * thread must exit its RCU read-side critical section before
>          * synchronize_rcu is done.  The next iteration of the loop will
>          * process the new thread or set ->waiting for it.  Hence, this can
>          * at worst cause synchronize_rcu() to wait for longer.

I don't understand this. The next iteration of the loop will move the new thread's
rcu_reader from registry to qsreaders even if we call rcu_read_lock() in the new thread.
Because rcu_gp_ongoing() will return false.

Thanks
Wen Congyang

>          *
>          * rcu_unregister_thread() may remove nodes from &qsreaders instead
>          * of &registry if it runs during qemu_event_wait.  That's okay;
>          * the node then will not be added back to &registry by QLIST_SWAP
>          * below.  The invariant is that the node is part of one list when
>          * rcu_registry_lock is released.
>          */
> 
> Paolo
> .
>