* KVM: register the kvm mmu cache with the shrinker. @ 2008-03-25 12:15 Izik Eidus 2008-03-25 12:36 ` Avi Kivity 0 siblings, 1 reply; 6+ messages in thread From: Izik Eidus @ 2008-03-25 12:15 UTC (permalink / raw) To: kvm-devel, Avi Kivity, Anthony Liguori, Marcelo Tosatti 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 <izike@qumranet.com> 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 <izike@qumranet.com> --- arch/x86/kvm/mmu.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 54 insertions(+), 0 deletions(-) diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index c67ec62..c42c0db 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c @@ -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/ ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: KVM: register the kvm mmu cache with the shrinker. 2008-03-25 12:15 KVM: register the kvm mmu cache with the shrinker Izik Eidus @ 2008-03-25 12:36 ` Avi Kivity 2008-03-25 13:03 ` Izik Eidus 0 siblings, 1 reply; 6+ messages in thread From: Avi Kivity @ 2008-03-25 12:36 UTC (permalink / raw) To: Izik Eidus; +Cc: kvm-devel, Marcelo Tosatti Izik Eidus wrote: > 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 <izike@qumranet.com> > 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 <izike@qumranet.com> > --- > arch/x86/kvm/mmu.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 54 insertions(+), 0 deletions(-) > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > index c67ec62..c42c0db 100644 > --- a/arch/x86/kvm/mmu.c > +++ b/arch/x86/kvm/mmu.c > @@ -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) > +{ > + 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. > + 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. > + 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) { > + 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() > + 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); > > I believe it is necessary to register the shrinker in order to have any observable effect. -- 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/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: KVM: register the kvm mmu cache with the shrinker. 2008-03-25 12:36 ` Avi Kivity @ 2008-03-25 13:03 ` Izik Eidus 2008-03-27 12:45 ` Avi Kivity 0 siblings, 1 reply; 6+ messages in thread From: Izik Eidus @ 2008-03-25 13:03 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm-devel, Marcelo Tosatti Avi Kivity wrote: > Izik Eidus wrote: >> 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 <izike@qumranet.com> >> 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 <izike@qumranet.com> >> --- >> arch/x86/kvm/mmu.c | 54 >> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 files changed, 54 insertions(+), 0 deletions(-) >> >> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c >> index c67ec62..c42c0db 100644 >> --- a/arch/x86/kvm/mmu.c >> +++ b/arch/x86/kvm/mmu.c >> @@ -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) >> +{ >> + 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. > >> + 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. > >> + 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) { > >> + 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() > >> + 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); >> >> > > 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) >From 989c2f8373d8257e9c2b9a8c6ed8d629cd2a9d74 Mon Sep 17 00:00:00 2001 From: Izik Eidus <izike@qumranet.com> 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 <izike@qumranet.com> --- arch/x86/kvm/mmu.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 49 insertions(+), 0 deletions(-) diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index c67ec62..b6c1cb2 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c @@ -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: -- 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/ ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: KVM: register the kvm mmu cache with the shrinker. 2008-03-25 13:03 ` Izik Eidus @ 2008-03-27 12:45 ` Avi Kivity 2008-03-30 12:17 ` Izik Eidus 0 siblings, 1 reply; 6+ messages in thread From: Avi Kivity @ 2008-03-27 12:45 UTC (permalink / raw) To: Izik Eidus; +Cc: kvm-devel, Marcelo Tosatti Izik Eidus wrote: > 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); > } > 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(). > > 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: > -- 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: KVM: register the kvm mmu cache with the shrinker. 2008-03-27 12:45 ` Avi Kivity @ 2008-03-30 12:17 ` Izik Eidus 2008-03-30 13:30 ` Avi Kivity 0 siblings, 1 reply; 6+ messages in thread From: Izik Eidus @ 2008-03-30 12:17 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm-devel, Marcelo Tosatti Avi Kivity wrote: > Izik Eidus wrote: >> 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); >> } >> > > 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(). > >> >> 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: >> > > ok, this should address all the issues. >From a4cd289ab2502172bde994612636f84bec5e7a60 Mon Sep 17 00:00:00 2001 From: Izik Eidus <izike@qumranet.com> 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 <izike@qumranet.com> --- arch/x86/kvm/mmu.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 56 insertions(+), 2 deletions(-) diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index a0be4f5..8a6a4f9 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c @@ -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; } -- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: KVM: register the kvm mmu cache with the shrinker. 2008-03-30 12:17 ` Izik Eidus @ 2008-03-30 13:30 ` Avi Kivity 0 siblings, 0 replies; 6+ messages in thread From: Avi Kivity @ 2008-03-30 13:30 UTC (permalink / raw) To: Izik Eidus; +Cc: kvm-devel, Marcelo Tosatti Izik Eidus wrote: > ok, this should address all the issues. > > >From a4cd289ab2502172bde994612636f84bec5e7a60 Mon Sep 17 00:00:00 2001 > From: Izik Eidus <izike@qumranet.com> > Date: Sun, 30 Mar 2008 15:13:47 +0300 > Subject: [PATCH] KVM: register the kvm mmu cache with the shrinker. > > Applied, thanks. -- 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-03-30 13:30 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-03-25 12:15 KVM: register the kvm mmu cache with the shrinker Izik Eidus 2008-03-25 12:36 ` Avi Kivity 2008-03-25 13:03 ` Izik Eidus 2008-03-27 12:45 ` Avi Kivity 2008-03-30 12:17 ` Izik Eidus 2008-03-30 13:30 ` Avi Kivity
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox