[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 09/15] memory: prepare flatview and radix-tree f
From: |
liu ping fan |
Subject: |
Re: [Qemu-devel] [PATCH 09/15] memory: prepare flatview and radix-tree for rcu style access |
Date: |
Sat, 11 Aug 2012 09:58:44 +0800 |
On Wed, Aug 8, 2012 at 5:41 PM, Avi Kivity <address@hidden> wrote:
> On 08/08/2012 09:25 AM, Liu Ping Fan wrote:
>> From: Liu Ping Fan <address@hidden>
>>
>> Flatview and radix view are all under the protection of pointer.
>> And this make sure the change of them seem to be atomic!
>>
>> The mr accessed by radix-tree leaf or flatview will be reclaimed
>> after the prev PhysMap not in use any longer
>>
>
> IMO this cleverness should come much later. Let's first take care of
> dropping the big qemu lock, then make swithcing memory maps more efficient.
>
> The initial paths could look like:
>
> lookup:
> take mem_map_lock
> lookup
> take ref
> drop mem_map_lock
>
> update:
> take mem_map_lock (in core_begin)
> do updates
> drop memo_map_lock
>
> Later we can replace mem_map_lock with either a rwlock or (real) rcu.
>
>
>>
>> #if !defined(CONFIG_USER_ONLY)
>>
>> -static void phys_map_node_reserve(unsigned nodes)
>> +static void phys_map_node_reserve(PhysMap *map, unsigned nodes)
>> {
>> - if (phys_map_nodes_nb + nodes > phys_map_nodes_nb_alloc) {
>> + if (map->phys_map_nodes_nb + nodes > map->phys_map_nodes_nb_alloc) {
>> typedef PhysPageEntry Node[L2_SIZE];
>> - phys_map_nodes_nb_alloc = MAX(phys_map_nodes_nb_alloc * 2, 16);
>> - phys_map_nodes_nb_alloc = MAX(phys_map_nodes_nb_alloc,
>> - phys_map_nodes_nb + nodes);
>> - phys_map_nodes = g_renew(Node, phys_map_nodes,
>> - phys_map_nodes_nb_alloc);
>> + map->phys_map_nodes_nb_alloc = MAX(map->phys_map_nodes_nb_alloc * 2,
>> + 16);
>> + map->phys_map_nodes_nb_alloc = MAX(map->phys_map_nodes_nb_alloc,
>> + map->phys_map_nodes_nb + nodes);
>> + map->phys_map_nodes = g_renew(Node, map->phys_map_nodes,
>> + map->phys_map_nodes_nb_alloc);
>> }
>> }
>
> Please have a patch that just adds the map parameter to all these
> functions. This makes the later patch, that adds the copy, easier to read.
>
>> +
>> +void cur_map_update(PhysMap *next)
>> +{
>> + qemu_mutex_lock(&cur_map_lock);
>> + physmap_put(cur_map);
>> + cur_map = next;
>> + smp_mb();
>> + qemu_mutex_unlock(&cur_map_lock);
>> +}
>
> IMO this can be mem_map_lock.
>
> If we take my previous suggestion:
>
> lookup:
> take mem_map_lock
> lookup
> take ref
> drop mem_map_lock
>
> update:
> take mem_map_lock (in core_begin)
> do updates
> drop memo_map_lock
>
> And update it to
>
>
> update:
> prepare next_map (in core_begin)
> do updates
> take mem_map_lock (in core_commit)
> switch maps
> drop mem_map_lock
> free old map
>
>
> Note the lookup path copies the MemoryRegionSection instead of
> referencing it. Thus we can destroy the old map without worrying; the
> only pointers will point to MemoryRegions, which will be protected by
> the refcounts on their Objects.
>
Just find there may be a leak here. If mrs points to subpage, then the
subpage_t could be crashed by destroy.
To avoid such situation, we can walk down the chain to pin us on the
Object based mr, but then we must expose the address convert in
subpage_read() right here. Right?
Regards,
pingfan
> This can be easily switched to rcu:
>
> update:
> prepare next_map (in core_begin)
> do updates
> switch maps - rcu_assign_pointer
> call_rcu(free old map) (or synchronize_rcu; free old maps)
>
> Again, this should be done after the simplictic patch that enables
> parallel lookup but keeps just one map.
>
>
>
> --
> error compiling committee.c: too many arguments to function
- Re: [Qemu-devel] [PATCH 05/15] memory: introduce life_ops to MemoryRegion, (continued)
[Qemu-devel] [PATCH 10/15] memory: change tcg related code to using PhysMap, Liu Ping Fan, 2012/08/08
[Qemu-devel] [PATCH 09/15] memory: prepare flatview and radix-tree for rcu style access, Liu Ping Fan, 2012/08/08
Re: [Qemu-devel] [PATCH 09/15] memory: prepare flatview and radix-tree for rcu style access, Blue Swirl, 2012/08/08
[Qemu-devel] [PATCH 13/15] hotplug: introduce qdev_unplug_complete() to remove device from views, Liu Ping Fan, 2012/08/08
- Re: [Qemu-devel] [PATCH 13/15] hotplug: introduce qdev_unplug_complete() to remove device from views, Paolo Bonzini, 2012/08/08
- Re: [Qemu-devel] [PATCH 13/15] hotplug: introduce qdev_unplug_complete() to remove device from views, Avi Kivity, 2012/08/08
- Re: [Qemu-devel] [PATCH 13/15] hotplug: introduce qdev_unplug_complete() to remove device from views, liu ping fan, 2012/08/09
- Re: [Qemu-devel] [PATCH 13/15] hotplug: introduce qdev_unplug_complete() to remove device from views, Paolo Bonzini, 2012/08/09
- Re: [Qemu-devel] [PATCH 13/15] hotplug: introduce qdev_unplug_complete() to remove device from views, liu ping fan, 2012/08/10
- Re: [Qemu-devel] [PATCH 13/15] hotplug: introduce qdev_unplug_complete() to remove device from views, Marcelo Tosatti, 2012/08/13