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.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to