[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC v4 2/3] memory: add depth assert in address_space_to_flatview
From: |
Peter Xu |
Subject: |
Re: [RFC v4 2/3] memory: add depth assert in address_space_to_flatview |
Date: |
Tue, 3 Jan 2023 12:43:27 -0500 |
Hi, Paolo,
On Wed, Dec 28, 2022 at 09:27:50AM +0100, Paolo Bonzini wrote:
> Il ven 23 dic 2022, 16:54 Peter Xu <peterx@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
- Re: [RFC v4 2/3] memory: add depth assert in address_space_to_flatview,
Peter Xu <=