Re: [Qemu-devel] [PATCH v2 03/11] exec: simplify destruction of the phys map

[Paolo Bonzini <pbonzini@redhat.com>]

Il 01/07/2013 16:23, Jan Kiszka ha scritto:
> On 2013-06-28 18:58, Paolo Bonzini wrote:
>> Do not bother visiting the radix tree when an address space is destroyed.
> 
> The "because" is missing, see my comment on the previous patch. If you
> can clarify this, the rest is fine.

After the previous patch, the "because" is that this is has become a
pointless exercise: all you're doing is zeroing out a structure that
will be freed as soon as you come back.

I'll add the justification to both patches.

Paolo

> Jan
> 
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  exec.c | 34 +---------------------------------
>>  1 file changed, 1 insertion(+), 33 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index e7eadf5..9c77285 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -154,12 +154,6 @@ static uint16_t phys_map_node_alloc(void)
>>      return ret;
>>  }
>>  
>> -static void phys_map_nodes_reset(void)
>> -{
>> -    phys_map_nodes_nb = 0;
>> -}
>> -
>> -
>>  static void phys_page_set_level(PhysPageEntry *lp, hwaddr *index,
>>                                  hwaddr *nb, uint16_t leaf,
>>                                  int level)
>> @@ -763,31 +757,6 @@ static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
>>                               uint16_t section);
>>  static subpage_t *subpage_init(AddressSpace *as, hwaddr base);
>>  
>> -static void destroy_l2_mapping(PhysPageEntry *lp, unsigned level)
>> -{
>> -    unsigned i;
>> -    PhysPageEntry *p;
>> -
>> -    if (lp->ptr == PHYS_MAP_NODE_NIL) {
>> -        return;
>> -    }
>> -
>> -    p = phys_map_nodes[lp->ptr];
>> -    for (i = 0; i < L2_SIZE; ++i) {
>> -        if (!p[i].is_leaf) {
>> -            destroy_l2_mapping(&p[i], level - 1);
>> -        }
>> -    }
>> -    lp->is_leaf = 0;
>> -    lp->ptr = PHYS_MAP_NODE_NIL;
>> -}
>> -
>> -static void destroy_all_mappings(AddressSpaceDispatch *d)
>> -{
>> -    destroy_l2_mapping(&d->phys_map, P_L2_LEVELS - 1);
>> -    phys_map_nodes_reset();
>> -}
>> -
>>  static uint16_t phys_section_add(MemoryRegionSection *section)
>>  {
>>      /* The physical section number is ORed with a page-aligned
>> @@ -820,6 +789,7 @@ static void phys_sections_clear(void)
>>          MemoryRegionSection *section = &phys_sections[--phys_sections_nb];
>>          phys_section_destroy(section->mr);
>>      }
>> +    phys_map_nodes_nb = 0;
>>  }
>>  
>>  static void register_subpage(AddressSpaceDispatch *d, MemoryRegionSection *section)
>> @@ -1724,7 +1694,6 @@ static void mem_begin(MemoryListener *listener)
>>  {
>>      AddressSpaceDispatch *d = container_of(listener, AddressSpaceDispatch, listener);
>>  
>> -    destroy_all_mappings(d);
>>      d->phys_map.ptr = PHYS_MAP_NODE_NIL;
>>  }
>>  
>> @@ -1791,7 +1760,6 @@ void address_space_destroy_dispatch(AddressSpace *as)
>>      AddressSpaceDispatch *d = as->dispatch;
>>  
>>      memory_listener_unregister(&d->listener);
>> -    destroy_l2_mapping(&d->phys_map, P_L2_LEVELS - 1);
>>      g_free(d);
>>      as->dispatch = NULL;
>>  }
>>
>