Re: [PATCH v2] KVM: Avoid zapping unrelated shadows in __kvm_set_memory_region()

[Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>]

On 04/14/2012 09:12 AM, Takuya Yoshikawa wrote:

> Hi,
> 
> On Wed, 11 Apr 2012 11:11:07 +0800
> Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com> wrote:
> 
>>>  restart:
>>> -	list_for_each_entry_safe(sp, node, &kvm->arch.active_mmu_pages, link)
>>> -		if (kvm_mmu_prepare_zap_page(kvm, sp, &invalid_list))
>>> -			goto restart;
>>> +	zapped = 0;
>>> +	list_for_each_entry_safe(sp, node, &kvm->arch.active_mmu_pages, link) {
>>> +		if ((slot >= 0) && !test_bit(slot, sp->slot_bitmap))
>>> +			continue;
>>> +
>>> +		zapped |= kvm_mmu_prepare_zap_page(kvm, sp, &invalid_list);
>>
>>
>> You should "goto restart" here like the origin code, also, "safe" version of
>> list_for_each is not needed.
> 
> 
> Thank you for looking into this part.
> 
> I understand that we can eliminate _safe in the original implementation.
> 
> 
> Can you tell me the reason why we should do "goto restart" immediately here?
> For performance, or correctness issue?
> 


kvm_mmu_prepare_zap_page may remove many sp in kvm->arch.active_mmu_pages list
that means the next node cached in list_for_each_entry_safe will become invalid.