Re: pre-cleanup for SMP: move mmu cache to be VM bais

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

Dong, Eddie wrote:
> Move per VCPU mmu_memory_cache to be VM basis. 
>
> Your opnion?
>
> Eddie
>
> diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
> index 0632d0b..77989b4 100644
> --- a/drivers/kvm/kvm.h
> +++ b/drivers/kvm/kvm.h
> @@ -300,11 +300,6 @@ struct kvm_vcpu {
>  
>  	struct kvm_mmu mmu;
>  
> -	struct kvm_mmu_memory_cache mmu_pte_chain_cache;
> -	struct kvm_mmu_memory_cache mmu_rmap_desc_cache;
> -	struct kvm_mmu_memory_cache mmu_page_cache;
> -	struct kvm_mmu_memory_cache mmu_page_header_cache;
> -
>  	gfn_t last_pt_write_gfn;
>  	int   last_pt_write_count;
>  
> @@ -383,6 +378,11 @@ struct kvm {
>  	unsigned long rmap_overflow;
>  	struct list_head vm_list;
>  	struct file *filp;
> +
> +	struct kvm_mmu_memory_cache mmu_pte_chain_cache;
> +	struct kvm_mmu_memory_cache mmu_rmap_desc_cache;
> +	struct kvm_mmu_memory_cache mmu_page_cache;
> +	struct kvm_mmu_memory_cache mmu_page_header_cache;
>  };
>  
>   

This exposes a race:
>
> static int mmu_topup_memory_caches(struct kvm_vcpu *vcpu)
> {
>     int r;
>
>     r = __mmu_topup_memory_caches(vcpu, GFP_NOWAIT);
>     if (r < 0) {
>         spin_unlock(&vcpu->kvm->lock);
>         kvm_arch_ops->vcpu_put(vcpu);
>         r = __mmu_topup_memory_caches(vcpu, GFP_KERNEL);
>         kvm_arch_ops->vcpu_load(vcpu);
>         spin_lock(&vcpu->kvm->lock);
>     }
>     return r;
> }

If, after we've allocated but before we grab the lock back, another vcpu 
comes along and steals our memory, we can oom with no recovery.

This can be fixed by having a while() in there, but I don't see a real 
need.  The memory caches are local to the vcpu so they have some modest 
performance benefit (although it's probably not meausrable given all the 
other work this code path does).

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/