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 ®istry; 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 ®istry if it runs during qemu_event_wait. That's okay; > * the node then will not be added back to ®istry by QLIST_SWAP > * below. The invariant is that the node is part of one list when > * rcu_registry_lock is released. > */ > > Paolo > . >
