[Devel] [PATCH rh7 4/2 v2] kvm: use _safe version of list iteration in mmu_shrink_scan()

[Kirill Tkhai]

On 07.06.2019 15:56, Konstantin Khorenko wrote:
> As we skip some VMs during shrink and don't want to iterate them again
> and again on each shrink, we move those skipped VMs to the list's tail,
> thus we need to use _safe version of list iteration.
> 
> Fixes: bb2d7ab43eba ("kvm: move VMs which we skip during shrink to vm_list
> tail")
> https://jira.sw.ru/browse/PSBM-95077
> 
> Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
> ---
>  arch/x86/kvm/mmu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 18c7f63fcccd..7d18cda1e2db 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -5343,13 +5343,13 @@ void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm, struct kvm_memslots *slots)
>  static unsigned long
>  mmu_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
>  {
> -	struct kvm *kvm;
> +	struct kvm *kvm, *tmp;
>  	int nr_to_scan = sc->nr_to_scan;
>  	unsigned long freed = 0;
>  
>  	spin_lock(&kvm_lock);
>  
> -	list_for_each_entry(kvm, &vm_list, vm_list) {
> +	list_for_each_entry_safe(kvm, tmp, &vm_list, vm_list) {
>  		int idx;
>  		LIST_HEAD(invalid_list);

I'm not sure this is enough. Imagine: you have 3 entries in the list,
and all of related kvm are not shrinkable. Then, you will just move
each of them to tail without actual shrinking, and number of useless
iterations will be nr_to_scan (you will move each entry to tail
nr_to_scan / 3 times). Not so good.

I'd suggested to move a batch of entries after we found actually
shrinkable element.

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 0c3ded90dd38..6d21f333f09a 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -5371,7 +5371,6 @@ mmu_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
 		 * per-vm shrinkers cry out
 		 * sadness comes quickly
 		 */
-		list_move_tail(&kvm->vm_list, &vm_list);
 
 		/*
 		 * n_used_mmu_pages is accessed without holding kvm->mmu_lock
@@ -5383,7 +5382,9 @@ mmu_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
 		      !kvm_has_zapped_obsolete_pages(kvm))
 			continue;
 
-		kvm_get_kvm(kvm);
+		if (!kvm_try_get_kvm(kvm))
+			continue;
+		list_bulk_move_tail(&vm_list, vm_list.next, &kvm->vm_list);
 		spin_unlock(&kvm_lock);
 
 		idx = srcu_read_lock(&kvm->srcu);


This requires to pick list_bulk_move_tail() primitive from recent kernel.

Kirill