[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 30/30] exec: put address space dispatch under RC
From: |
Jan Kiszka |
Subject: |
Re: [Qemu-devel] [PATCH 30/30] exec: put address space dispatch under RCU critical section |
Date: |
Fri, 28 Jun 2013 21:38:48 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
On 2013-06-28 20:26, Paolo Bonzini wrote:
> With this change, address space dispatch can be moved outside the
> BQL. The actual I/O would still have to happen within the lock.
>
> The next step would be to introduce a function that can only
> be called from outside the BQL, address_space_rw_unlocked. The
> function would do something like
>
> mr = address_space_translate(...)
> if (!mr->unlocked) {
> locked = true;
> qemu_mutex_lock_iothread();
> }
> ...dispatch...
> if (locked) {
> qemu_mutex_unlock_iothread();
> }
>
> (Note that subpages are already ready to be changed to unlocked
> access).
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> cputlb.c | 4 +++-
> exec.c | 30 ++++++++++++++++++++++++++----
> hw/ppc/spapr_iommu.c | 10 +++++++++-
> 3 files changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/cputlb.c b/cputlb.c
> index 82875b1..97bfe70 100644
> --- a/cputlb.c
> +++ b/cputlb.c
> @@ -267,8 +267,9 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
> tlb_add_large_page(env, vaddr, size);
> }
>
> + rcu_read_lock();
> sz = size;
> - d = address_space_memory.dispatch;
> + d = rcu_dereference(&address_space_memory.dispatch);
> section = address_space_translate_for_iotlb(d, paddr, &xlat, &sz);
> assert(sz >= TARGET_PAGE_SIZE);
>
> @@ -321,6 +322,7 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
> } else {
> te->addr_write = -1;
> }
> + rcu_read_unlock();
> }
>
> /* NOTE: this function can trigger an exception */
> diff --git a/exec.c b/exec.c
> index e564014..8c6f925 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -97,6 +97,8 @@ struct PhysPageEntry {
> typedef PhysPageEntry Node[L2_SIZE];
>
> struct AddressSpaceDispatch {
> + struct rcu_head rcu;
> +
> /* This is a multi-level map on the physical address space.
> * The bottom level has pointers to MemoryRegionSections.
> */
> @@ -120,6 +122,8 @@ typedef struct subpage_t {
> #define PHYS_SECTION_WATCH 3
>
> typedef struct PhysPageMap {
> + struct rcu_head rcu;
> +
> unsigned sections_nb;
> unsigned sections_nb_alloc;
> unsigned nodes_nb;
> @@ -236,6 +240,7 @@ bool memory_region_is_unassigned(MemoryRegion *mr)
> && mr != &io_mem_watch;
> }
>
> +/* Called from RCU critical section */
> static MemoryRegionSection *address_space_lookup_region(AddressSpaceDispatch
> *d,
> hwaddr addr,
> bool resolve_subpage)
> @@ -252,6 +257,7 @@ static MemoryRegionSection
> *address_space_lookup_region(AddressSpaceDispatch *d,
> return section;
> }
>
> +/* Called from RCU critical section */
> static MemoryRegionSection *
> address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr,
> hwaddr *xlat,
> hwaddr *plen, bool resolve_subpage)
> @@ -280,8 +286,10 @@ MemoryRegion *address_space_translate(AddressSpace *as,
> hwaddr addr,
> MemoryRegion *mr;
> hwaddr len = *plen;
>
> + rcu_read_lock();
> for (;;) {
> - section = address_space_translate_internal(as->dispatch, addr,
> &addr, plen, true);
> + AddressSpaceDispatch *d = rcu_dereference(&as->dispatch);
> + section = address_space_translate_internal(d, addr, &addr, plen,
> true);
> mr = section->mr;
>
> if (!mr->iommu_ops) {
> @@ -303,9 +311,11 @@ MemoryRegion *address_space_translate(AddressSpace *as,
> hwaddr addr,
> *plen = len;
> *xlat = addr;
> memory_region_ref(mr);
> + rcu_read_unlock();
> return mr;
> }
At this point, do we still have unowned memory regions? If so, we must
not return them here if the caller is not holding the BQL (which is
supposed to protect their registration/modification/deregistration).
I played with a version today that returns NULL in this case. Then the
caller of address_space_translate can take the BQL and retry the
translation.
Jan
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH 21/30] exec: separate current memory map from the one being built, (continued)
- [Qemu-devel] [PATCH 21/30] exec: separate current memory map from the one being built, Paolo Bonzini, 2013/06/28
- [Qemu-devel] [PATCH 22/30] memory: move MemoryListener declaration earlier, Paolo Bonzini, 2013/06/28
- [Qemu-devel] [PATCH 23/30] exec: move listener from AddressSpaceDispatch to AddressSpace, Paolo Bonzini, 2013/06/28
- [Qemu-devel] [PATCH 24/30] exec: separate current radix tree from the one being built, Paolo Bonzini, 2013/06/28
- [Qemu-devel] [PATCH 26/30] exec: remove cur_map, Paolo Bonzini, 2013/06/28
- [Qemu-devel] [PATCH 25/30] exec: put memory map in AddressSpaceDispatch, Paolo Bonzini, 2013/06/28
- [Qemu-devel] [PATCH 27/30] exec: change some APIs to take AddressSpaceDispatch, Paolo Bonzini, 2013/06/28
- [Qemu-devel] [PATCH 28/30] exec: change iotlb APIs to take AddressSpaceDispatch, Paolo Bonzini, 2013/06/28
- [Qemu-devel] [PATCH 29/30] exec: add a reference to the region returned by address_space_translate, Paolo Bonzini, 2013/06/28
- [Qemu-devel] [PATCH 30/30] exec: put address space dispatch under RCU critical section, Paolo Bonzini, 2013/06/28
- Re: [Qemu-devel] [PATCH 30/30] exec: put address space dispatch under RCU critical section,
Jan Kiszka <=