On Wed, Mar 31, 2021 at 3:22 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Wed, Mar 31, 2021, Ben Gardon wrote:
> > In order to parallelize more operations for the TDP MMU, make the
> > refcount on TDP MMU roots atomic, so that a future patch can allow
> > multiple threads to take a reference on the root concurrently, while
> > holding the MMU lock in read mode.
> >
> > Signed-off-by: Ben Gardon <bgardon@xxxxxxxxxx>
> > ---
>
> ...
>
> > @@ -88,10 +88,12 @@ static struct kvm_mmu_page *tdp_mmu_next_root(struct kvm *kvm,
> > next_root = list_first_entry(&kvm->arch.tdp_mmu_roots,
> > typeof(*next_root), link);
> >
> > + while (!list_entry_is_head(next_root, &kvm->arch.tdp_mmu_roots, link) &&
> > + !kvm_tdp_mmu_get_root(kvm, next_root))
> > + next_root = list_next_entry(next_root, link);
> > +
> > if (list_entry_is_head(next_root, &kvm->arch.tdp_mmu_roots, link))
> > next_root = NULL;
> > - else
> > - kvm_tdp_mmu_get_root(kvm, next_root);
> >
> > if (prev_root)
> > kvm_tdp_mmu_put_root(kvm, prev_root);
> > @@ -158,14 +160,13 @@ hpa_t kvm_tdp_mmu_get_vcpu_root_hpa(struct kvm_vcpu *vcpu)
> >
> > /* Check for an existing root before allocating a new one. */
> > for_each_tdp_mmu_root(kvm, root) {
> > - if (root->role.word == role.word) {
> > - kvm_tdp_mmu_get_root(kvm, root);
> > + if (root->role.word == role.word &&
> > + kvm_tdp_mmu_get_root(kvm, root))
>
> I'm not opposed to changing this logic while making the refcount atomic, but it
> needs to be explained in the changelog. As is, the changelog makes it sound
> like the patch is a pure refactoring of the type.
Thanks for pointing that out. I'll add a note in the description in
v2. Those felt like natural changes since the introduction of the
atomic requires additional failure handling. I don't think there's any
way to add it as a separate commit without just introducing dead code,
but that would certainly be preferable.
