From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57741) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7zRw-0004tt-LZ for qemu-devel@nongnu.org; Thu, 25 Jun 2015 01:11:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z7zRt-0007fn-Cr for qemu-devel@nongnu.org; Thu, 25 Jun 2015 01:11:44 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56602) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7zRt-0007ej-2l for qemu-devel@nongnu.org; Thu, 25 Jun 2015 01:11:41 -0400 Date: Thu, 25 Jun 2015 13:11:36 +0800 From: Fam Zheng Message-ID: <20150625051136.GJ17695@ad.nay.redhat.com> References: <1435163110-2724-1-git-send-email-pbonzini@redhat.com> <1435163110-2724-6-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1435163110-2724-6-git-send-email-pbonzini@redhat.com> Subject: Re: [Qemu-devel] [PATCH 5/9] memory: let address_space_rw/ld*/st* run outside the BQL List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: borntraeger@de.ibm.com, Frederic Konrad , qemu-devel@nongnu.org, Jan Kiszka On Wed, 06/24 18:25, Paolo Bonzini wrote: > From: Jan Kiszka > > The MMIO case is further broken up in two cases: if the caller does not > hold the BQL on invocation, the unlocked one takes or avoids BQL depending > on the locking strategy of the target memory region and its coalesced > MMIO handling. In this case, the caller should not hold _any_ lock > (a friendly suggestion which is disregarded by virtio-scsi-dataplane). > > Signed-off-by: Jan Kiszka > Cc: Frederic Konrad > Message-Id: <1434646046-27150-6-git-send-email-pbonzini@redhat.com> > Signed-off-by: Paolo Bonzini > --- > exec.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 60 insertions(+), 9 deletions(-) > > diff --git a/exec.c b/exec.c > index f2e6603..fd0401e 100644 > --- a/exec.c > +++ b/exec.c > @@ -48,6 +48,7 @@ > #endif > #include "exec/cpu-all.h" > #include "qemu/rcu_queue.h" > +#include "qemu/main-loop.h" > #include "exec/cputlb.h" > #include "translate-all.h" > > @@ -2316,11 +2317,27 @@ static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) > return l; > } > > -static void prepare_mmio_access(MemoryRegion *mr) > +static bool prepare_mmio_access(MemoryRegion *mr) > { > + bool unlocked = !qemu_mutex_iothread_locked(); > + bool release_lock = false; > + > + if (unlocked && mr->global_locking) { > + qemu_mutex_lock_iothread(); > + unlocked = false; > + release_lock = true; > + } > if (mr->flush_coalesced_mmio) { > + if (unlocked) { > + qemu_mutex_lock_iothread(); > + } > qemu_flush_coalesced_mmio_buffer(); > + if (unlocked) { > + qemu_mutex_unlock_iothread(); > + } > } > + > + return release_lock; > } > > MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, > @@ -2332,6 +2349,7 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, > hwaddr addr1; > MemoryRegion *mr; > MemTxResult result = MEMTX_OK; > + bool release_lock = false; > > rcu_read_lock(); > while (len > 0) { > @@ -2340,7 +2358,7 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, > > if (is_write) { > if (!memory_access_is_direct(mr, is_write)) { > - prepare_mmio_access(mr); > + release_lock |= prepare_mmio_access(mr); > l = memory_access_size(mr, l, addr1); > /* XXX: could force current_cpu to NULL to avoid > potential bugs */ > @@ -2382,7 +2400,7 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, > } else { > if (!memory_access_is_direct(mr, is_write)) { > /* I/O case */ > - prepare_mmio_access(mr); > + release_lock |= prepare_mmio_access(mr); > l = memory_access_size(mr, l, addr1); > switch (l) { > case 8: > @@ -2418,6 +2436,12 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, > memcpy(buf, ptr, l); > } > } > + > + if (release_lock) { > + qemu_mutex_unlock_iothread(); > + release_lock = false; > + } > + > len -= l; > buf += l; > addr += l; > @@ -2744,11 +2768,12 @@ static inline uint32_t address_space_ldl_internal(AddressSpace *as, hwaddr addr, > hwaddr l = 4; > hwaddr addr1; > MemTxResult r; > + bool release_lock = false; > > rcu_read_lock(); > mr = address_space_translate(as, addr, &addr1, &l, false); > if (l < 4 || !memory_access_is_direct(mr, false)) { > - prepare_mmio_access(mr); > + release_lock |= prepare_mmio_access(mr); > > /* I/O case */ > r = memory_region_dispatch_read(mr, addr1, &val, 4, attrs); > @@ -2782,6 +2807,9 @@ static inline uint32_t address_space_ldl_internal(AddressSpace *as, hwaddr addr, > if (result) { > *result = r; > } > + if (release_lock) { > + qemu_mutex_unlock_iothread(); > + } > rcu_read_unlock(); > return val; > } > @@ -2834,12 +2862,13 @@ static inline uint64_t address_space_ldq_internal(AddressSpace *as, hwaddr addr, > hwaddr l = 8; > hwaddr addr1; > MemTxResult r; > + bool release_lock = false; > > rcu_read_lock(); > mr = address_space_translate(as, addr, &addr1, &l, > false); > if (l < 8 || !memory_access_is_direct(mr, false)) { > - prepare_mmio_access(mr); > + release_lock |= prepare_mmio_access(mr); > > /* I/O case */ > r = memory_region_dispatch_read(mr, addr1, &val, 8, attrs); > @@ -2873,6 +2902,9 @@ static inline uint64_t address_space_ldq_internal(AddressSpace *as, hwaddr addr, > if (result) { > *result = r; > } > + if (release_lock) { > + qemu_mutex_unlock_iothread(); > + } > rcu_read_unlock(); > return val; > } > @@ -2945,12 +2977,13 @@ static inline uint32_t address_space_lduw_internal(AddressSpace *as, > hwaddr l = 2; > hwaddr addr1; > MemTxResult r; > + bool release_lock = false; > > rcu_read_lock(); > mr = address_space_translate(as, addr, &addr1, &l, > false); > if (l < 2 || !memory_access_is_direct(mr, false)) { > - prepare_mmio_access(mr); > + release_lock |= prepare_mmio_access(mr); > > /* I/O case */ > r = memory_region_dispatch_read(mr, addr1, &val, 2, attrs); > @@ -2984,6 +3017,9 @@ static inline uint32_t address_space_lduw_internal(AddressSpace *as, > if (result) { > *result = r; > } > + if (release_lock) { > + qemu_mutex_unlock_iothread(); > + } > rcu_read_unlock(); > return val; > } > @@ -3036,12 +3072,13 @@ void address_space_stl_notdirty(AddressSpace *as, hwaddr addr, uint32_t val, > hwaddr addr1; > MemTxResult r; > uint8_t dirty_log_mask; > + bool release_lock = false; > > rcu_read_lock(); > mr = address_space_translate(as, addr, &addr1, &l, > true); > if (l < 4 || !memory_access_is_direct(mr, true)) { > - prepare_mmio_access(mr); > + release_lock |= prepare_mmio_access(mr); > > r = memory_region_dispatch_write(mr, addr1, val, 4, attrs); > } else { > @@ -3057,6 +3094,9 @@ void address_space_stl_notdirty(AddressSpace *as, hwaddr addr, uint32_t val, > if (result) { > *result = r; > } > + if (release_lock) { > + qemu_mutex_unlock_iothread(); > + } > rcu_read_unlock(); > } > > @@ -3077,12 +3117,13 @@ static inline void address_space_stl_internal(AddressSpace *as, > hwaddr l = 4; > hwaddr addr1; > MemTxResult r; > + bool release_lock = false; > > rcu_read_lock(); > mr = address_space_translate(as, addr, &addr1, &l, > true); > if (l < 4 || !memory_access_is_direct(mr, true)) { > - prepare_mmio_access(mr); > + release_lock |= prepare_mmio_access(mr); > > #if defined(TARGET_WORDS_BIGENDIAN) > if (endian == DEVICE_LITTLE_ENDIAN) { > @@ -3115,6 +3156,9 @@ static inline void address_space_stl_internal(AddressSpace *as, > if (result) { > *result = r; > } > + if (release_lock) { > + qemu_mutex_unlock_iothread(); > + } > rcu_read_unlock(); > } > > @@ -3184,11 +3228,12 @@ static inline void address_space_stw_internal(AddressSpace *as, > hwaddr l = 2; > hwaddr addr1; > MemTxResult r; > + bool release_lock = false; > > rcu_read_lock(); > mr = address_space_translate(as, addr, &addr1, &l, true); > if (l < 2 || !memory_access_is_direct(mr, true)) { > - prepare_mmio_access(mr); > + release_lock |= prepare_mmio_access(mr); > > #if defined(TARGET_WORDS_BIGENDIAN) > if (endian == DEVICE_LITTLE_ENDIAN) { > @@ -3221,6 +3266,12 @@ static inline void address_space_stw_internal(AddressSpace *as, > if (result) { > *result = r; > } > + if (release_lock) { > + qemu_mutex_unlock_iothread(); > + } > + if (release_lock) { > + qemu_mutex_unlock_iothread(); > + } Bad rebase? Fam > rcu_read_unlock(); > } > > -- > 1.8.3.1 > >