| Message ID | 20210630214802.1902448-1-dmatlack@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | KVM: x86/mmu: Fast page fault support for the TDP MMU | expand |
On Wed, Jun 30, 2021 at 09:47:56PM +0000, David Matlack wrote:
> This patch series adds support for the TDP MMU in the fast_page_fault
Nowhere in this message do you explain what the TDP MMU is.
On Thu, Jul 01, 2021 at 08:47:06AM +0200, Paolo Bonzini wrote: > Il gio 1 lug 2021, 03:17 Matthew Wilcox <willy@infradead.org> ha scritto: > > > On Wed, Jun 30, 2021 at 09:47:56PM +0000, David Matlack wrote: > > > This patch series adds support for the TDP MMU in the fast_page_fault > > > > Nowhere in this message do you explain what the TDP MMU is. > > > > I don't know if you rolled a 1 too much in your last D&D session or what, > but this seems like a slightly petty remark given the wealth of information > that was in the commit message. Yes, there was a wealth of information, but I couldn't understand any of it because I have no idea what a TDP MMU is. Actually, the length of the intro message annoyed me, because I had to scan the whole thing to try to figure out whether he explained what a TDP MMU is anywhere in it. > The patches only touch kvm code, therefore David probably expected only kvm > reviewers to be interested---in which case, anyone who hasn't lived under a > rock for a year or two would know. He cc'd me. That's usually a request for a review. So I had to try to understand why I would care. At this point, I don't think I care. Somebody explain to me why I should care? That's the point of a cover letter. > Anyway the acronym is fairly > Google-friendly: > https://lore.kernel.org/kvm/20201014182700.2888246-1-bgardon@google.com/ > http://lkml.iu.edu/hypermail/linux/kernel/2010.2/07021.html Now I'm really confused, and I don't understand why I was cc'd at all. get_maintainer.pl going wild?
On Thu, Jul 1, 2021 at 5:08 AM Matthew Wilcox <willy@infradead.org> wrote: > > Now I'm really confused, and I don't understand why I was cc'd at all. > get_maintainer.pl going wild? Apologies for the confusion. I could have made it more clear why I cc'd you in the cover letter. I cc'd you, Yu, David, and Andrew (Morton) since this series introduces a new selftest that uses on page_idle and you recently discussed removing page_idle. See the second paragraph of the cover letter with the link to your page_idle patch, and patch 6 which introduces the selftest. I will make it more explicit in the future when cc'ing non-KVM developers on my KVM patches.
On 30.06.21 23:47, David Matlack wrote: > This patch series adds support for the TDP MMU in the fast_page_fault > path, which enables certain write-protection and access tracking faults > to be handled without taking the KVM MMU lock. This series brings the > performance of these faults up to par with the legacy MMU. > > Since there is not currently any KVM test coverage for access tracking > faults, this series introduces a new KVM selftest, > access_tracking_perf_test. Note that this test relies on page_idle to > enable access tracking from userspace (since it is the only available > usersapce API to do so) and page_idle is being considered for removal > from Linux > (https://lore.kernel.org/linux-mm/20210612000714.775825-1-willy@infradead.org/). Well, at least a new selftest that implicitly tests a part of page_idle -- nice :) Haven't looked into the details, but if you can live with page tables starting unpopulated and only monitoring what gets populated on r/w access, you might be able to achieve something similar using /proc/self/pagemap and softdirty handling. Unpopulated page (e.g., via MADV_DISCARD) -> trigger read or write access -> sense if page populated in pagemap Populated page-> clear all softdirty bits -> trigger write access -> sense if page is softdirty in pagemap See https://lkml.kernel.org/r/20210419135443.12822-6-david@redhat.com for an example. But I'm actually fairly happy to see page_idel getting used. Maybe you could extend that test using pagemap, if it's applicable to your test setup.
On Thu, Jul 01, 2021 at 07:00:51PM +0200, David Hildenbrand wrote: > On 30.06.21 23:47, David Matlack wrote: > > This patch series adds support for the TDP MMU in the fast_page_fault > > path, which enables certain write-protection and access tracking faults > > to be handled without taking the KVM MMU lock. This series brings the > > performance of these faults up to par with the legacy MMU. > > > > Since there is not currently any KVM test coverage for access tracking > > faults, this series introduces a new KVM selftest, > > access_tracking_perf_test. Note that this test relies on page_idle to > > enable access tracking from userspace (since it is the only available > > usersapce API to do so) and page_idle is being considered for removal > > from Linux > > (https://lore.kernel.org/linux-mm/20210612000714.775825-1-willy@infradead.org/). > > Well, at least a new selftest that implicitly tests a part of page_idle -- > nice :) > > Haven't looked into the details, but if you can live with page tables > starting unpopulated and only monitoring what gets populated on r/w access, > you might be able to achieve something similar using /proc/self/pagemap and > softdirty handling. > > Unpopulated page (e.g., via MADV_DISCARD) -> trigger read or write access -> > sense if page populated in pagemap > Populated page-> clear all softdirty bits -> trigger write access -> sense > if page is softdirty in pagemap Thanks for the suggestion. I modified by test to write 4 to /proc/self/clear_refs rather than marking pages in page_idle. However, by doing so I was no longer able to exercise KVM's fast_page_fault handler [1]. It looks like the reason why is that clear_refs issues the invalidate_range mmu notifiers, which will cause KVM to fully refault the page from the host MM upon subsequent guest memory accesses. In contrast, page_idle uses clear_young which KVM can handle with fast_page_fault. Let me know if I misunderstood your suggestion though. [1] https://www.kernel.org/doc/html/latest/virt/kvm/locking.html#exception > > See https://lkml.kernel.org/r/20210419135443.12822-6-david@redhat.com for an > example. > > But I'm actually fairly happy to see page_idel getting used. Maybe you could > extend that test using pagemap, if it's applicable to your test setup. > > -- > Thanks, > > David / dhildenb >
On 02.07.21 00:11, David Matlack wrote: > On Thu, Jul 01, 2021 at 07:00:51PM +0200, David Hildenbrand wrote: >> On 30.06.21 23:47, David Matlack wrote: >>> This patch series adds support for the TDP MMU in the fast_page_fault >>> path, which enables certain write-protection and access tracking faults >>> to be handled without taking the KVM MMU lock. This series brings the >>> performance of these faults up to par with the legacy MMU. >>> >>> Since there is not currently any KVM test coverage for access tracking >>> faults, this series introduces a new KVM selftest, >>> access_tracking_perf_test. Note that this test relies on page_idle to >>> enable access tracking from userspace (since it is the only available >>> usersapce API to do so) and page_idle is being considered for removal >>> from Linux >>> (https://lore.kernel.org/linux-mm/20210612000714.775825-1-willy@infradead.org/). >> >> Well, at least a new selftest that implicitly tests a part of page_idle -- >> nice :) >> >> Haven't looked into the details, but if you can live with page tables >> starting unpopulated and only monitoring what gets populated on r/w access, >> you might be able to achieve something similar using /proc/self/pagemap and >> softdirty handling. >> >> Unpopulated page (e.g., via MADV_DISCARD) -> trigger read or write access -> >> sense if page populated in pagemap >> Populated page-> clear all softdirty bits -> trigger write access -> sense >> if page is softdirty in pagemap > > Thanks for the suggestion. I modified by test to write 4 to > /proc/self/clear_refs rather than marking pages in page_idle. However, > by doing so I was no longer able to exercise KVM's fast_page_fault > handler [1]. > > It looks like the reason why is that clear_refs issues the > invalidate_range mmu notifiers, which will cause KVM to fully refault > the page from the host MM upon subsequent guest memory accesses. In > contrast, page_idle uses clear_young which KVM can handle with > fast_page_fault. Right, the only thing you could provoke may be (again, did not look into the details): 1. 4 > /proc/self/clear_refs to clear all softdirty bits 2. Trigger a read fault. This will populate the shared zeropage on private anonymous memory and populate an actual page on e.g., shmem. 3. Trigger a write fault. This should result in a COW fault on private anonymous memory and (IIRC) a WP-fault on shmem. The COW on private anonymous memory would invalidate the secondary MMU again via MMU notifiers I guess, so it's not what we want. It *might* work on shmem, I might be wrong, though. But again, I haven't looked into the details/checked the code and we might actually not be able to test with softdirty at all. IMHO, page idle tracking might be good enough for a test. Consider it rather a brain dump than a suggestion ;)
This patch series adds support for the TDP MMU in the fast_page_fault path, which enables certain write-protection and access tracking faults to be handled without taking the KVM MMU lock. This series brings the performance of these faults up to par with the legacy MMU. Since there is not currently any KVM test coverage for access tracking faults, this series introduces a new KVM selftest, access_tracking_perf_test. Note that this test relies on page_idle to enable access tracking from userspace (since it is the only available usersapce API to do so) and page_idle is being considered for removal from Linux (https://lore.kernel.org/linux-mm/20210612000714.775825-1-willy@infradead.org/). Design ------ This series enables the existing fast_page_fault handler to operate independent of whether the TDP MMU is enabled or not by abstracting out the details behind a new lockless page walk API. An alternative design considered was to add a separate fast_page_fault handler to the TDP MMU. The code that inspects the spte and genereates the new spte can be shared with the legacy MMU. However with this design the retry loop has to be duplicated, there are many calls back and forth between mmu.c and tdp_mmu.c, and passing around the RET_PF_* values gets complicated. Testing ------- This series was tested on an Intel Cascade Lake machine. The kvm_intel parameters eptad and pml were disabled to force access and dirty tracking to go through fast_page_fault. All tests were run with the TDP MMU enabled and then again disabled. Tests ran: - All KVM selftests with default arguments - All x86_64 kvm-unit-tests. - ./access_tracking_perf_test -v 4 - ./access_tracking_perf_test -v 4 -o - ./access_tracking_perf_test -v 4 -s anonymous_thp - ./access_tracking_perf_test -v 4 -s anonymous_thp -o - ./access_tracking_perf_test -v 64 - ./dirty_log_perf_test -v 4 - ./dirty_log_perf_test -v 4 -o - ./dirty_log_perf_test -v 4 -s anonymous_thp - ./dirty_log_perf_test -v 4 -s anonymous_thp -o - ./dirty_log_perf_test -v 64 For certain tests I also collected the fast_page_fault tracepoint to manually make sure it was getting triggered properly: perf record -e kvmmmu:fast_page_fault --filter "old_spte != 0" -- <test> Performance Results ------------------- To measure performance I ran dirty_log_perf_test and access_tracking_perf_test with 64 vCPUs. For dirty_log_perf_test performance is measured by "Iteration 2 dirty memory time", the time it takes for all vCPUs to write to their memory after it has been write-protected. For access_tracking_perf_test performance is measured by "Writing to idle memory", the time it takes for all vCPUs to write to their memory after it has been access-protected. Metric | tdp_mmu=Y before | tdp_mmu=Y after --------------------------------- | ------------------ | -------------------- Iteration 2 dirty memory time | 3.545234984s | 0.315209625s Writing to idle memory | 3.249645416s | 0.301676189s The performance improvement comes from less time spent acquiring the mmu lock in read mode and less time looking up the memslot for the faulting gpa. The TDP MMU is now on par with the legacy MMU: Metric | tdp_mmu=N | tdp_mmu=Y --------------------------------- | ------------------ | -------------------- Iteration 2 dirty memory time | 0.303452990s | 0.315209625s Writing to idle memory | 0.291742127s | 0.301676189s v2: * Split is_tdp_mmu_root cleanup into a separate series. [Sean] https://lore.kernel.org/kvm/20210617231948.2591431-1-dmatlack@google.com/ * Split walk_shadow_page_lockless into 2 APIs. [Sean] * Perform rcu_dereference on TDP MMU sptep. * Add comment to tdp_mmu_set_spte_atomic explaining new interaction * with fast_pf_fix_direct_spte. [Ben] * Document pagemap shifts in access_tracking_perf_test. [Ben] * Skip test if lacking pagemap permissions (present pfn is 0). [Ben] * Add Ben's Reviewed-by tags. v1: https://lore.kernel.org/kvm/20210611235701.3941724-1-dmatlack@google.com/ David Matlack (6): KVM: x86/mmu: Rename cr2_or_gpa to gpa in fast_page_fault KVM: x86/mmu: Fix use of enums in trace_fast_page_fault KVM: x86/mmu: Make walk_shadow_page_lockless_{begin,end} interoperate with the TDP MMU KVM: x86/mmu: fast_page_fault support for the TDP MMU KVM: selftests: Fix missing break in dirty_log_perf_test arg parsing KVM: selftests: Introduce access_tracking_perf_test arch/x86/kvm/mmu/mmu.c | 83 +++- arch/x86/kvm/mmu/mmutrace.h | 3 + arch/x86/kvm/mmu/tdp_mmu.c | 56 ++- arch/x86/kvm/mmu/tdp_mmu.h | 8 +- tools/testing/selftests/kvm/.gitignore | 1 + tools/testing/selftests/kvm/Makefile | 1 + .../selftests/kvm/access_tracking_perf_test.c | 429 ++++++++++++++++++ .../selftests/kvm/dirty_log_perf_test.c | 1 + 8 files changed, 550 insertions(+), 32 deletions(-) create mode 100644 tools/testing/selftests/kvm/access_tracking_perf_test.c