On 04/14/2012 09:12 AM, Takuya Yoshikawa wrote:
> Hi,
>
> On Wed, 11 Apr 2012 11:11:07 +0800
> Xiao Guangrong <xiaoguangrong@xxxxxxxxxxxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
