Re: [RESEND PATCH v2 4/5] kvm: Remove ept_identity_pagetable from struct kvm_arch.

[Tang Chen]

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 linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/