On Thu, Jan 6, 2022 at 3:14 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Mon, Dec 13, 2021, David Matlack wrote:
> > Add a tracepoint that records whenever KVM eagerly splits a huge page.
> >
> > Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx>
> > ---
> > arch/x86/kvm/mmu/mmutrace.h | 20 ++++++++++++++++++++
> > arch/x86/kvm/mmu/tdp_mmu.c | 2 ++
> > 2 files changed, 22 insertions(+)
> >
> > diff --git a/arch/x86/kvm/mmu/mmutrace.h b/arch/x86/kvm/mmu/mmutrace.h
> > index de5e8e4e1aa7..4feabf773387 100644
> > --- a/arch/x86/kvm/mmu/mmutrace.h
> > +++ b/arch/x86/kvm/mmu/mmutrace.h
> > @@ -416,6 +416,26 @@ TRACE_EVENT(
> > )
> > );
> >
> > +TRACE_EVENT(
> > + kvm_mmu_split_huge_page,
> > + TP_PROTO(u64 gfn, u64 spte, int level),
> > + TP_ARGS(gfn, spte, level),
> > +
> > + TP_STRUCT__entry(
> > + __field(u64, gfn)
> > + __field(u64, spte)
> > + __field(int, level)
> > + ),
> > +
> > + TP_fast_assign(
> > + __entry->gfn = gfn;
> > + __entry->spte = spte;
> > + __entry->level = level;
> > + ),
> > +
> > + TP_printk("gfn %llx spte %llx level %d", __entry->gfn, __entry->spte, __entry->level)
> > +);
> > +
> > #endif /* _TRACE_KVMMMU_H */
> >
> > #undef TRACE_INCLUDE_PATH
> > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> > index be5eb74ac053..e6910b9b5c12 100644
> > --- a/arch/x86/kvm/mmu/tdp_mmu.c
> > +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> > @@ -1325,6 +1325,8 @@ tdp_mmu_split_huge_page_atomic(struct kvm *kvm, struct tdp_iter *iter, struct kv
> > u64 child_spte;
> > int i;
> >
> > + trace_kvm_mmu_split_huge_page(iter->gfn, huge_spte, level);
>
> This should either be called iff splitting is successful, or it should record
> whether or not the split was successful.
Blegh. My intention was to do the former but it's obviously wrong if
the cmpxchg fails.
> The latter is probably useful info,
> and easy to do, e.g. assuming this is changed to return an int like the lower
> helpers:
>
>
> ret = tdp_mmu_install_sp_atomic(kvm, iter, sp, false);
>
> /*
> * tdp_mmu_install_sp_atomic will handle subtracting the split huge
> * page from stats, but we have to manually update the new present child
> * pages on success.
> */
> if (!ret)
> kvm_update_page_stats(kvm, level - 1, PT64_ENT_PER_PAGE);
>
> trace_kvm_mmu_split_huge_page(iter->gfn, huge_spte, level, ret);
>
> return ret;
>
> and then the tracpoint can do 'ret ? "failed" : "succeeded"' or something.
If we do this we should capture all the reasons why splitting might
fail. cmpxchg races are one, and the other is failing to allocate the
sp memory. I'll take a look at doing this in the next version. It
doesn't look too difficult.
>
> > +
> > init_child_tdp_mmu_page(sp, iter);
> >
> > for (i = 0; i < PT64_ENT_PER_PAGE; i++) {
> > --
> > 2.34.1.173.g76aa8bc2d0-goog
> >
