On 08/08/2012 09:25 AM, Liu Ping Fan wrote:
> From: Liu Ping Fan <pingfank@xxxxxxxxxxxxxxxxxx>
>
> 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.
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
--
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
