* [PATCH] KVM: make kvm_mmu_zap_page() return the number of pages it actually freed.
@ 2010-05-05 1:03 Gui Jianfeng
2010-05-06 9:07 ` Avi Kivity
0 siblings, 1 reply; 4+ messages in thread
From: Gui Jianfeng @ 2010-05-05 1:03 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti; +Cc: kvm
Currently, kvm_mmu_zap_page() returning the number of freed children sp.
This might confuse the caller, because caller don't know the actual freed
number. Let's make kvm_mmu_zap_page() return the number of pages it actually
freed.
Signed-off-by: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
---
arch/x86/kvm/mmu.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 51eb6d6..8ab6820 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1503,6 +1503,8 @@ static int kvm_mmu_zap_page(struct kvm *kvm, struct kvm_mmu_page *sp)
if (sp->unsync)
kvm_unlink_unsync_page(kvm, sp);
if (!sp->root_count) {
+ /* Count self */
+ ret++;
hlist_del(&sp->hash_link);
kvm_mmu_free_page(kvm, sp);
} else {
@@ -1539,7 +1541,6 @@ void kvm_mmu_change_mmu_pages(struct kvm *kvm, unsigned int kvm_nr_mmu_pages)
page = container_of(kvm->arch.active_mmu_pages.prev,
struct kvm_mmu_page, link);
used_pages -= kvm_mmu_zap_page(kvm, page);
- used_pages--;
}
kvm_nr_mmu_pages = used_pages;
kvm->arch.n_free_mmu_pages = 0;
@@ -2908,7 +2909,7 @@ static int kvm_mmu_remove_some_alloc_mmu_pages(struct kvm *kvm)
page = container_of(kvm->arch.active_mmu_pages.prev,
struct kvm_mmu_page, link);
- return kvm_mmu_zap_page(kvm, page) + 1;
+ return kvm_mmu_zap_page(kvm, page);
}
static int mmu_shrink(int nr_to_scan, gfp_t gfp_mask)
--
1.6.5.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: make kvm_mmu_zap_page() return the number of pages it actually freed.
2010-05-05 1:03 [PATCH] KVM: make kvm_mmu_zap_page() return the number of pages it actually freed Gui Jianfeng
@ 2010-05-06 9:07 ` Avi Kivity
2010-05-06 9:25 ` Gui Jianfeng
0 siblings, 1 reply; 4+ messages in thread
From: Avi Kivity @ 2010-05-06 9:07 UTC (permalink / raw)
To: Gui Jianfeng; +Cc: Marcelo Tosatti, kvm
On 05/05/2010 04:03 AM, Gui Jianfeng wrote:
> Currently, kvm_mmu_zap_page() returning the number of freed children sp.
> This might confuse the caller, because caller don't know the actual freed
> number. Let's make kvm_mmu_zap_page() return the number of pages it actually
> freed.
>
>
if (kvm_mmu_zap_page(kvm, sp))
goto restart;
Needs to be updated.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: make kvm_mmu_zap_page() return the number of pages it actually freed.
2010-05-06 9:07 ` Avi Kivity
@ 2010-05-06 9:25 ` Gui Jianfeng
2010-05-06 9:30 ` Avi Kivity
0 siblings, 1 reply; 4+ messages in thread
From: Gui Jianfeng @ 2010-05-06 9:25 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm
Avi Kivity wrote:
> On 05/05/2010 04:03 AM, Gui Jianfeng wrote:
>> Currently, kvm_mmu_zap_page() returning the number of freed children sp.
>> This might confuse the caller, because caller don't know the actual freed
>> number. Let's make kvm_mmu_zap_page() return the number of pages it
>> actually
>> freed.
>>
>>
>
> if (kvm_mmu_zap_page(kvm, sp))
> goto restart;
>
> Needs to be updated.
Hi Avi,
if kvm_mmu_zap_page() returns 1, we don't know whether the freed sp is the one we passes into
or the child. So here just restart hash walk as long as kvm_mmu_zap_page() returning a positive
number, although sometimes un-needed hash walk will happen. This fix gets code simpler, the
idea comes from Marcelo.
Thanks,
Gui
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: make kvm_mmu_zap_page() return the number of pages it actually freed.
2010-05-06 9:25 ` Gui Jianfeng
@ 2010-05-06 9:30 ` Avi Kivity
0 siblings, 0 replies; 4+ messages in thread
From: Avi Kivity @ 2010-05-06 9:30 UTC (permalink / raw)
To: Gui Jianfeng; +Cc: Marcelo Tosatti, kvm
On 05/06/2010 12:25 PM, Gui Jianfeng wrote:
> Avi Kivity wrote:
>
>> On 05/05/2010 04:03 AM, Gui Jianfeng wrote:
>>
>>> Currently, kvm_mmu_zap_page() returning the number of freed children sp.
>>> This might confuse the caller, because caller don't know the actual freed
>>> number. Let's make kvm_mmu_zap_page() return the number of pages it
>>> actually
>>> freed.
>>>
>>>
>>>
>> if (kvm_mmu_zap_page(kvm, sp))
>> goto restart;
>>
>> Needs to be updated.
>>
> Hi Avi,
>
> if kvm_mmu_zap_page() returns 1, we don't know whether the freed sp is the one we passes into
> or the child. So here just restart hash walk as long as kvm_mmu_zap_page() returning a positive
> number, although sometimes un-needed hash walk will happen. This fix gets code simpler, the
> idea comes from Marcelo.
>
I see. Note this can invoke quadratic behaviour. I'll apply the patch,
but we need to improve this area.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-05-06 9:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-05 1:03 [PATCH] KVM: make kvm_mmu_zap_page() return the number of pages it actually freed Gui Jianfeng
2010-05-06 9:07 ` Avi Kivity
2010-05-06 9:25 ` Gui Jianfeng
2010-05-06 9:30 ` Avi Kivity
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.