KVM: x86: Drop unused return value of kvm_mmu_remove_some_alloc_mmu_pages

freed_pages is never evaluated, so remove it as well as the return code kvm_mmu_remove_some_alloc_mmu_pages so far delivered to its only user.

freed_pages is never evaluated, so remove it as well as the return code kvm_mmu_remove_some_alloc_mmu_pages so far delivered to its only user.

Signed-off-by Jan Kiszka<[email protected]>

---
  arch/x86/kvm/mmu.c |   12 ++++++------
  1 files changed, 6 insertions(+), 6 deletions(-)

arch/x86/kvm/mmu.c +4 -4

index b1178d1..efb8576 100644
@@ -3898,14 +3898,14 @@ restart:
  	spin_unlock(&kvm->mmu_lock);
  }
 
-static int kvm_mmu_remove_some_alloc_mmu_pages(struct kvm *kvm,
-					       struct list_head *invalid_list)
+static void kvm_mmu_remove_some_alloc_mmu_pages(struct kvm *kvm,
+						struct list_head *invalid_list)
  {
  	struct kvm_mmu_page *page;
 
  	page = container_of(kvm->arch.active_mmu_pages.prev,
  			    struct kvm_mmu_page, link);
-	return kvm_mmu_prepare_zap_page(kvm, page, invalid_list);
+	kvm_mmu_prepare_zap_page(kvm, page, invalid_list);
  }
 
  static int mmu_shrink(struct shrinker *shrink, struct shrink_control *sc)

@@ -3920,15 +3920,15 @@ static int mmu_shrink(struct shrinker *shrink, struct shrink_control *sc)
  	raw_spin_lock(&kvm_lock);
 
  	list_for_each_entry(kvm,&vm_list, vm_list) {
-		int idx, freed_pages;
+		int idx;
  		LIST_HEAD(invalid_list);
 
  		idx = srcu_read_lock(&kvm->srcu);

Hi, I was looking at the same place differently.

Quoted 50 lines · Jan Kiszka

freed_pages is never evaluated, so remove it as well as the return code kvm_mmu_remove_some_alloc_mmu_pages so far delivered to its only user.

Signed-off-by Jan Kiszka<[email protected]>

---
  arch/x86/kvm/mmu.c |   12 ++++++------
  1 files changed, 6 insertions(+), 6 deletions(-)

arch/x86/kvm/mmu.c +3 -3

index b1178d1..efb8576 100644
@@ -3898,14 +3898,14 @@ restart:
      spin_unlock(&kvm->mmu_lock);
  }
 
-static int kvm_mmu_remove_some_alloc_mmu_pages(struct kvm *kvm,
-                           struct list_head *invalid_list)
+static void kvm_mmu_remove_some_alloc_mmu_pages(struct kvm *kvm,
+                        struct list_head *invalid_list)
  {
      struct kvm_mmu_page *page;
 
      page = container_of(kvm->arch.active_mmu_pages.prev,
                  struct kvm_mmu_page, link);
-    return kvm_mmu_prepare_zap_page(kvm, page, invalid_list);
+    kvm_mmu_prepare_zap_page(kvm, page, invalid_list);
  }
 
  static int mmu_shrink(struct shrinker *shrink, struct

shrink_control *sc)

arch/x86/kvm/mmu.c +3 -3

@@ -3920,15 +3920,15 @@ static int mmu_shrink(struct shrinker
 *shrink, struct shrink_control *sc)
      raw_spin_lock(&kvm_lock);
 
      list_for_each_entry(kvm,&vm_list, vm_list) {
-        int idx, freed_pages;
+        int idx;
          LIST_HEAD(invalid_list);
 
          idx = srcu_read_lock(&kvm->srcu);
          spin_lock(&kvm->mmu_lock);
          if (!kvm_freed&&  nr_to_scan>  0&&
          kvm->arch.n_used_mmu_pages>  0) {
-            freed_pages = kvm_mmu_remove_some_alloc_mmu_pages(kvm,
-                            &invalid_list);
+            kvm_mmu_remove_some_alloc_mmu_pages(kvm,
+                            &invalid_list);
              kvm_freed = kvm;
          }

Quoted 1 line

          nr_to_scan--;

I think mmu_shrink() is doing meaningless things.

nr_to_scan should be treated as the number of objects to scan. Here, the objects we
are trying to free is not kvm instances but shadow pages and their related objects.

So, decrementing it for each iteration is not at all what the caller expected.

Furthermore this code just frees from one VM and breaks the loop. So nr_to_scan is
not functioning well.

I was thinking how to improve this shrinker, but now, I also feel that registering
mmu_shrink() might not worth it: it may free some memory in the case of shadow paging,
but otherwise, there is little we can free by this.

Is there any need for mmu_shrink()?

Quoted 2 lines

Is there any need for mmu_shrink()?

Without it a user can easily pin large amounts of kernel memory by filling guest memory with page tables and shadowing them all.

Quoted 5 lines · Avi Kivity

Quoted 2 lines

Is there any need for mmu_shrink()?

Without it a user can easily pin large amounts of kernel memory by filling guest memory with page tables and shadowing them all.

It would make an interesting test case, btw, to try to crash the kernel this way, and exercise the shrinker.