On Wed, Aug 8, 2012 at 5:13 PM, Avi Kivity <avi@xxxxxxxxxx> wrote:
> On 08/08/2012 09:25 AM, Liu Ping Fan wrote:
>> From: Liu Ping Fan <pingfank@xxxxxxxxxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxxxxxxx>
>> ---
>> 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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
