Although the doc comment says "Called within RCU critical section" the
address_space_read() function takes the RCU read lock itself. There is
no need for the caller to hold an RCU read lock.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Fixes: b2a44fcad74f1cc7a6786d38eba7db12ab2352ba ("address_space_read: address_space_to_flatview needs RCU lock")
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/exec/memory.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index c3d417d317..60508a1e19 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2768,7 +2768,7 @@ static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
  *
  * Return a MemTxResult indicating whether the operation succeeded
  * or failed (eg unassigned memory, device rejected the transaction,
- * IOMMU fault).  Called within RCU critical section.
+ * IOMMU fault).
  *
  * @as: #AddressSpace to be accessed
  * @addr: address within that address space
-- 
2.31.1

On 9/9/21 4:10 PM, Stefan Hajnoczi wrote:
> Although the doc comment says "Called within RCU critical section" the
> address_space_read() function takes the RCU read lock itself. There is
> no need for the caller to hold an RCU read lock.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Fixes: b2a44fcad74f1cc7a6786d38eba7db12ab2352ba ("address_space_read: address_space_to_flatview needs RCU lock")
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  include/exec/memory.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

On 09.09.21 16:10, Stefan Hajnoczi wrote:
> Although the doc comment says "Called within RCU critical section" the
> address_space_read() function takes the RCU read lock itself. There is
> no need for the caller to hold an RCU read lock.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Fixes: b2a44fcad74f1cc7a6786d38eba7db12ab2352ba ("address_space_read: address_space_to_flatview needs RCU lock")
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   include/exec/memory.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index c3d417d317..60508a1e19 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -2768,7 +2768,7 @@ static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
>    *
>    * Return a MemTxResult indicating whether the operation succeeded
>    * or failed (eg unassigned memory, device rejected the transaction,
> - * IOMMU fault).  Called within RCU critical section.
> + * IOMMU fault).
>    *
>    * @as: #AddressSpace to be accessed
>    * @addr: address within that address space
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

On 09/09/21 16:10, Stefan Hajnoczi wrote:
> Although the doc comment says "Called within RCU critical section" the
> address_space_read() function takes the RCU read lock itself. There is
> no need for the caller to hold an RCU read lock.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Fixes: b2a44fcad74f1cc7a6786d38eba7db12ab2352ba ("address_space_read: address_space_to_flatview needs RCU lock")
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   include/exec/memory.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index c3d417d317..60508a1e19 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -2768,7 +2768,7 @@ static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
>    *
>    * Return a MemTxResult indicating whether the operation succeeded
>    * or failed (eg unassigned memory, device rejected the transaction,
> - * IOMMU fault).  Called within RCU critical section.
> + * IOMMU fault).
>    *
>    * @as: #AddressSpace to be accessed
>    * @addr: address within that address space
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>