Re: [PATCH] unalias rework

[Izik Eidus <ieidus@redhat.com>]

Marcelo Tosatti wrote:
> Hi Izik,
>
> On Thu, Sep 04, 2008 at 05:13:20PM +0300, izik eidus wrote:
>  
>> +        struct kvm_memory_slot *alias_slot = &kvm->memslots[i];
>> +
>> +        if (alias_slot->base_gfn == slot->base_gfn)
>> +            return 1;
>> +    }
>> +    return 0;
>> +}
>> +
>> +static void update_alias_slots(struct kvm *kvm, struct 
>> kvm_memory_slot *free)
>> +{
>> +    int i;
>> +
>> +    if (is_aliased_slot(kvm, free))
>> +        return;
>> +
>> +    for (i = KVM_MEMORY_SLOTS; i < KVM_MEMORY_SLOTS + KVM_ALIAS_SLOTS;
>> +         ++i) {
>> +        struct kvm_memory_slot *alias_memslot = &kvm->memslots[i];
>> +        unsigned long size = free->npages << PAGE_SHIFT;
>> +
>> +        if (alias_memslot->userspace_addr >= free->userspace_addr &&
>> +            alias_memslot->userspace_addr < free->userspace_addr +
>> +            size) {
>> +            alias_memslot->flags = free->flags;
>> +            if (free->dirty_bitmap) {
>> +                unsigned long offset =
>> +                        alias_memslot->userspace_addr -
>> +                        free->userspace_addr;
>> +                unsigned dirty_offset;
>> +                unsigned long bitmap_addr;
>> +
>> +                offset = offset >> PAGE_SHIFT;
>> +                dirty_offset = ALIGN(offset, BITS_PER_LONG) / 8;
>> +                bitmap_addr = (unsigned long) free->dirty_bitmap;
>> +                bitmap_addr += dirty_offset;
>> +                alias_memslot->dirty_bitmap = (unsigned long *) 
>> bitmap_addr;
>> +            } else
>> +                alias_memslot->dirty_bitmap = NULL;
>> +        }
>> +    }
>> +}
>> +
>>  /*
>>   * Free any memory in @free but not in @dont.
>>   */
>> -static void kvm_free_physmem_slot(struct kvm_memory_slot *free,
>> +static void kvm_free_physmem_slot(struct kvm *kvm,
>> +                  struct kvm_memory_slot *free,
>>                    struct kvm_memory_slot *dont)
>>  {
>>      if (!dont || free->rmap != dont->rmap)
>> @@ -385,10 +433,16 @@ static void kvm_free_physmem_slot(struct 
>> kvm_memory_slot *free,
>>      if (!dont || free->lpage_info != dont->lpage_info)
>>          vfree(free->lpage_info);
>>  
>> -    free->npages = 0;
>>      free->dirty_bitmap = NULL;
>>      free->rmap = NULL;
>>      free->lpage_info = NULL;
>> +
>> +#ifdef CONFIG_X86
>> +    update_alias_slots(kvm, free);
>> +    if (dont)
>> +        update_alias_slots(kvm, dont);
>> +#endif
>> +    free->npages = 0;
>>     
>
> Why is this needed? I don't understand when would you free a memslot
> without freeing any aliases that map it first?
>
>   
(sent it already, but for some reason it didnt reach to the mailing list)
beacuse in case of dont != NULL, we actually dont free the memslot...