Skip to contentSkip to conversation

KVM: register the kvm mmu cache with the shrinker.

PATCH Applied Izik Eidus 2 participants 6 messages Mar 25, 2008 – Mar 30, 2008

Subsystems (3)
x86 KVM KVM x86
lore
Share & tools
lore

Izik Eidus

author
#0 lore raw

this is the shrinker patch with all comments applied beside adding aging mechanism it look like the aging mechanism is not really needed and therefor for now it isn't implemented.

Quoted 1 line

From 8503a57ae88ba819e4eac6371172772c98b485f0 Mon Sep 17 00:00:00 2001

From: Izik Eidus <[email protected]>
Date: Tue, 25 Mar 2008 14:03:27 +0200
Subject: [PATCH] KVM: register the kvm mmu cache with the shrinker.

Signed-off-by Izik Eidus <[email protected]>
---
 arch/x86/kvm/mmu.c |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 54 insertions(+), 0 deletions(-)
arch/x86/kvm/mmu.c +51 -1
index c67ec62..c42c0db 100644
@@ -1972,6 +1972,57 @@ void kvm_mmu_zap_all(struct kvm *kvm)
 	kvm_flush_remote_tlbs(kvm);
 }
 
+void kvm_mmu_remove_one_alloc_mmu_page(struct kvm *kvm)
+{
+	struct kvm_mmu_page *page;
+
+	page = container_of(kvm->arch.active_mmu_pages.prev,
+			    struct kvm_mmu_page, link);
+	kvm_mmu_zap_page(kvm, page);
+}
+
+static int mmu_shrink(int nr_to_scan, gfp_t gfp_mask)
+{
+	struct kvm *kvm;
+	struct kvm *kvm_freed = NULL;
+	int cache_count = 0;
+
+	spin_lock(&kvm_lock);
+	if (list_empty(&vm_list)) {
+		spin_unlock(&kvm_lock);
+		return 0;
+	}
+	list_for_each_entry(kvm, &vm_list, vm_list) {
+		int npages;
+
+		spin_lock(&kvm->mmu_lock);
+		npages = kvm->arch.n_alloc_mmu_pages -
+			 kvm->arch.n_free_mmu_pages;
+		cache_count += npages - KVM_MIN_ALLOC_MMU_PAGES;
+		if (!kvm_freed && nr_to_scan > 0 && npages >
+		    KVM_MIN_ALLOC_MMU_PAGES) {
+			kvm_mmu_remove_one_alloc_mmu_page(kvm);
+			cache_count--;
+			kvm_freed = kvm;
+		}
+		nr_to_scan--;
+
+		spin_unlock(&kvm->mmu_lock);
+	}
+	if (kvm_freed) {
+		list_del(&kvm_freed->vm_list);
+		list_add_tail(&kvm_freed->vm_list, &vm_list);
+	}
+	spin_unlock(&kvm_lock);
+
+	return cache_count;
+}
+
+static struct shrinker mmu_shrinker = {
+	.shrink = mmu_shrink,
+	.seeks = DEFAULT_SEEKS * 10,
+};
+
 void kvm_mmu_module_exit(void)
 {
 	if (pte_chain_cache)

@@ -1980,6 +2031,7 @@ void kvm_mmu_module_exit(void)
 		kmem_cache_destroy(rmap_desc_cache);
 	if (mmu_page_header_cache)
 		kmem_cache_destroy(mmu_page_header_cache);
 
--

woof.

-------------------------------------------------------------------------
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/

#1 lore raw
Quoted 23 lines · Izik Eidus

this is the shrinker patch with all comments applied beside adding aging mechanism it look like the aging mechanism is not really needed and therefor for now it isn't implemented.

From 8503a57ae88ba819e4eac6371172772c98b485f0 Mon Sep 17 00:00:00 2001
From: Izik Eidus <[email protected]>
Date: Tue, 25 Mar 2008 14:03:27 +0200
Subject: [PATCH] KVM: register the kvm mmu cache with the shrinker.

Signed-off-by Izik Eidus <[email protected]>
---
 arch/x86/kvm/mmu.c |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 54 insertions(+), 0 deletions(-)
arch/x86/kvm/mmu.c +2 -0
index c67ec62..c42c0db 100644
@@ -1972,6 +1972,57 @@ void kvm_mmu_zap_all(struct kvm *kvm)
 	kvm_flush_remote_tlbs(kvm);
 }
+static int mmu_shrink(int nr_to_scan, gfp_t gfp_mask)
+{

arch/x86/kvm/mmu.c void kvm_mmu_zap_all(struct kvm *kvm) · line 1985

Quoted 10 lines
arch/x86/kvm/mmu.c +9 -0
+	struct kvm *kvm;
+	struct kvm *kvm_freed = NULL;
+	int cache_count = 0;
+
+	spin_lock(&kvm_lock);
+	if (list_empty(&vm_list)) {
+		spin_unlock(&kvm_lock);
+		return 0;
+	}

Unnecessary, if the list is empty the loop below doesn't execute and we return 0 anyway.

arch/x86/kvm/mmu.c void kvm_mmu_zap_all(struct kvm *kvm)

Quoted 8 lines
arch/x86/kvm/mmu.c +7 -0
+	list_for_each_entry(kvm, &vm_list, vm_list) {
+		int npages;
+
+		spin_lock(&kvm->mmu_lock);
+		npages = kvm->arch.n_alloc_mmu_pages -
+			 kvm->arch.n_free_mmu_pages;
+		cache_count += npages - KVM_MIN_ALLOC_MMU_PAGES;

I think we should allow the shrinker to go below KVM_MIN_ALLOC_MMU_PAGES; in particular, if the vm is inactive, we should be able to shrink it to nothing.

When the VM starts executing again, it will reallocate those pages.

arch/x86/kvm/mmu.c void kvm_mmu_zap_all(struct kvm *kvm)

Quoted 3 lines
arch/x86/kvm/mmu.c +2 -0
+		if (!kvm_freed && nr_to_scan > 0 && npages >
+		    KVM_MIN_ALLOC_MMU_PAGES) {

Don't split an expression like that; the tightly binding expression should stay on the same line:

if (!kvm_freed && nr_to_scan > 0 &&

     npages > KVM_MN_ALLOC_MMU_PAGES) {

arch/x86/kvm/mmu.c void kvm_mmu_zap_all(struct kvm *kvm)

Quoted 13 lines
arch/x86/kvm/mmu.c +12 -0
+			kvm_mmu_remove_one_alloc_mmu_page(kvm);
+			cache_count--;
+			kvm_freed = kvm;
+		}
+		nr_to_scan--;
+
+		spin_unlock(&kvm->mmu_lock);
+	}
+	if (kvm_freed) {
+		list_del(&kvm_freed->vm_list);
+		list_add_tail(&kvm_freed->vm_list, &vm_list);
+	}

list_move_tail()

Quoted 14 lines
arch/x86/kvm/mmu.c +10 -0
+	spin_unlock(&kvm_lock);
+
+	return cache_count;
+}
+
+static struct shrinker mmu_shrinker = {
+	.shrink = mmu_shrink,
+	.seeks = DEFAULT_SEEKS * 10,
+};
+
 void kvm_mmu_module_exit(void)
 {
 	if (pte_chain_cache)

arch/x86/kvm/mmu.c void kvm_mmu_module_exit(void) · line 2035

Quoted 5 lines
arch/x86/kvm/mmu.c +0 -0
@@ -1980,6 +2031,7 @@ void kvm_mmu_module_exit(void)
 		kmem_cache_destroy(rmap_desc_cache);
 	if (mmu_page_header_cache)
 		kmem_cache_destroy(mmu_page_header_cache);

I believe it is necessary to register the shrinker in order to have any observable effect.

Signature · 7 lines
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
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/
author
#2 lore raw
Quoted 96 lines · Avi Kivity
Quoted 26 lines · Izik Eidus

this is the shrinker patch with all comments applied beside adding aging mechanism
it look like the aging mechanism is not really needed and therefor for now it isn't
implemented.

From 8503a57ae88ba819e4eac6371172772c98b485f0 Mon Sep 17 00:00:00 2001
From: Izik Eidus <[email protected]>
Date: Tue, 25 Mar 2008 14:03:27 +0200
Subject: [PATCH] KVM: register the kvm mmu cache with the shrinker.

Signed-off-by Izik Eidus <[email protected]>
---
 arch/x86/kvm/mmu.c |   54
++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 54 insertions(+), 0 deletions(-)
arch/x86/kvm/mmu.c +2 -0
index c67ec62..c42c0db 100644
@@ -1972,6 +1972,57 @@ void kvm_mmu_zap_all(struct kvm *kvm)
     kvm_flush_remote_tlbs(kvm);
 }
+static int mmu_shrink(int nr_to_scan, gfp_t gfp_mask)
+{

arch/x86/kvm/mmu.c void kvm_mmu_zap_all(struct kvm *kvm) · line 1985

Quoted 10 lines
arch/x86/kvm/mmu.c +9 -0
+    struct kvm *kvm;
+    struct kvm *kvm_freed = NULL;
+    int cache_count = 0;
+
+    spin_lock(&kvm_lock);
+    if (list_empty(&vm_list)) {
+        spin_unlock(&kvm_lock);
+        return 0;
+    }

Unnecessary, if the list is empty the loop below doesn't execute and we return 0 anyway.

arch/x86/kvm/mmu.c void kvm_mmu_zap_all(struct kvm *kvm) · line 1993

Quoted 8 lines
arch/x86/kvm/mmu.c +7 -0
+    list_for_each_entry(kvm, &vm_list, vm_list) {
+        int npages;
+
+        spin_lock(&kvm->mmu_lock);
+        npages = kvm->arch.n_alloc_mmu_pages -
+             kvm->arch.n_free_mmu_pages;
+        cache_count += npages - KVM_MIN_ALLOC_MMU_PAGES;

I think we should allow the shrinker to go below KVM_MIN_ALLOC_MMU_PAGES; in particular, if the vm is inactive, we should be able to shrink it to nothing.

When the VM starts executing again, it will reallocate those pages.

arch/x86/kvm/mmu.c void kvm_mmu_zap_all(struct kvm *kvm) · line 1996

Quoted 3 lines
arch/x86/kvm/mmu.c +2 -0
+        if (!kvm_freed && nr_to_scan > 0 && npages >
+            KVM_MIN_ALLOC_MMU_PAGES) {

Don't split an expression like that; the tightly binding expression should stay on the same line:

if (!kvm_freed && nr_to_scan > 0 &&

    npages > KVM_MN_ALLOC_MMU_PAGES) {

arch/x86/kvm/mmu.c void kvm_mmu_zap_all(struct kvm *kvm) · line 2009

Quoted 13 lines
arch/x86/kvm/mmu.c +12 -0
+            kvm_mmu_remove_one_alloc_mmu_page(kvm);
+            cache_count--;
+            kvm_freed = kvm;
+        }
+        nr_to_scan--;
+
+        spin_unlock(&kvm->mmu_lock);
+    }
+    if (kvm_freed) {
+        list_del(&kvm_freed->vm_list);
+        list_add_tail(&kvm_freed->vm_list, &vm_list);
+    }

list_move_tail()

Quoted 14 lines
arch/x86/kvm/mmu.c +10 -0
+    spin_unlock(&kvm_lock);
+
+    return cache_count;
+}
+
+static struct shrinker mmu_shrinker = {
+    .shrink = mmu_shrink,
+    .seeks = DEFAULT_SEEKS * 10,
+};
+
 void kvm_mmu_module_exit(void)
 {
     if (pte_chain_cache)

@@ -1980,6 +2031,7 @@ void kvm_mmu_module_exit(void)
Quoted 9 lines

arch/x86/kvm/mmu.c void kvm_mmu_module_exit(void) · line 2035

Quoted 5 lines
arch/x86/kvm/mmu.c +0 -0
         kmem_cache_destroy(rmap_desc_cache);
     if (mmu_page_header_cache)
         kmem_cache_destroy(mmu_page_header_cache);

I believe it is necessary to register the shrinker in order to have any observable effect.

sorry, i didnt send the whole patch
here the one with your comments applied.
(btw, i have just saw something weird about the memory, so wait before you applies it)

Quoted 1 line

From 989c2f8373d8257e9c2b9a8c6ed8d629cd2a9d74 Mon Sep 17 00:00:00 2001

From: Izik Eidus <[email protected]>
Date: Tue, 25 Mar 2008 14:03:27 +0200
Subject: [PATCH] KVM: register the kvm mmu cache with the shrinker.

Signed-off-by Izik Eidus <[email protected]>
---
 arch/x86/kvm/mmu.c |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 49 insertions(+), 0 deletions(-)
arch/x86/kvm/mmu.c +49 -0
index c67ec62..b6c1cb2 100644
@@ -1972,6 +1972,52 @@ void kvm_mmu_zap_all(struct kvm *kvm)
 	kvm_flush_remote_tlbs(kvm);
 }
 
+void kvm_mmu_remove_one_alloc_mmu_page(struct kvm *kvm)
+{
+	struct kvm_mmu_page *page;
+
+	page = container_of(kvm->arch.active_mmu_pages.prev,
+			    struct kvm_mmu_page, link);
+	kvm_mmu_zap_page(kvm, page);
+}
+
+static int mmu_shrink(int nr_to_scan, gfp_t gfp_mask)
+{
+	struct kvm *kvm;
+	struct kvm *kvm_freed = NULL;
+	int cache_count = 0;
+
+	spin_lock(&kvm_lock);
+
+	list_for_each_entry(kvm, &vm_list, vm_list) {
+		int npages;
+
+		spin_lock(&kvm->mmu_lock);
+		npages = kvm->arch.n_alloc_mmu_pages -
+			 kvm->arch.n_free_mmu_pages;
+		cache_count += npages;
+		if (!kvm_freed && nr_to_scan > 0 && npages > 0) {
+			kvm_mmu_remove_one_alloc_mmu_page(kvm);
+			cache_count--;
+			kvm_freed = kvm;
+		}
+		nr_to_scan--;
+
+		spin_unlock(&kvm->mmu_lock);
+	}
+	if (kvm_freed)
+		list_move_tail(&kvm_freed->vm_list, &vm_list);
+
+	spin_unlock(&kvm_lock);
+
+	return cache_count;
+}
+
+static struct shrinker mmu_shrinker = {
+	.shrink = mmu_shrink,
+	.seeks = DEFAULT_SEEKS * 10,
+};
+
 void kvm_mmu_module_exit(void)
 {
 	if (pte_chain_cache)

@@ -1980,6 +2026,7 @@ void kvm_mmu_module_exit(void)
 		kmem_cache_destroy(rmap_desc_cache);
 	if (mmu_page_header_cache)
 		kmem_cache_destroy(mmu_page_header_cache);
+	unregister_shrinker(&mmu_shrinker);
 }
 
 int kvm_mmu_module_init(void)

@@ -2001,6 +2048,8 @@ int kvm_mmu_module_init(void)
 	if (!mmu_page_header_cache)
 		goto nomem;
 
+	register_shrinker(&mmu_shrinker);
+
 	return 0;
 
 nomem:
Signature · 10 lines
1.5.3.6


-- 
woof.

-------------------------------------------------------------------------
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/
#3 lore raw

arch/x86/kvm/mmu.c void kvm_mmu_module_exit(void) · line 2031

Quoted 10 lines · Izik Eidus

void kvm_mmu_module_exit(void)
{

arch/x86/kvm/mmu.c +1 -0
 	if (pte_chain_cache)

@@ -1980,6 +2026,7 @@ void kvm_mmu_module_exit(void)
 		kmem_cache_destroy(rmap_desc_cache);
 	if (mmu_page_header_cache)
 		kmem_cache_destroy(mmu_page_header_cache);
+	unregister_shrinker(&mmu_shrinker);
 }

kvm_mmu_module_exit() can be called from the nomem: label below, so the shrinker will be unregistered despite not being registered.

Suggest renaming kvm_mmu_module_exit() to mmu_destroy_caches(), and adding a new kvm_mmu_module_exit() which will call mmu_destroy_caches() and unregister_shrinker().

Quoted 12 lines
arch/x86/kvm/mmu.c +2 -0
 int kvm_mmu_module_init(void)

@@ -2001,6 +2048,8 @@ int kvm_mmu_module_init(void)
 	if (!mmu_page_header_cache)
 		goto nomem;
 
+	register_shrinker(&mmu_shrinker);
+
 	return 0;
 
 nomem:
Signature · 8 lines
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
author
#4 lore raw
Quoted 25 lines · Avi Kivity

Review void kvm_mmu_module_exit(void) · line 2031

Quoted 10 lines · Izik Eidus

void kvm_mmu_module_exit(void)
{
if (pte_chain_cache)

(quoted diff) +1 -0
@@ -1980,6 +2026,7 @@ void kvm_mmu_module_exit(void)
         kmem_cache_destroy(rmap_desc_cache);
     if (mmu_page_header_cache)
         kmem_cache_destroy(mmu_page_header_cache);
+    unregister_shrinker(&mmu_shrinker);
 }

kvm_mmu_module_exit() can be called from the nomem: label below, so the shrinker will be unregistered despite not being registered.

Suggest renaming kvm_mmu_module_exit() to mmu_destroy_caches(), and adding a new kvm_mmu_module_exit() which will call mmu_destroy_caches() and unregister_shrinker().

Quoted 6 lines
(quoted diff) +0 -0
 int kvm_mmu_module_init(void)

@@ -2001,6 +2048,8 @@ int kvm_mmu_module_init(void)
     if (!mmu_page_header_cache)
         goto nomem;
Quoted 8 lines

Review int kvm_mmu_module_init(void) · line 2055

Quoted 6 lines
(quoted diff) +2 -0
+    register_shrinker(&mmu_shrinker);
+
     return 0;
 
 nomem:

ok, this should address all the issues.

Quoted 1 line

From a4cd289ab2502172bde994612636f84bec5e7a60 Mon Sep 17 00:00:00 2001

From: Izik Eidus <[email protected]>
Date: Sun, 30 Mar 2008 15:13:47 +0300
Subject: [PATCH] KVM: register the kvm mmu cache with the shrinker.

Signed-off-by Izik Eidus <[email protected]>
---
 arch/x86/kvm/mmu.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 56 insertions(+), 2 deletions(-)
arch/x86/kvm/mmu.c +56 -2
index a0be4f5..8a6a4f9 100644
@@ -1963,7 +1963,53 @@ void kvm_mmu_zap_all(struct kvm *kvm)
 	kvm_flush_remote_tlbs(kvm);
 }
 
-void kvm_mmu_module_exit(void)
+void kvm_mmu_remove_one_alloc_mmu_page(struct kvm *kvm)
+{
+	struct kvm_mmu_page *page;
+
+	page = container_of(kvm->arch.active_mmu_pages.prev,
+			    struct kvm_mmu_page, link);
+	kvm_mmu_zap_page(kvm, page);
+}
+
+static int mmu_shrink(int nr_to_scan, gfp_t gfp_mask)
+{
+	struct kvm *kvm;
+	struct kvm *kvm_freed = NULL;
+	int cache_count = 0;
+
+	spin_lock(&kvm_lock);
+
+	list_for_each_entry(kvm, &vm_list, vm_list) {
+		int npages;
+
+		spin_lock(&kvm->mmu_lock);
+		npages = kvm->arch.n_alloc_mmu_pages -
+			 kvm->arch.n_free_mmu_pages;
+		cache_count += npages;
+		if (!kvm_freed && nr_to_scan > 0 && npages > 0) {
+			kvm_mmu_remove_one_alloc_mmu_page(kvm);
+			cache_count--;
+			kvm_freed = kvm;
+		}
+		nr_to_scan--;
+
+		spin_unlock(&kvm->mmu_lock);
+	}
+	if (kvm_freed)
+		list_move_tail(&kvm_freed->vm_list, &vm_list);
+
+	spin_unlock(&kvm_lock);
+
+	return cache_count;
+}
+
+static struct shrinker mmu_shrinker = {
+	.shrink = mmu_shrink,
+	.seeks = DEFAULT_SEEKS * 10,
+};
+
+void mmu_destroy_caches(void)
 {
 	if (pte_chain_cache)
 		kmem_cache_destroy(pte_chain_cache);

@@ -1973,6 +2019,12 @@ void kvm_mmu_module_exit(void)
 		kmem_cache_destroy(mmu_page_header_cache);
 }
 
+void kvm_mmu_module_exit(void)
+{
+	mmu_destroy_caches();
+	unregister_shrinker(&mmu_shrinker);
+}
+
 int kvm_mmu_module_init(void)
 {
 	pte_chain_cache = kmem_cache_create("kvm_pte_chain",

@@ -1992,10 +2044,12 @@ int kvm_mmu_module_init(void)
 	if (!mmu_page_header_cache)
 		goto nomem;
 
+	register_shrinker(&mmu_shrinker);
+
 	return 0;
 
 nomem:
-	kvm_mmu_module_exit();
+	mmu_destroy_caches();
 	return -ENOMEM;
 }
Signature · 11 lines
1.5.3.6


-- 
woof.

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
Applied
#5 lore raw
Quoted 8 lines · Izik Eidus

ok, this should address all the issues.

Quoted 1 line

From a4cd289ab2502172bde994612636f84bec5e7a60 Mon Sep 17 00:00:00 2001

From: Izik Eidus <[email protected]>
Date: Sun, 30 Mar 2008 15:13:47 +0300
Subject: [PATCH] KVM: register the kvm mmu cache with the shrinker.

Applied, thanks.

Signature · 8 lines
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace