From: liu ping fan <qemulist@gmail.com> To: Avi Kivity <avi@redhat.com> Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org, "Anthony Liguori" <anthony@codemonkey.ws>, "Jan Kiszka" <jan.kiszka@siemens.com>, "Marcelo Tosatti" <mtosatti@redhat.com>, "Stefan Hajnoczi" <stefanha@gmail.com>, "Paolo Bonzini" <pbonzini@redhat.com>, "Blue Swirl" <blauwirbel@gmail.com>, "Andreas Färber" <afaerber@suse.de> Subject: Re: [PATCH 04/15] memory: MemoryRegion topology must be stable when updating Date: Thu, 9 Aug 2012 15:28:44 +0800 [thread overview] Message-ID: <CAJnKYQmga1h-SGEOM31xO1hd=SnRtCUoFbV9b8W=WDv=fj+E+w@mail.gmail.com> (raw) In-Reply-To: <50222DB6.8020505@redhat.com> On Wed, Aug 8, 2012 at 5:13 PM, Avi Kivity <avi@redhat.com> wrote: > On 08/08/2012 09:25 AM, Liu Ping Fan wrote: >> From: Liu Ping Fan <pingfank@linux.vnet.ibm.com> >> >> Using mem_map_lock to protect among updaters. So we can get the intact >> snapshot of mem topology -- FlatView & radix-tree. >> >> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com> >> --- >> exec.c | 3 +++ >> memory.c | 22 ++++++++++++++++++++++ >> memory.h | 2 ++ >> 3 files changed, 27 insertions(+), 0 deletions(-) >> >> diff --git a/exec.c b/exec.c >> index 8244d54..0e29ef9 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -210,6 +210,8 @@ static unsigned phys_map_nodes_nb, phys_map_nodes_nb_alloc; >> The bottom level has pointers to MemoryRegionSections. */ >> static PhysPageEntry phys_map = { .ptr = PHYS_MAP_NODE_NIL, .is_leaf = 0 }; >> >> +QemuMutex mem_map_lock; >> + >> static void io_mem_init(void); >> static void memory_map_init(void); >> >> @@ -637,6 +639,7 @@ void cpu_exec_init_all(void) >> #if !defined(CONFIG_USER_ONLY) >> memory_map_init(); >> io_mem_init(); >> + qemu_mutex_init(&mem_map_lock); >> #endif >> } >> >> diff --git a/memory.c b/memory.c >> index aab4a31..5986532 100644 >> --- a/memory.c >> +++ b/memory.c >> @@ -761,7 +761,9 @@ void memory_region_transaction_commit(void) >> assert(memory_region_transaction_depth); >> --memory_region_transaction_depth; >> if (!memory_region_transaction_depth && memory_region_update_pending) { >> + qemu_mutex_lock(&mem_map_lock); >> memory_region_update_topology(NULL); >> + qemu_mutex_unlock(&mem_map_lock); >> } >> } > > Seems to me that nothing in memory.c can susceptible to races. It must > already be called under the big qemu lock, and with the exception of > mutators (memory_region_set_*), changes aren't directly visible. > Yes, what I want to do is "prepare unplug out of protection of global lock". When io-dispatch and mmio-dispatch are all out of big lock, we will run into the following scene: In vcpu context A, qdev_unplug_complete()-> delete subregion; In context B, write pci bar --> pci mapping update -> add subregion > I think it's sufficient to take the mem_map_lock at the beginning of > core_begin() and drop it at the end of core_commit(). That means all > updates of volatile state, phys_map, are protected. > The mem_map_lock is to protect both address_space_io and address_space_memory. When without the protection of big lock, competing will raise among the updaters (memory_region_{add,del}_subregion and the readers generate_memory_topology()->render_memory_region(). If just in core_begin/commit, we will duplicate it for xx_begin/commit, right? And at the same time, mr->subregions is exposed under SMP without big lock. Thanks and regards, pingfan > > > -- > error compiling committee.c: too many arguments to function
WARNING: multiple messages have this Message-ID (diff)
From: liu ping fan <qemulist@gmail.com> To: Avi Kivity <avi@redhat.com> Cc: kvm@vger.kernel.org, "Jan Kiszka" <jan.kiszka@siemens.com>, "Marcelo Tosatti" <mtosatti@redhat.com>, qemu-devel@nongnu.org, "Blue Swirl" <blauwirbel@gmail.com>, "Anthony Liguori" <anthony@codemonkey.ws>, "Stefan Hajnoczi" <stefanha@gmail.com>, "Paolo Bonzini" <pbonzini@redhat.com>, "Andreas Färber" <afaerber@suse.de> Subject: Re: [Qemu-devel] [PATCH 04/15] memory: MemoryRegion topology must be stable when updating Date: Thu, 9 Aug 2012 15:28:44 +0800 [thread overview] Message-ID: <CAJnKYQmga1h-SGEOM31xO1hd=SnRtCUoFbV9b8W=WDv=fj+E+w@mail.gmail.com> (raw) In-Reply-To: <50222DB6.8020505@redhat.com> On Wed, Aug 8, 2012 at 5:13 PM, Avi Kivity <avi@redhat.com> wrote: > On 08/08/2012 09:25 AM, Liu Ping Fan wrote: >> From: Liu Ping Fan <pingfank@linux.vnet.ibm.com> >> >> Using mem_map_lock to protect among updaters. So we can get the intact >> snapshot of mem topology -- FlatView & radix-tree. >> >> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com> >> --- >> exec.c | 3 +++ >> memory.c | 22 ++++++++++++++++++++++ >> memory.h | 2 ++ >> 3 files changed, 27 insertions(+), 0 deletions(-) >> >> diff --git a/exec.c b/exec.c >> index 8244d54..0e29ef9 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -210,6 +210,8 @@ static unsigned phys_map_nodes_nb, phys_map_nodes_nb_alloc; >> The bottom level has pointers to MemoryRegionSections. */ >> static PhysPageEntry phys_map = { .ptr = PHYS_MAP_NODE_NIL, .is_leaf = 0 }; >> >> +QemuMutex mem_map_lock; >> + >> static void io_mem_init(void); >> static void memory_map_init(void); >> >> @@ -637,6 +639,7 @@ void cpu_exec_init_all(void) >> #if !defined(CONFIG_USER_ONLY) >> memory_map_init(); >> io_mem_init(); >> + qemu_mutex_init(&mem_map_lock); >> #endif >> } >> >> diff --git a/memory.c b/memory.c >> index aab4a31..5986532 100644 >> --- a/memory.c >> +++ b/memory.c >> @@ -761,7 +761,9 @@ void memory_region_transaction_commit(void) >> assert(memory_region_transaction_depth); >> --memory_region_transaction_depth; >> if (!memory_region_transaction_depth && memory_region_update_pending) { >> + qemu_mutex_lock(&mem_map_lock); >> memory_region_update_topology(NULL); >> + qemu_mutex_unlock(&mem_map_lock); >> } >> } > > Seems to me that nothing in memory.c can susceptible to races. It must > already be called under the big qemu lock, and with the exception of > mutators (memory_region_set_*), changes aren't directly visible. > Yes, what I want to do is "prepare unplug out of protection of global lock". When io-dispatch and mmio-dispatch are all out of big lock, we will run into the following scene: In vcpu context A, qdev_unplug_complete()-> delete subregion; In context B, write pci bar --> pci mapping update -> add subregion > I think it's sufficient to take the mem_map_lock at the beginning of > core_begin() and drop it at the end of core_commit(). That means all > updates of volatile state, phys_map, are protected. > The mem_map_lock is to protect both address_space_io and address_space_memory. When without the protection of big lock, competing will raise among the updaters (memory_region_{add,del}_subregion and the readers generate_memory_topology()->render_memory_region(). If just in core_begin/commit, we will duplicate it for xx_begin/commit, right? And at the same time, mr->subregions is exposed under SMP without big lock. Thanks and regards, pingfan > > > -- > error compiling committee.c: too many arguments to function
next prev parent reply other threads:[~2012-08-09 7:29 UTC|newest] Thread overview: 154+ messages / expand[flat|nested] mbox.gz Atom feed top 2012-08-08 6:25 [PATCH 0/15 v2] prepare unplug out of protection of global lock Liu Ping Fan 2012-08-08 6:25 ` [Qemu-devel] " Liu Ping Fan 2012-08-08 6:25 ` [PATCH 01/15] atomic: introduce atomic operations Liu Ping Fan 2012-08-08 6:25 ` [Qemu-devel] " Liu Ping Fan 2012-08-08 8:55 ` Paolo Bonzini 2012-08-08 8:55 ` [Qemu-devel] " Paolo Bonzini 2012-08-08 9:02 ` Avi Kivity 2012-08-08 9:02 ` [Qemu-devel] " Avi Kivity 2012-08-08 9:05 ` 陳韋任 (Wei-Ren Chen) 2012-08-08 9:05 ` 陳韋任 (Wei-Ren Chen) 2012-08-08 9:15 ` Avi Kivity 2012-08-08 9:15 ` [Qemu-devel] " Avi Kivity 2012-08-08 9:21 ` Peter Maydell 2012-08-08 9:21 ` Peter Maydell 2012-08-08 13:09 ` Stefan Hajnoczi 2012-08-08 13:09 ` Stefan Hajnoczi 2012-08-08 13:18 ` Paolo Bonzini 2012-08-08 13:18 ` Paolo Bonzini 2012-08-08 13:32 ` Peter Maydell 2012-08-08 13:32 ` [Qemu-devel] " Peter Maydell 2012-08-08 13:49 ` Paolo Bonzini 2012-08-08 13:49 ` [Qemu-devel] " Paolo Bonzini 2012-08-08 14:00 ` Avi Kivity 2012-08-08 14:00 ` [Qemu-devel] " Avi Kivity 2012-08-08 6:25 ` [PATCH 02/15] qom: using atomic ops to re-implement object_ref Liu Ping Fan 2012-08-08 6:25 ` [Qemu-devel] " Liu Ping Fan 2012-08-08 6:25 ` [PATCH 03/15] qom: introduce reclaimer to release obj Liu Ping Fan 2012-08-08 6:25 ` [Qemu-devel] " Liu Ping Fan 2012-08-08 9:05 ` Avi Kivity 2012-08-08 9:05 ` [Qemu-devel] " Avi Kivity 2012-08-08 9:07 ` Paolo Bonzini 2012-08-08 9:07 ` [Qemu-devel] " Paolo Bonzini 2012-08-08 9:15 ` Avi Kivity 2012-08-08 9:15 ` [Qemu-devel] " Avi Kivity 2012-08-09 7:33 ` liu ping fan 2012-08-09 7:33 ` [Qemu-devel] " liu ping fan 2012-08-09 7:49 ` Paolo Bonzini 2012-08-09 7:49 ` [Qemu-devel] " Paolo Bonzini 2012-08-09 8:18 ` Avi Kivity 2012-08-09 8:18 ` [Qemu-devel] " Avi Kivity 2012-08-10 6:43 ` liu ping fan 2012-08-10 6:43 ` [Qemu-devel] " liu ping fan 2012-08-08 9:35 ` Paolo Bonzini 2012-08-08 9:35 ` [Qemu-devel] " Paolo Bonzini 2012-08-09 7:38 ` liu ping fan 2012-08-09 7:38 ` [Qemu-devel] " liu ping fan 2012-08-08 6:25 ` [PATCH 04/15] memory: MemoryRegion topology must be stable when updating Liu Ping Fan 2012-08-08 6:25 ` [Qemu-devel] " Liu Ping Fan 2012-08-08 9:13 ` Avi Kivity 2012-08-08 9:13 ` [Qemu-devel] " Avi Kivity 2012-08-09 7:28 ` liu ping fan [this message] 2012-08-09 7:28 ` liu ping fan 2012-08-09 8:24 ` Avi Kivity 2012-08-09 8:24 ` [Qemu-devel] " Avi Kivity 2012-08-10 6:44 ` liu ping fan 2012-08-10 6:44 ` [Qemu-devel] " liu ping fan 2012-08-13 18:28 ` Marcelo Tosatti 2012-08-13 18:28 ` [Qemu-devel] " Marcelo Tosatti 2012-08-08 19:17 ` Blue Swirl 2012-08-08 19:17 ` [Qemu-devel] " Blue Swirl 2012-08-09 7:28 ` liu ping fan 2012-08-09 7:28 ` [Qemu-devel] " liu ping fan 2012-08-09 17:09 ` Blue Swirl 2012-08-09 17:09 ` [Qemu-devel] " Blue Swirl 2012-08-08 6:25 ` [PATCH 05/15] memory: introduce life_ops to MemoryRegion Liu Ping Fan 2012-08-08 6:25 ` [Qemu-devel] " Liu Ping Fan 2012-08-08 9:18 ` Avi Kivity 2012-08-08 9:18 ` [Qemu-devel] " Avi Kivity 2012-08-08 6:25 ` [PATCH 06/15] memory: use refcnt to manage MemoryRegion Liu Ping Fan 2012-08-08 6:25 ` [Qemu-devel] " Liu Ping Fan 2012-08-08 9:20 ` Avi Kivity 2012-08-08 9:20 ` [Qemu-devel] " Avi Kivity 2012-08-09 7:27 ` liu ping fan 2012-08-09 7:27 ` [Qemu-devel] " liu ping fan 2012-08-09 8:38 ` Avi Kivity 2012-08-09 8:38 ` [Qemu-devel] " Avi Kivity 2012-08-10 6:44 ` liu ping fan 2012-08-10 6:44 ` [Qemu-devel] " liu ping fan 2012-08-12 8:43 ` Avi Kivity 2012-08-12 8:43 ` [Qemu-devel] " Avi Kivity 2012-08-08 6:25 ` [PATCH 07/15] memory: inc/dec mr's ref when adding/removing from mem view Liu Ping Fan 2012-08-08 6:25 ` [Qemu-devel] " Liu Ping Fan 2012-08-08 6:25 ` [PATCH 08/15] memory: introduce PhysMap to present snapshot of toploygy Liu Ping Fan 2012-08-08 6:25 ` [Qemu-devel] " Liu Ping Fan 2012-08-08 9:27 ` Avi Kivity 2012-08-08 9:27 ` [Qemu-devel] " Avi Kivity 2012-08-08 19:18 ` Blue Swirl 2012-08-08 19:18 ` [Qemu-devel] " Blue Swirl 2012-08-09 7:29 ` liu ping fan 2012-08-09 7:29 ` [Qemu-devel] " liu ping fan 2012-08-08 6:25 ` [PATCH 09/15] memory: prepare flatview and radix-tree for rcu style access Liu Ping Fan 2012-08-08 6:25 ` [Qemu-devel] " Liu Ping Fan 2012-08-08 9:41 ` Avi Kivity 2012-08-08 9:41 ` [Qemu-devel] " Avi Kivity 2012-08-11 1:58 ` liu ping fan 2012-08-11 1:58 ` [Qemu-devel] " liu ping fan 2012-08-11 10:06 ` liu ping fan 2012-08-11 10:06 ` [Qemu-devel] " liu ping fan 2012-08-08 19:23 ` Blue Swirl 2012-08-08 19:23 ` [Qemu-devel] " Blue Swirl 2012-08-09 7:29 ` liu ping fan 2012-08-09 7:29 ` [Qemu-devel] " liu ping fan 2012-08-08 6:25 ` [PATCH 10/15] memory: change tcg related code to using PhysMap Liu Ping Fan 2012-08-08 6:25 ` [Qemu-devel] " Liu Ping Fan 2012-08-08 6:25 ` [PATCH 11/15] lock: introduce global lock for device tree Liu Ping Fan 2012-08-08 6:25 ` [Qemu-devel] " Liu Ping Fan 2012-08-08 9:41 ` Paolo Bonzini 2012-08-08 9:41 ` [Qemu-devel] " Paolo Bonzini 2012-08-09 7:28 ` liu ping fan 2012-08-09 7:28 ` [Qemu-devel] " liu ping fan 2012-08-09 7:41 ` Paolo Bonzini 2012-08-09 7:41 ` [Qemu-devel] " Paolo Bonzini 2012-08-08 9:42 ` Avi Kivity 2012-08-08 9:42 ` [Qemu-devel] " Avi Kivity 2012-08-09 7:27 ` liu ping fan 2012-08-09 7:27 ` [Qemu-devel] " liu ping fan 2012-08-09 8:31 ` Avi Kivity 2012-08-09 8:31 ` [Qemu-devel] " Avi Kivity 2012-08-08 6:25 ` [PATCH 12/15] qdev: using devtree lock to protect device's accessing Liu Ping Fan 2012-08-08 6:25 ` [Qemu-devel] " Liu Ping Fan 2012-08-08 9:33 ` Peter Maydell 2012-08-08 9:33 ` [Qemu-devel] " Peter Maydell 2012-08-08 6:25 ` [PATCH 13/15] hotplug: introduce qdev_unplug_complete() to remove device from views Liu Ping Fan 2012-08-08 6:25 ` [Qemu-devel] " Liu Ping Fan 2012-08-08 9:52 ` Paolo Bonzini 2012-08-08 9:52 ` [Qemu-devel] " Paolo Bonzini 2012-08-08 10:07 ` Avi Kivity 2012-08-08 10:07 ` [Qemu-devel] " Avi Kivity 2012-08-09 7:28 ` liu ping fan 2012-08-09 7:28 ` [Qemu-devel] " liu ping fan 2012-08-09 8:00 ` Paolo Bonzini 2012-08-09 8:00 ` [Qemu-devel] " Paolo Bonzini 2012-08-10 6:42 ` liu ping fan 2012-08-10 6:42 ` [Qemu-devel] " liu ping fan 2012-08-13 18:53 ` Marcelo Tosatti 2012-08-13 18:53 ` [Qemu-devel] " Marcelo Tosatti 2012-08-13 18:51 ` Marcelo Tosatti 2012-08-13 18:51 ` [Qemu-devel] " Marcelo Tosatti 2012-08-08 6:25 ` [PATCH 14/15] qom: object_unref call reclaimer Liu Ping Fan 2012-08-08 6:25 ` [Qemu-devel] " Liu Ping Fan 2012-08-08 9:40 ` Paolo Bonzini 2012-08-08 9:40 ` [Qemu-devel] " Paolo Bonzini 2012-08-13 18:56 ` Marcelo Tosatti 2012-08-13 18:56 ` [Qemu-devel] " Marcelo Tosatti 2012-08-08 6:25 ` [PATCH 15/15] e1000: using new interface--unmap to unplug Liu Ping Fan 2012-08-08 6:25 ` [Qemu-devel] " Liu Ping Fan 2012-08-08 9:56 ` Paolo Bonzini 2012-08-08 9:56 ` [Qemu-devel] " Paolo Bonzini 2012-08-09 7:28 ` liu ping fan 2012-08-09 7:28 ` [Qemu-devel] " liu ping fan 2012-08-09 7:40 ` Paolo Bonzini 2012-08-09 7:40 ` [Qemu-devel] " Paolo Bonzini 2012-08-10 6:43 ` liu ping fan 2012-08-10 6:43 ` [Qemu-devel] " liu ping fan
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAJnKYQmga1h-SGEOM31xO1hd=SnRtCUoFbV9b8W=WDv=fj+E+w@mail.gmail.com' \ --to=qemulist@gmail.com \ --cc=afaerber@suse.de \ --cc=anthony@codemonkey.ws \ --cc=avi@redhat.com \ --cc=blauwirbel@gmail.com \ --cc=jan.kiszka@siemens.com \ --cc=kvm@vger.kernel.org \ --cc=mtosatti@redhat.com \ --cc=pbonzini@redhat.com \ --cc=qemu-devel@nongnu.org \ --cc=stefanha@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.