diff options
author | Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp> | 2012-08-20 05:35:39 -0400 |
---|---|---|
committer | Avi Kivity <avi@redhat.com> | 2012-08-22 08:27:13 -0400 |
commit | 35f2d16bb9ace0fb2671b8232839944ad9057c6f (patch) | |
tree | 763610b668cb62664b55a838488e96dad829a9b0 /arch/x86 | |
parent | e8143ccb6b501f78bb95d9c5ee100d18423008cf (diff) |
KVM: MMU: Fix mmu_shrink() so that it can free mmu pages as intended
Although the possible race described in
commit 85b7059169e128c57a3a8a3e588fb89cb2031da1
KVM: MMU: fix shrinking page from the empty mmu
was correct, the real cause of that issue was a more trivial bug of
mmu_shrink() introduced by
commit 1952639665e92481c34c34c3e2a71bf3e66ba362
KVM: MMU: do not iterate over all VMs in mmu_shrink()
Here is the bug:
if (kvm->arch.n_used_mmu_pages > 0) {
if (!nr_to_scan--)
break;
continue;
}
We skip VMs whose n_used_mmu_pages is not zero and try to shrink others:
in other words we try to shrink empty ones by mistake.
This patch reverses the logic so that mmu_shrink() can free pages from
the first VM whose n_used_mmu_pages is not zero. Note that we also add
comments explaining the role of nr_to_scan which is not practically
important now, hoping this will be improved in the future.
Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
Cc: Gleb Natapov <gleb@redhat.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
Diffstat (limited to 'arch/x86')
-rw-r--r-- | arch/x86/kvm/mmu.c | 13 |
1 files changed, 9 insertions, 4 deletions
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index 01ca00423938..7fbd0d273ea8 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c | |||
@@ -4113,16 +4113,21 @@ static int mmu_shrink(struct shrinker *shrink, struct shrink_control *sc) | |||
4113 | LIST_HEAD(invalid_list); | 4113 | LIST_HEAD(invalid_list); |
4114 | 4114 | ||
4115 | /* | 4115 | /* |
4116 | * Never scan more than sc->nr_to_scan VM instances. | ||
4117 | * Will not hit this condition practically since we do not try | ||
4118 | * to shrink more than one VM and it is very unlikely to see | ||
4119 | * !n_used_mmu_pages so many times. | ||
4120 | */ | ||
4121 | if (!nr_to_scan--) | ||
4122 | break; | ||
4123 | /* | ||
4116 | * n_used_mmu_pages is accessed without holding kvm->mmu_lock | 4124 | * n_used_mmu_pages is accessed without holding kvm->mmu_lock |
4117 | * here. We may skip a VM instance errorneosly, but we do not | 4125 | * here. We may skip a VM instance errorneosly, but we do not |
4118 | * want to shrink a VM that only started to populate its MMU | 4126 | * want to shrink a VM that only started to populate its MMU |
4119 | * anyway. | 4127 | * anyway. |
4120 | */ | 4128 | */ |
4121 | if (kvm->arch.n_used_mmu_pages > 0) { | 4129 | if (!kvm->arch.n_used_mmu_pages) |
4122 | if (!nr_to_scan--) | ||
4123 | break; | ||
4124 | continue; | 4130 | continue; |
4125 | } | ||
4126 | 4131 | ||
4127 | idx = srcu_read_lock(&kvm->srcu); | 4132 | idx = srcu_read_lock(&kvm->srcu); |
4128 | spin_lock(&kvm->mmu_lock); | 4133 | spin_lock(&kvm->mmu_lock); |