Messages in this thread | | | Date | Wed, 16 Jun 2010 11:48:00 +0300 | From | Avi Kivity <> | Subject | Re: [RFC][PATCH 4/9] create aggregate kvm_total_used_mmu_pages value |
| |
On 06/15/2010 04:55 PM, Dave Hansen wrote: > Note: this is the real meat of the patch set. It can be applied up > to this point, and everything will probably be improved, at least > a bit. > > Of slab shrinkers, the VM code says: > > * Note that 'shrink' will be passed nr_to_scan == 0 when the VM is > * querying the cache size, so a fastpath for that case is appropriate. > > and it *means* it. Look at how it calls the shrinkers: > > nr_before = (*shrinker->shrink)(0, gfp_mask); > shrink_ret = (*shrinker->shrink)(this_scan, gfp_mask); > > So, if you do anything stupid in your shrinker, the VM will doubly > punish you. >
Ouch.
> The mmu_shrink() function takes the global kvm_lock, then acquires > every VM's kvm->mmu_lock in sequence. If we have 100 VMs, then > we're going to take 101 locks. We do it twice, so each call takes > 202 locks. If we're under memory pressure, we can have each cpu > trying to do this. It can get really hairy, and we've seen lock > spinning in mmu_shrink() be the dominant entry in profiles. > > This is guaranteed to optimize at least half of those lock > aquisitions away. It removes the need to take any of the locks > when simply trying to count objects. > > > Signed-off-by: Dave Hansen<dave@linux.vnet.ibm.com> > --- > > linux-2.6.git-dave/arch/x86/kvm/mmu.c | 33 +++++++++++++++++++++++---------- > 1 file changed, 23 insertions(+), 10 deletions(-) > > diff -puN arch/x86/kvm/mmu.c~make_global_used_value arch/x86/kvm/mmu.c > --- linux-2.6.git/arch/x86/kvm/mmu.c~make_global_used_value 2010-06-09 15:14:30.000000000 -0700 > +++ linux-2.6.git-dave/arch/x86/kvm/mmu.c 2010-06-09 15:14:30.000000000 -0700 > @@ -891,6 +891,19 @@ static int is_empty_shadow_page(u64 *spt > } > #endif > > +/* > + * This value is the sum of all of the kvm instances's > + * kvm->arch.n_used_mmu_pages values. We need a global, > + * aggregate version in order to make the slab shrinker > + * faster > + */ > +static unsigned int kvm_total_used_mmu_pages; >
The variable needs to be at the top of the file.
> +static inline void kvm_mod_used_mmu_pages(struct kvm *kvm, int nr) > +{ > + kvm->arch.n_used_mmu_pages += nr; > + kvm_total_used_mmu_pages += nr; >
Needs an atomic operation, since there's no global lock here. To avoid bouncing this cacheline around, make the variable percpu and make readers take a sum across all cpus. Side benefit is that you no longer need an atomic but a local_t, which is considerably cheaper.
> +} > + >
-- error compiling committee.c: too many arguments to function
| |