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

Submitted by Kirill Tkhai on June 7, 2019, 1:35 p.m.

Details

Message ID a6a4abb7-0dcf-c5ec-ddc6-8a50e92a5394@virtuozzo.com
State New
Series "Series without cover letter"
Headers show

Commit Message

Kirill Tkhai June 7, 2019, 1:35 p.m.
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@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.



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

Kirill

Patch hide | download patch | download mbox

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);