All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcelo Tosatti <mtosatti@redhat.com>
To: Takuya Yoshikawa <takuya.yoshikawa@gmail.com>
Cc: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>,
	Gleb Natapov <gleb@redhat.com>,
	avi@redhat.com, kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: MMU: Fix mmu_shrink() so that it can free mmu pages as intended
Date: Fri, 20 Jul 2012 11:42:12 -0300	[thread overview]
Message-ID: <20120720144212.GA19260@amt.cnet> (raw)
In-Reply-To: <20120720100434.270f2dca4604063a8789ee5a@gmail.com>

On Fri, Jul 20, 2012 at 10:04:34AM +0900, Takuya Yoshikawa wrote:
> On Wed, 18 Jul 2012 17:52:46 -0300
> Marcelo Tosatti <mtosatti@redhat.com> wrote:
> 
> > Can't understand, can you please expand more clearly? 
> 
> I think mmu pages are not worth freeing under usual memory pressure,
> especially when we have EPT/NPT on.
> 
> What's happening:
> shrink_slab() vainly calls mmu_shrink() with the default batch size 128,
> and mmu_shrink() takes a long time to zap mmu pages far fewer than the
> requested number, usually just frees one.  Sadly, KVM may recreate the
> page soon after that.
> 
> Since we set the seeks 10 times greater than the default, total_scan is
> very small and shrink_slab() just wastes time for freeing such small
> amount of may-be-reallocated-soon memory: I want it to use time for
> scanning other objects instead.
> 
> Actually the total amount of memory used for mmu pages is not huge in
> the case of EPT/NPT on: maybe smaller that that of rmap?

rmap size is a function of mmu pages, so mmu_shrink indirectly 
releases rmap also.

> So, it's clear that no one wants mmu pages to be freed as other objects.
> Sure, our seeks size prevents shrink_slab() from calling mmu_shrink()
> usually.  But what if administrators want to drop clean caches on the
> host?
> 
> Documentation/sysctl/vm.txt says:
>   Writing to this will cause the kernel to drop clean caches, dentries and
>   inodes from memory, causing that memory to become free.
> 
>   To free pagecache:
>           echo 1 > /proc/sys/vm/drop_caches
>   To free dentries and inodes:
>           echo 2 > /proc/sys/vm/drop_caches
>   To free pagecache, dentries and inodes:
>           echo 3 > /proc/sys/vm/drop_caches
> 
> I don't want mmu pages to be freed in such cases.

drop_caches should be used in special occasions. I would not worry
about it.

> So, how about stopping reporting/returning the total number of used
> mmu pages to shrink_slab()?
> 
> If we do so, it will think that there are not enough objects to get
> memory back from KVM.

No, its important to be able to release memory quickly in low memory
conditions.

I bet the reasoning behind current seeks value (10*default) is close to
arbitrary.

mmu_shrink can be smarter, by freeing pages which are less likely to
be used. IIRC Avi had some nice ideas for LRU-like schemes (search the
archives).

You can also consider the fact that freeing a higher level pagetable
frees all of its children (that is quite dumb actually, sequential
shrink passes should free only pages with no children).

> In the case of shadow paging, guests can do bad things to allocate
> enormous mmu pages, so we should report such exceeded numbers to
> shrink_slab() as freeable objects, not the total.

A guest idle for 2 months should not have its mmu pages in memory.

>   |--- needed ---|--- freeable under memory pressure ---|
> 
> We may be able to use n_max_mmu_pages for this: the shrinker tries
> to free mmu pages unless the number reaches the goal.
> 
> Thanks,
> 	Takuya
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

      reply	other threads:[~2012-07-20 15:05 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-05 10:56 [PATCH] KVM: MMU: Fix mmu_shrink() so that it can free mmu pages as intended Takuya Yoshikawa
2012-07-05 11:50 ` Gleb Natapov
2012-07-05 14:05   ` Takuya Yoshikawa
2012-07-12  9:35     ` Takuya Yoshikawa
2012-07-18 20:52       ` Marcelo Tosatti
2012-07-20  1:04         ` Takuya Yoshikawa
2012-07-20 14:42           ` Marcelo Tosatti [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120720144212.GA19260@amt.cnet \
    --to=mtosatti@redhat.com \
    --cc=avi@redhat.com \
    --cc=gleb@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=takuya.yoshikawa@gmail.com \
    --cc=yoshikawa.takuya@oss.ntt.co.jp \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.