Messages in this thread | | | Date | Tue, 15 Jul 2014 18:39:25 +0800 | From | Tang Chen <> | Subject | Re: [RESEND PATCH v2 4/5] kvm: Remove ept_identity_pagetable from struct kvm_arch. |
| |
On 07/14/2014 10:27 PM, Gleb Natapov wrote: ...... >>>> if (likely(kvm->arch.ept_identity_pagetable_done)) >>>> return 1; >>>> - ret = 0; >>>> identity_map_pfn = kvm->arch.ept_identity_map_addr>> PAGE_SHIFT; >>>> + >>>> + mutex_lock(&kvm->slots_lock); >>> Why move this out of alloc_identity_pagetable()? >>> >> >> Referring to the original code, I think mutex_lock(&kvm->slots_lock) is used >> to protect kvm->arch.ept_identity_pagetable. If two or more threads try to >> modify it at the same time, the mutex ensures that the identity table is >> only >> allocated once. >> >> Now, we dropped kvm->arch.ept_identity_pagetable. And use >> kvm->arch.ept_identity_pagetable_done >> to check if the identity table is allocated and initialized. So we should >> protect >> memory slot operation in alloc_identity_pagetable() and >> kvm->arch.ept_identity_pagetable_done >> with this mutex. >> >> Of course, I can see that the name "slots_lock" indicates that it may be >> used >> to protect the memory slot operation only. Maybe move it out here is not >> suitable. >> >> If I'm wrong, please tell me. >> > No, you are right that besides memory slot creation slots_lock protects checking > of ept_identity_pagetable here, but after you patch ept_identity_pagetable_done is > tested outside of slots_lock so the allocation can happen twice, no?
Oh, yes. Will fix it in the next version.
Thanks.
| |