From mboxrd@z Thu Jan  1 00:00:00 1970
From: Izik Eidus <izik@qumranet.com>
Subject: Re: [PATCH] unalias rework
Date: Fri, 26 Sep 2008 22:09:24 +0300
Message-ID: <48DD3364.2030406@qumranet.com>
References: <48BFED00.80006@qumranet.com> <20080926001543.GA29572@dmt.cnet>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: kvm@vger.kernel.org
To: Marcelo Tosatti <mtosatti@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from il.qumranet.com ([212.179.150.194]:51000 "EHLO il.qumranet.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752522AbYIZTJW (ORCPT <rfc822;kvm@vger.kernel.org>);
	Fri, 26 Sep 2008 15:09:22 -0400
In-Reply-To: <20080926001543.GA29572@dmt.cnet>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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?
>
>   
beacuse in case of dont != NULL, we actually dont free the memslot...