[Qemu-devel] [PATCH 0/7] memory: address_space_to_flatview needs RCU lock

Paolo Bonzini posted 7 patches 6 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180305083655.6186-1-pbonzini@redhat.com
Test checkpatch passed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 passed
Test ppcbe passed
Test ppcle passed
Test s390x passed
exec.c                         | 90 +++++++++++++++++++++++++-----------------
hw/intc/openpic_kvm.c          |  4 --
include/exec/memory-internal.h | 13 ++++--
include/exec/memory.h          | 47 ++++++++++++++--------
memory.c                       | 30 --------------
5 files changed, 93 insertions(+), 91 deletions(-)
[Qemu-devel] [PATCH 0/7] memory: address_space_to_flatview needs RCU lock
Posted by Paolo Bonzini 6 years, 1 month ago
I noticed that the introduction of flatview_{read,write} placed
address_space_to_flatview outside the RCU lock.  This is wrong and has
to be fixed, because address_space_to_flatview does an atomic_rcu_read.
These patches fix this one function at a time.

Paolo Bonzini (7):
  openpic_kvm: drop address_space_to_flatview call
  memory: inline some performance-sensitive accessors
  address_space_write: address_space_to_flatview needs RCU lock
  address_space_read: address_space_to_flatview needs RCU lock
  address_space_access_valid: address_space_to_flatview needs RCU lock
  address_space_map: address_space_to_flatview needs RCU lock
  address_space_rw: address_space_to_flatview needs RCU lock

 exec.c                         | 90 +++++++++++++++++++++++++-----------------
 hw/intc/openpic_kvm.c          |  4 --
 include/exec/memory-internal.h | 13 ++++--
 include/exec/memory.h          | 47 ++++++++++++++--------
 memory.c                       | 30 --------------
 5 files changed, 93 insertions(+), 91 deletions(-)

-- 
2.14.3


Re: [Qemu-devel] [PATCH 0/7] memory: address_space_to_flatview needs RCU lock
Posted by Alexey Kardashevskiy 6 years, 1 month ago
On 05/03/18 19:36, Paolo Bonzini wrote:
> I noticed that the introduction of flatview_{read,write} placed
> address_space_to_flatview outside the RCU lock.  This is wrong and has
> to be fixed, because address_space_to_flatview does an atomic_rcu_read.
> These patches fix this one function at a time.


out of curiosity - has this caused any actual bug? should be hard to
reproduce, I suppose...


> 
> Paolo Bonzini (7):
>   openpic_kvm: drop address_space_to_flatview call
>   memory: inline some performance-sensitive accessors
>   address_space_write: address_space_to_flatview needs RCU lock
>   address_space_read: address_space_to_flatview needs RCU lock
>   address_space_access_valid: address_space_to_flatview needs RCU lock
>   address_space_map: address_space_to_flatview needs RCU lock
>   address_space_rw: address_space_to_flatview needs RCU lock
> 
>  exec.c                         | 90 +++++++++++++++++++++++++-----------------
>  hw/intc/openpic_kvm.c          |  4 --
>  include/exec/memory-internal.h | 13 ++++--
>  include/exec/memory.h          | 47 ++++++++++++++--------
>  memory.c                       | 30 --------------
>  5 files changed, 93 insertions(+), 91 deletions(-)
> 


-- 
Alexey

Re: [Qemu-devel] [PATCH 0/7] memory: address_space_to_flatview needs RCU lock
Posted by Paolo Bonzini 6 years, 1 month ago
On 06/03/2018 08:47, Alexey Kardashevskiy wrote:
> On 05/03/18 19:36, Paolo Bonzini wrote:
>> I noticed that the introduction of flatview_{read,write} placed
>> address_space_to_flatview outside the RCU lock.  This is wrong and has
>> to be fixed, because address_space_to_flatview does an atomic_rcu_read.
>> These patches fix this one function at a time.
> 
> out of curiosity - has this caused any actual bug? should be hard to
> reproduce, I suppose...

No, I just noticed it by code inspection, while looking at a
reimplementation of MemoryRegionCache (reverted in 2.9).  One of the
differences between address_space_read and address_space_read_cached was
that the latter expects to be called within rcu_read_lock, so it was
surprising that address_space_read was expecting the same. :)

Paolo