Skip to contentSkip to conversation

KVM swapping with mmu notifiers #v5

PATCH Andrea Arcangeli 2 participants 4 messages Jan 31, 2008 – Feb 1, 2008 3 messages not in the archive

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

Hi,

Usual patch but adapted to mmu notifier #v5, works fine here as expected.

I doubt Christoph's V4 was close to final yet, GRU wasn't covered at all yet, not even mremap was covered at all (nor XPMEM nor GRU) in V4.

The first workable APIs for XPMEM (to close the SMP race I explained since export-noifiers #v1) is just an idea from last night... and for the first time xpmem I think may work safe.

I think my #v5 is small enough, should already fit KVM and GRU, it should provide an API that allows optimization and extension over time, and it can be extended to support XPMEM once that will work in practice. I really think it's better idea to be able at least to test some code before pushing in mainline some broad VM visible API. This is what I did with KVM infact. Once KVM was solid swapping 3G over 1G of ram I pushed the mmu notifiers to lkml.

Being dependent on XPMEM support being merged, to merge KVM/GRU doesn't sound a good idea. My patch provides no overhead with MMU_NOTIFIER=n too. Hope Christoph agrees with my proposal to use #v5 as the mmu core and to merge it in mainline with higher priority, to mostly close the discussions on KVM and GRU (optimizations remains possible) and to keep working incrementally on XPMEM and to push it in mainline whenever you verified that it doesn't crash at runtime and that you don't need yet another change of API.

arch/x86/kvm/Kconfig +1 -0
index 4086080..c527d7d 100644
@@ -18,6 +18,7 @@ config KVM
 	tristate "Kernel-based Virtual Machine (KVM) support"
 	depends on ARCH_SUPPORTS_KVM && EXPERIMENTAL
 	select PREEMPT_NOTIFIERS
+	select MMU_NOTIFIER
 	select ANON_INODES
 	---help---
 	  Support hosting fully virtualized guest machines using hardware
arch/x86/kvm/mmu.c +104 -0

110 lines not on this page · raw message

arch/x86/kvm/x86.c +42 -0

54 lines not on this page · raw message

include/asm-x86/kvm_host.h +5 -0
index d6db0de..72a7ff4 100644
@@ -13,6 +13,7 @@
 
 #include <linux/types.h>
 #include <linux/mm.h>
+#include <linux/mmu_notifier.h>
 
 #include <linux/kvm.h>
 #include <linux/kvm_para.h>
@@ -287,6 +288,8 @@ struct kvm_arch{
 	int round_robin_prev_vcpu;
 	unsigned int tss_addr;
 	struct page *apic_access_page;
+
+	struct mmu_notifier mmu_notifier;
 };
 
 struct kvm_vm_stat {
@@ -404,6 +407,8 @@ int kvm_mmu_create(struct kvm_vcpu *vcpu);
 int kvm_mmu_setup(struct kvm_vcpu *vcpu);
 void kvm_mmu_set_nonpresent_ptes(u64 trap_pte, u64 notrap_pte);
 
+void kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
+int kvm_age_hva(struct kvm *kvm, unsigned long hva);
 int kvm_mmu_reset_context(struct kvm_vcpu *vcpu);
 void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot);
 void kvm_mmu_zap_all(struct kvm *kvm);

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw
Quoted 2 lines · Andrea Arcangeli

I doubt Christoph's V4 was close to final yet, GRU wasn't covered at all yet, not even mremap was covered at all (nor XPMEM nor GRU) in V4.

The GRU not covered? Why would you think that way? mremap is covered because of the callbacks in unmap_region().

Quoted 8 lines

Being dependent on XPMEM support being merged, to merge KVM/GRU doesn't sound a good idea. My patch provides no overhead with MMU_NOTIFIER=n too. Hope Christoph agrees with my proposal to use #v5 as the mmu core and to merge it in mainline with higher priority, to mostly close the discussions on KVM and GRU (optimizations remains possible) and to keep working incrementally on XPMEM and to push it in mainline whenever you verified that it doesn't crash at runtime and that you don't need yet another change of API.

Please read the comments on your #5. #5 makes wrong assumptions about the nature of pte locks. As a result locking is broken.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

A message that is not in the archive (sent off-list or not yet received): Pine.LNX.4.64.0801311219100.25477-RYO/[email protected]

#4 lore raw
Quoted 7 lines · Christoph Lameter
Quoted 2 lines · Andrea Arcangeli

I doubt Christoph's V4 was close to final yet, GRU wasn't covered at all yet, not even mremap was covered at all (nor XPMEM nor GRU) in V4.

The GRU not covered? Why would you think that way? mremap is covered because of the callbacks in unmap_region().

I wouldn't be so sure. ptep_clear_flush is called for a reason and you have zero range_start _before_ the ptep_clear_flush. If you're right it means ptep_clear_flush there is called for no good reason and it should be replaced with ptep_get_and_clear and eliminate an unnecessary tlb flush from the mremap fast path, and a tlb flush that will cost an huge lot with threads, an IPI for every single PTE in SMP! So you may be right, but then it means we found a really stupid spot to optimize in mremap. (I've to say I've already found a silly thing in the ptep_ that sets the accessed bitflag, pte entries w/o accessed bit set can't be tlb-cached, this is an hardware thing, so the tlb flush there on x86 is a total waste of ipis)

Quoted 11 lines
Quoted 8 lines

Being dependent on XPMEM support being merged, to merge KVM/GRU doesn't sound a good idea. My patch provides no overhead with MMU_NOTIFIER=n too. Hope Christoph agrees with my proposal to use #v5 as the mmu core and to merge it in mainline with higher priority, to mostly close the discussions on KVM and GRU (optimizations remains possible) and to keep working incrementally on XPMEM and to push it in mainline whenever you verified that it doesn't crash at runtime and that you don't need yet another change of API.

Please read the comments on your #5. #5 makes wrong assumptions about the nature of pte locks. As a result locking is broken.

You misunderstood the locking, #v5 is obviously safe. If #v5 wasn't safe, any SMP with >4 cpus would crash already regardless of my changes...

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

A message that is not in the archive (sent off-list or not yet received): [email protected]

#6 lore raw
Quoted 7 lines · Andrea Arcangeli
Quoted 2 lines

The GRU not covered? Why would you think that way? mremap is covered because of the callbacks in unmap_region().

I wouldn't be so sure. ptep_clear_flush is called for a reason and you have zero range_start _before_ the ptep_clear_flush. If you're right it means ptep_clear_flush there is called for no good reason and it should be replaced with ptep_get_and_clear and eliminate an

Ok. I see the point. Will post in response to the other thread.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/