On 18/09/2017 12:17, Alexey Kardashevskiy wrote:
> + physmr = memory_region_unalias_entire(old_view->root);
> +
> + key = !physmr->enabled ? 0 : physmr;
> + new_view = (FlatView *) g_hash_table_lookup(views, key);
> + if (new_view) {
> + continue;
> + }
> +
> + new_view = generate_memory_topology(physmr);
> +
> + new_view->dispatch = address_space_dispatch_new(new_view);
> + for (i = 0; i < new_view->nr; i++) {
> + MemoryRegionSection mrs =
> + section_from_flat_range(&new_view->ranges[i], new_view);
> + flatview_add_to_dispatch(new_view, &mrs);
> + }
> + address_space_dispatch_compact(new_view->dispatch);
> +
> + g_hash_table_insert(views, key, new_view);
> + }
> +
> + /* Replace FVs in ASes */
> + QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> + old_view = as->current_map;
> + physmr = memory_region_unalias_entire(old_view->root);
> +
> + key = !physmr->enabled ? 0 : physmr;
This is duplicate, perhaps memory_region_unalias_entire should do it
instead? Does it make sense for flatview->root to be NULL, or does it
break something else? If something breaks, disregard the remaining
comments.
(BTW, maybe you can rename the function to memory_region_get_flatview_root).
>
> +
> + /* Unref FVs from temporary table */
> + g_hash_table_foreach_remove(views, flatview_unref_g_hash, 0);
> + g_hash_table_unref(views);
> }
You can avoid g_hash_table_foreach_remove and also flatview_unref_g_hash
instead use g_hash_table_new_full (casting flatview_unref to
GDestroyNotify should work fine).
>
> @@ -2690,13 +2721,6 @@ AddressSpace
> *address_space_init_shareable(MemoryRegion *root, const char *name)
> {
> AddressSpace *as;
>
> - QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> - if (root == as->root && as->malloced) {
> - as->ref_count++;
> - return as;
> - }
> - }
> -
> as = g_malloc0(sizeof *as);
> address_space_init(as, root, name);
> as->malloced = true;
Is this on purpose because it's now pointless? Maybe it should be
pointed out in the commit message.
Paolo
