From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41815) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TBM80-0007uY-Lc for qemu-devel@nongnu.org; Tue, 11 Sep 2012 04:47:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TBM7s-0007dL-1i for qemu-devel@nongnu.org; Tue, 11 Sep 2012 04:47:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54361) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TBM7r-0007dH-OE for qemu-devel@nongnu.org; Tue, 11 Sep 2012 04:47:19 -0400 Message-ID: <504EFA91.5040100@redhat.com> Date: Tue, 11 Sep 2012 11:47:13 +0300 From: Avi Kivity MIME-Version: 1.0 References: <1347349912-15611-1-git-send-email-qemulist@gmail.com> <1347349912-15611-7-git-send-email-qemulist@gmail.com> In-Reply-To: <1347349912-15611-7-git-send-email-qemulist@gmail.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V3 06/11] memory: make mmio dispatch able to be out of biglock List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Liu Ping Fan Cc: Jan Kiszka , Marcelo Tosatti , qemu-devel@nongnu.org, Anthony Liguori , Paolo Bonzini On 09/11/2012 10:51 AM, Liu Ping Fan wrote: > From: Liu Ping Fan > > Without biglock, we try to protect the mr by increase refcnt. > If we can inc refcnt, go backward and resort to biglock. > > Another point is memory radix-tree can be flushed by another > thread, so we should get the copy of terminal mr to survive > from such issue. > > void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf, > int len, int is_write) > { > @@ -3413,14 +3479,35 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf, > uint8_t *ptr; > uint32_t val; > target_phys_addr_t page; > - MemoryRegionSection *section; > + MemoryRegionSection *section, obj_mrs; > + int ret; > + int need_biglock = 0; > > + /* Will finally removed after all mr->ops implement ref/unref() */ > +try_big_lock: > + if (need_biglock == 1) { > + qemu_mutex_lock_iothread(); > + } > while (len > 0) { > page = addr & TARGET_PAGE_MASK; > l = (page + TARGET_PAGE_SIZE) - addr; > if (l > len) > l = len; > + > + qemu_mutex_lock(&mem_map_lock); > section = phys_page_find(page >> TARGET_PAGE_BITS); > + if (need_biglock == 0) { > + obj_mrs = mrs_get_terminal(section, addr); > + ret = mrs_ref(&obj_mrs); > + if (ret <= 0) { > + need_biglock = 1; > + qemu_mutex_unlock(&mem_map_lock); > + goto try_big_lock; > + } > + /* rely on local variable */ > + section = &obj_mrs; > + } > + qemu_mutex_unlock(&mem_map_lock); > If on the first pass we find that we need the big lock, we may find on the second pass that we don't need it since the memory map has changed. So on the second pass we might actually need to drop the big lock. Could code it like section, need_lock = lookup(...) if need_lock: lock(big_lock) section, need_lock = lookup(...) if not need_lock: unlock(big_lock) dispatch(section) if need_lock: unlock(big_lock) It's ugly as hell, so we'll to apologize to readers in a big comment explaining what's going on. -- error compiling committee.c: too many arguments to function