LKML Archive on lore.kernel.org help / color / mirror / Atom feed
From: Mingwei Zhang <firstname.lastname@example.org> To: Peter Xu <email@example.com> Cc: Jim Mattson <firstname.lastname@example.org>, Paolo Bonzini <email@example.com>, Sean Christopherson <firstname.lastname@example.org>, Vitaly Kuznetsov <email@example.com>, Wanpeng Li <firstname.lastname@example.org>, Joerg Roedel <email@example.com>, kvm <firstname.lastname@example.org>, LKML <email@example.com>, Ben Gardon <firstname.lastname@example.org>, David Matlack <email@example.com>, Jing Zhang <firstname.lastname@example.org> Subject: Re: [PATCH v4 3/3] KVM: x86/mmu: Add detailed page size stats Date: Tue, 10 Aug 2021 18:06:51 -0700 [thread overview] Message-ID: <CAL715WJWPzBqmjeTJ6mZa=dUaF5+MdqaCrk5CEzvcz1X99cm0g@mail.gmail.com> (raw) In-Reply-To: <YRMKPd2ZarXCX6vm@t490s> Hi Peter, Thanks for the comments. Please check my feedback inline. > > From a functionality point of view, the above patch seems duplicate > > with mine. > > The rmap statistics are majorly for rmap, not huge pages. > Got you. I guess the focus of the stat is different. > > I have a question: why change to using atomic ops? As most kvm statistics > seems to be not with atomics before. > > AFAIK atomics are expensive, and they get even more expensive when the host is > bigger (it should easily go into ten-nanosecond level). So I have no idea > whether it's worth it for persuing that accuracy. > > Thanks, Yes, regarding the usage of 'atomic_t', we previously had discussions internally with Sean. So I think the main reason is because: in KVM currently, mmu may have several modes. Among them, one is the mmu running with TDP enabled (say, legacy mode in this scope) and another one is the TDP mmu mode (mmu/tdp_mmu.c). In the legacy mode, mmu will update spte under a write lock, while in comparison, in the TDP MMU mode, mmu will use a read lock. I copied a simple code snippet to illustrate the situation: ».......if (is_tdp_mmu_fault) ».......».......read_lock(&vcpu->kvm->mmu_lock); ».......else ».......».......write_lock(&vcpu->kvm->mmu_lock); So here comes the problem: how do we make the page stat update correctly across all modes? For the latter case, we definitely have to update the stat in an atomic way unless we want extra locking (we don't want that). So we could either 1) use a branch to update the stat differently for each mode or 2) use an atomic update across all cases. After review, Sean mentioned (in pre-v1 internal review) that we should just use atomic. I agree since adding a branch at such a hot path may slow down even more globally, especially if there is branch misprediction? But I appreciate your feedback as well. Regarding the pursuit for accuracy, I think there might be several reasons. One of the most critical reasons that I know is that we need to ensure dirty logging works correctly, i.e., when dirty logging is enabled, all huge pages (both 2MB and 1GB) _are_ gone. Hope that clarifies a little bit? Thanks. -Mingwei -Mingwei > > -- > Peter Xu >
next prev parent reply other threads:[~2021-08-11 1:07 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-08-03 4:46 [PATCH v4 0/3] Add detailed page size stats in KVM stats Mingwei Zhang 2021-08-03 4:46 ` [PATCH v4 1/3] KVM: x86/mmu: Remove redundant spte present check in mmu_set_spte Mingwei Zhang 2021-08-03 4:46 ` [PATCH v4 2/3] KVM: x86/mmu: Avoid collision with !PRESENT SPTEs in TDP MMU lpage stats Mingwei Zhang 2021-08-03 4:46 ` [PATCH v4 3/3] KVM: x86/mmu: Add detailed page size stats Mingwei Zhang 2021-08-09 22:26 ` Jim Mattson 2021-08-09 23:39 ` Mingwei Zhang 2021-08-10 0:01 ` Mingwei Zhang 2021-08-10 23:22 ` Peter Xu 2021-08-11 1:06 ` Mingwei Zhang [this message] 2021-08-11 13:12 ` Peter Xu 2021-08-12 17:44 ` Mingwei Zhang 2021-08-11 11:36 ` Paolo Bonzini
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAL715WJWPzBqmjeTJ6mZa=dUaF5+MdqaCrk5CEzvcz1X99cm0g@mail.gmail.com' \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).