| Message ID | 20231027172640.2335197-1-dmatlack@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | KVM: Performance and correctness fixes for CLEAR_DIRTY_LOG | expand |
On Fri, Oct 27, 2023, David Matlack wrote: > This series reduces the impact of CLEAR_DIRTY_LOG on guest performance > (Patch 3) and fixes 2 minor bugs found along the way (Patches 1 and 2). > David Matlack (3): > KVM: x86/mmu: Fix off-by-1 when splitting huge pages during CLEAR > KVM: x86/mmu: Check for leaf SPTE when clearing dirty bit in the TDP > MMU > KVM: Aggressively drop and reacquire mmu_lock during CLEAR_DIRTY_LOG Is there an actual dependency between 1-2 and 3? AFAICT, no? I ask because I can take the first two as soon as -rc1 is out, but the generic change definitely needs testing and acks from other architectures.
On Tue, Nov 7, 2023 at 4:05 PM Sean Christopherson <seanjc@google.com> wrote: > > On Fri, Oct 27, 2023, David Matlack wrote: > > This series reduces the impact of CLEAR_DIRTY_LOG on guest performance > > (Patch 3) and fixes 2 minor bugs found along the way (Patches 1 and 2). > > David Matlack (3): > > KVM: x86/mmu: Fix off-by-1 when splitting huge pages during CLEAR > > KVM: x86/mmu: Check for leaf SPTE when clearing dirty bit in the TDP > > MMU > > KVM: Aggressively drop and reacquire mmu_lock during CLEAR_DIRTY_LOG > > Is there an actual dependency between 1-2 and 3? AFAICT, no? I ask because I > can take the first two as soon as -rc1 is out, but the generic change definitely > needs testing and acks from other architectures. No, there isn't any dependency between any of the commits. Feel free to grab 1-2 independent of 3.
On Fri, 27 Oct 2023 10:26:37 -0700, David Matlack wrote: > This series reduces the impact of CLEAR_DIRTY_LOG on guest performance > (Patch 3) and fixes 2 minor bugs found along the way (Patches 1 and 2). > > We've observed that guest performance can drop while userspace is > issuing CLEAR_DIRTY_LOG ioctls and tracked down the problem to > contention on the mmu_lock in vCPU threads. CLEAR_DIRTY_LOG holds the > write-lock, so this isn't that surprising. We previously explored > converting CLEAR_DIRTY_LOG to hold the read-lock [1], but that has some > negative consequences: > > [...] Applied 1 and 2 to kvm-x86 mmu. To get traction on #3, I recommend resending it as a standalone patch with all KVM arch maintainers Cc'd. [1/3] KVM: x86/mmu: Fix off-by-1 when splitting huge pages during CLEAR https://github.com/kvm-x86/linux/commit/7cd1bf039eeb [2/3] KVM: x86/mmu: Check for leaf SPTE when clearing dirty bit in the TDP MMU https://github.com/kvm-x86/linux/commit/76d1492924bc -- https://github.com/kvm-x86/linux/tree/next
On Thu, Nov 30, 2023, Sean Christopherson wrote: > On Fri, 27 Oct 2023 10:26:37 -0700, David Matlack wrote: > > This series reduces the impact of CLEAR_DIRTY_LOG on guest performance > > (Patch 3) and fixes 2 minor bugs found along the way (Patches 1 and 2). > > > > We've observed that guest performance can drop while userspace is > > issuing CLEAR_DIRTY_LOG ioctls and tracked down the problem to > > contention on the mmu_lock in vCPU threads. CLEAR_DIRTY_LOG holds the > > write-lock, so this isn't that surprising. We previously explored > > converting CLEAR_DIRTY_LOG to hold the read-lock [1], but that has some > > negative consequences: > > > > [...] > > Applied 1 and 2 to kvm-x86 mmu. To get traction on #3, I recommend resending it > as a standalone patch with all KVM arch maintainers Cc'd. > > [1/3] KVM: x86/mmu: Fix off-by-1 when splitting huge pages during CLEAR > https://github.com/kvm-x86/linux/commit/7cd1bf039eeb > [2/3] KVM: x86/mmu: Check for leaf SPTE when clearing dirty bit in the TDP MMU > https://github.com/kvm-x86/linux/commit/76d1492924bc Force pushed because the Fixes: in patch 1 referenced a Google-internal commit. New hashes: [1/2] KVM: x86/mmu: Fix off-by-1 when splitting huge pages during CLEAR https://github.com/kvm-x86/linux/commit/1aa4bb916808 [2/2] KVM: x86/mmu: Check for leaf SPTE when clearing dirty bit in the TDP MMU https://github.com/kvm-x86/linux/commit/45a61ebb2211
This series reduces the impact of CLEAR_DIRTY_LOG on guest performance (Patch 3) and fixes 2 minor bugs found along the way (Patches 1 and 2). We've observed that guest performance can drop while userspace is issuing CLEAR_DIRTY_LOG ioctls and tracked down the problem to contention on the mmu_lock in vCPU threads. CLEAR_DIRTY_LOG holds the write-lock, so this isn't that surprising. We previously explored converting CLEAR_DIRTY_LOG to hold the read-lock [1], but that has some negative consequences: - Pretty significant code churn is required on x86 and ARM to support doing CLEAR under the read-lock. Things get especially hairy on x86 when considering how to support the Shadow MMU. - Holding the read-lock means KVM will have to use atomic compare-and-exchange operations during eager splitting and clearing dirty bits, which can be quite slow on certain ARM platforms. This series proposed an alternative (well, complimentary, really) approach of simply dropping mmu_lock more frequently. I tested this series out with one of our internal Live Migration tests where the guest is running MySQL in a 160 vCPU VM (Intel Broadwell host) and it eliminates the performance drops we were seeing when userspace issues CLEAR ioctls. Furthermore I don't see any noticeable improvement when I test with this series plus a prototype patch convert CLEAR to the read lock on x86. i.e. It seems we can eliminate most of the lock contention by just dropping the lock more frequently. Cc: Vipin Sharma <vipinsh@google.com> [1] https://lore.kernel.org/kvm/20230602160914.4011728-1-vipinsh@google.com/ David Matlack (3): KVM: x86/mmu: Fix off-by-1 when splitting huge pages during CLEAR KVM: x86/mmu: Check for leaf SPTE when clearing dirty bit in the TDP MMU KVM: Aggressively drop and reacquire mmu_lock during CLEAR_DIRTY_LOG arch/x86/kvm/mmu/mmu.c | 2 +- arch/x86/kvm/mmu/tdp_mmu.c | 7 ++++--- virt/kvm/kvm_main.c | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) base-commit: 2b3f2325e71f09098723727d665e2e8003d455dc