Hi, Paolo, On Wed, Dec 28, 2022 at 09:27:50AM +0100, Paolo Bonzini wrote: > Il ven 23 dic 2022, 16:54 Peter Xu <pet...@redhat.com> ha scritto: > > > > This is not valid because the transaction could happen in *another* > > thread. > > > In that case memory_region_transaction_depth() will be > 0, but RCU is > > > needed. > > > > Do you mean the code is wrong, or the comment? Note that the code has > > checked rcu_read_locked() where introduced in patch 1, but maybe something > > else was missed? > > > > The assertion is wrong. It will succeed even if RCU is unlocked in this > thread but a transaction is in progress in another thread.
IIUC this is the case where the context: (1) doesn't have RCU read lock held, and, (2) doesn't have BQL held. Is it safe at all to reference any flatview in such a context? The thing is I think the flatview pointer can be freed anytime if both locks are not taken. > Perhaps you can check (memory_region_transaction_depth() > 0 && > !qemu_mutex_iothread_locked()) || rcu_read_locked() instead? What if one thread calls address_space_to_flatview() with BQL held but not RCU read lock held? I assume it's a legal operation, but it seems to be able to trigger the assert already? Thanks, -- Peter Xu
