Skip to contentSkip to conversation

add 2nd stage page fault handling during live migration

PATCH Superseded Mario Smarduch 2 participants 3 messages Apr 17, 2014 – Apr 18, 2014

Subsystems (2)

v1 of 11 Superseded Latest is v11 →

lore
Share & tools
lore
#0 lore raw

PATCH 4/5 add 2nd stage page fault handling during live migration

Additional logic to handle second stage page faults during migration. Primarily page faults are prevented from creating huge pages.

---
 arch/arm/kvm/mmu.c |   33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)
arch/arm/kvm/mmu.c +31 -2
index 47bec1c..ebec33c 100644
@@ -839,6 +839,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	struct kvm_mmu_memory_cache *memcache = &vcpu->arch.mmu_page_cache;
 	struct vm_area_struct *vma;
 	pfn_t pfn;
+	bool migration_active;
 
 	write_fault = kvm_is_write_fault(kvm_vcpu_get_hsr(vcpu));
 	if (fault_status == FSC_PERM && !write_fault) {
@@ -890,12 +891,22 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 		return -EFAULT;
 
 	spin_lock(&kvm->mmu_lock);
+	/* place inside lock to prevent race condition when whole VM is being
+	 * write proteced initially, prevent pmd update if it's split up.
+	 */
+	migration_active = vcpu->kvm->arch.migration_in_progress;
+
 	if (mmu_notifier_retry(kvm, mmu_seq))
 		goto out_unlock;
-	if (!hugetlb && !force_pte)
+
+	/* During migration don't rebuild huge pages */
+	if (!hugetlb && !force_pte && !migration_active)
 		hugetlb = transparent_hugepage_adjust(&pfn, &fault_ipa);
 
-	if (hugetlb) {
+	/* Steer away from installing PMDs if migrating, migration failed,
+	 * or this an initial page fault. Migrating huge pages is too slow.
+	 */
+	if (!migration_active && hugetlb) {
 		pmd_t new_pmd = pfn_pmd(pfn, PAGE_S2);
 		new_pmd = pmd_mkhuge(new_pmd);
 		if (writable) {
@@ -907,6 +918,22 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	} else {
 		pte_t new_pte = pfn_pte(pfn, PAGE_S2);
 		if (writable) {
+			/* First convert huge page pfn to normal 4k page pfn,
+			 * while  migration is in progress.
+			 * Second in migration mode and rare case where
+			 * splitting of huge pages fails check if pmd is
+			 * mapping a huge page if it is then clear it so
+			 * stage2_set_pte() can map in a small page.
+			 */
+			if (migration_active && hugetlb) {
+				pmd_t *pmd;
+				pfn += (fault_ipa >> PAGE_SHIFT) &
+							(PTRS_PER_PMD-1);
+				new_pte = pfn_pte(pfn, PAGE_S2);
+				pmd = stage2_get_pmd(kvm, NULL, fault_ipa);
+				if (pmd && kvm_pmd_huge(*pmd))
+					clear_pmd_entry(kvm, pmd, fault_ipa);
+			}
 			kvm_set_s2pte_writable(&new_pte);
 			kvm_set_pfn_dirty(pfn);
 		}
@@ -914,6 +941,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 		ret = stage2_set_pte(kvm, memcache, fault_ipa, &new_pte, false);
 	}
 
+	if (writable)
+		mark_page_dirty(kvm, gfn);
 
 out_unlock:
 	spin_unlock(&kvm->mmu_lock);
Signature · 1 line
1.7.9.5
#1 lore raw
Quoted 51 lines · Mario Smarduch

Additional logic to handle second stage page faults during migration. Primarily page faults are prevented from creating huge pages.

---
 arch/arm/kvm/mmu.c |   33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)
arch/arm/kvm/mmu.c +14 -2
index 47bec1c..ebec33c 100644
@@ -839,6 +839,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	struct kvm_mmu_memory_cache *memcache = &vcpu->arch.mmu_page_cache;
 	struct vm_area_struct *vma;
 	pfn_t pfn;
+	bool migration_active;
 
 	write_fault = kvm_is_write_fault(kvm_vcpu_get_hsr(vcpu));
 	if (fault_status == FSC_PERM && !write_fault) {
@@ -890,12 +891,22 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 		return -EFAULT;
 
 	spin_lock(&kvm->mmu_lock);
+	/* place inside lock to prevent race condition when whole VM is being
+	 * write proteced initially, prevent pmd update if it's split up.
+	 */
+	migration_active = vcpu->kvm->arch.migration_in_progress;
+
 	if (mmu_notifier_retry(kvm, mmu_seq))
 		goto out_unlock;
-	if (!hugetlb && !force_pte)
+
+	/* During migration don't rebuild huge pages */
+	if (!hugetlb && !force_pte && !migration_active)
 		hugetlb = transparent_hugepage_adjust(&pfn, &fault_ipa);
 
-	if (hugetlb) {
+	/* Steer away from installing PMDs if migrating, migration failed,
+	 * or this an initial page fault. Migrating huge pages is too slow.
+	 */
+	if (!migration_active && hugetlb) {
 		pmd_t new_pmd = pfn_pmd(pfn, PAGE_S2);
 		new_pmd = pmd_mkhuge(new_pmd);
 		if (writable) {
@@ -907,6 +918,22 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	} else {
 		pte_t new_pte = pfn_pte(pfn, PAGE_S2);
 		if (writable) {
+			/* First convert huge page pfn to normal 4k page pfn,

arch/arm/kvm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 931

Quoted 10 lines
arch/arm/kvm/mmu.c +10 -0
+			 * while  migration is in progress.
+			 * Second in migration mode and rare case where
+			 * splitting of huge pages fails check if pmd is
+			 * mapping a huge page if it is then clear it so
+			 * stage2_set_pte() can map in a small page.
+			 */
+			if (migration_active && hugetlb) {
+				pmd_t *pmd;
+				pfn += (fault_ipa >> PAGE_SHIFT) &
+							(PTRS_PER_PMD-1);

Shouldn't that be "pfn += pte_index(fault_addr);"?

arch/arm/kvm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 944

Quoted 13 lines
arch/arm/kvm/mmu.c +6 -0
+				new_pte = pfn_pte(pfn, PAGE_S2);
+				pmd = stage2_get_pmd(kvm, NULL, fault_ipa);
+				if (pmd && kvm_pmd_huge(*pmd))
+					clear_pmd_entry(kvm, pmd, fault_ipa);
+			}
 			kvm_set_s2pte_writable(&new_pte);
 			kvm_set_pfn_dirty(pfn);
 		}
@@ -914,6 +941,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 		ret = stage2_set_pte(kvm, memcache, fault_ipa, &new_pte, false);
 	}
 
+	if (writable)

Shouldn't that be done only when migration is active?

Quoted 4 lines
arch/arm/kvm/mmu.c +1 -0
+		mark_page_dirty(kvm, gfn);
 
 out_unlock:
 	spin_unlock(&kvm->mmu_lock);
Signature · 1 line
Jazz is not dead. It just smells funny.
#2 lore raw

Hi Marc,

arch/arm/kvm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 931

Quoted 4 lines
arch/arm/kvm/mmu.c +4 -0
+			if (migration_active && hugetlb) {
+				pmd_t *pmd;
+				pfn += (fault_ipa >> PAGE_SHIFT) &
+							(PTRS_PER_PMD-1);

MZ> Shouldn't that be "pfn += pte_index(fault_addr);"?

I'll change much cleaner.

arch/arm/kvm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 944

Quoted 3 lines
arch/arm/kvm/mmu.c +1 -0
 	}
 
+	if (writable)

MZ> Shouldn't that be done only when migration is active? Convention in other architectures is call it anytime page is dirty, the function checks if dirty map is allocated if not it returns.

arch/arm/kvm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 948

Quoted 4 lines
arch/arm/kvm/mmu.c +1 -0
+		mark_page_dirty(kvm, gfn);
 
 out_unlock:
 	spin_unlock(&kvm->mmu_lock);

--
Jazz is not dead. It just smells funny.