Virtualization - Feb 2020 - [PATCH RFC] virtio_balloon: conservative balloon page shrinking

There are cases that users want to shrink balloon pages after the
pagecache depleted. The conservative_shrinker lets the shrinker
shrink balloon pages when all the pagecache has been reclaimed.

Signed-off-by: Wei Wang <wei.w.wang at intel.com>
---
 drivers/virtio/virtio_balloon.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 93f995f6cf36..b4c5bb13a867 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -42,6 +42,10 @@
 static struct vfsmount *balloon_mnt;
 #endif
 
+static bool conservative_shrinker = true;
+module_param(conservative_shrinker, bool, 0644);
+MODULE_PARM_DESC(conservative_shrinker, "conservatively shrink balloon
pages");
+
 enum virtio_balloon_vq {
 	VIRTIO_BALLOON_VQ_INFLATE,
 	VIRTIO_BALLOON_VQ_DEFLATE,
@@ -796,6 +800,10 @@ static unsigned long shrink_balloon_pages(struct
virtio_balloon *vb,
 {
 	unsigned long pages_freed = 0;
 
+	/* Balloon pages only gets shrunk when the pagecache depleted */
+	if (conservative_shrinker && global_node_page_state(NR_FILE_PAGES))
+		return 0;
+
 	/*
 	 * One invocation of leak_balloon can deflate at most
 	 * VIRTIO_BALLOON_ARRAY_PFNS_MAX balloon pages, so we call it
@@ -837,7 +845,11 @@ static unsigned long virtio_balloon_shrinker_count(struct
shrinker *shrinker,
 					struct virtio_balloon, shrinker);
 	unsigned long count;
 
-	count = vb->num_pages / VIRTIO_BALLOON_PAGES_PER_PAGE;
+	if (conservative_shrinker && global_node_page_state(NR_FILE_PAGES))
+		count = 0;
+	else
+		count = vb->num_pages / VIRTIO_BALLOON_PAGES_PER_PAGE;
+
 	count += vb->num_free_page_blocks * VIRTIO_BALLOON_HINT_BLOCK_PAGES;
 
 	return count;
-- 
2.17.1

On Thu, Feb 06, 2020 at 04:01:47PM +0800, Wei Wang
wrote:
> There are cases that users want to shrink balloon pages after the
> pagecache depleted. The conservative_shrinker lets the shrinker
> shrink balloon pages when all the pagecache has been reclaimed.
> 
> Signed-off-by: Wei Wang <wei.w.wang at intel.com>
I'd rather avoid module parameters, but otherwise looks 
like a reasonable idea.
Tyler, what do you think?

> ---
>  drivers/virtio/virtio_balloon.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c
b/drivers/virtio/virtio_balloon.c
> index 93f995f6cf36..b4c5bb13a867 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -42,6 +42,10 @@
>  static struct vfsmount *balloon_mnt;
>  #endif
>  
> +static bool conservative_shrinker = true;
> +module_param(conservative_shrinker, bool, 0644);
> +MODULE_PARM_DESC(conservative_shrinker, "conservatively shrink
balloon pages");
> +
>  enum virtio_balloon_vq {
>  	VIRTIO_BALLOON_VQ_INFLATE,
>  	VIRTIO_BALLOON_VQ_DEFLATE,
> @@ -796,6 +800,10 @@ static unsigned long shrink_balloon_pages(struct
virtio_balloon *vb,
>  {
>  	unsigned long pages_freed = 0;
>  
> +	/* Balloon pages only gets shrunk when the pagecache depleted */
> +	if (conservative_shrinker &&
global_node_page_state(NR_FILE_PAGES))
> +		return 0;
> +
>  	/*
>  	 * One invocation of leak_balloon can deflate at most
>  	 * VIRTIO_BALLOON_ARRAY_PFNS_MAX balloon pages, so we call it
> @@ -837,7 +845,11 @@ static unsigned long
virtio_balloon_shrinker_count(struct shrinker *shrinker,
>  					struct virtio_balloon, shrinker);
>  	unsigned long count;
>  
> -	count = vb->num_pages / VIRTIO_BALLOON_PAGES_PER_PAGE;
> +	if (conservative_shrinker &&
global_node_page_state(NR_FILE_PAGES))
I'd rather have an API for that in mm/. In particular, do we want other
shrinkers to run, not just pagecache? To pick an example I'm familiar
with, kvm mmu cache for nested virt?
> +		count = 0;
> +	else
> +		count = vb->num_pages / VIRTIO_BALLOON_PAGES_PER_PAGE;
> +
>  	count += vb->num_free_page_blocks * VIRTIO_BALLOON_HINT_BLOCK_PAGES;
>  
>  	return count;
> -- 
> 2.17.1

On 06.02.20 09:01, Wei Wang wrote:
> There are cases that users want to shrink balloon pages after the
> pagecache depleted. The conservative_shrinker lets the shrinker
> shrink balloon pages when all the pagecache has been reclaimed.
> 
> Signed-off-by: Wei Wang <wei.w.wang at intel.com>
> ---
>  drivers/virtio/virtio_balloon.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c
b/drivers/virtio/virtio_balloon.c
> index 93f995f6cf36..b4c5bb13a867 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -42,6 +42,10 @@
>  static struct vfsmount *balloon_mnt;
>  #endif
>  
> +static bool conservative_shrinker = true;
> +module_param(conservative_shrinker, bool, 0644);
> +MODULE_PARM_DESC(conservative_shrinker, "conservatively shrink
balloon pages");
> +
>  enum virtio_balloon_vq {
>  	VIRTIO_BALLOON_VQ_INFLATE,
>  	VIRTIO_BALLOON_VQ_DEFLATE,
> @@ -796,6 +800,10 @@ static unsigned long shrink_balloon_pages(struct
virtio_balloon *vb,
>  {
>  	unsigned long pages_freed = 0;
>  
> +	/* Balloon pages only gets shrunk when the pagecache depleted */
> +	if (conservative_shrinker &&
global_node_page_state(NR_FILE_PAGES))
> +		return 0;
> +
>  	/*
>  	 * One invocation of leak_balloon can deflate at most
>  	 * VIRTIO_BALLOON_ARRAY_PFNS_MAX balloon pages, so we call it
> @@ -837,7 +845,11 @@ static unsigned long
virtio_balloon_shrinker_count(struct shrinker *shrinker,
>  					struct virtio_balloon, shrinker);
>  	unsigned long count;
>  
> -	count = vb->num_pages / VIRTIO_BALLOON_PAGES_PER_PAGE;
> +	if (conservative_shrinker &&
global_node_page_state(NR_FILE_PAGES))
> +		count = 0;
> +	else
> +		count = vb->num_pages / VIRTIO_BALLOON_PAGES_PER_PAGE;
> +
>  	count += vb->num_free_page_blocks * VIRTIO_BALLOON_HINT_BLOCK_PAGES;
>  
>  	return count;
> 
so dropping caches (echo 3 > /proc/sys/vm/drop_caches) will no longer
deflate the balloon when conservative_shrinker=true?

-- 
Thanks,

David / dhildenb

On Thursday, February 6, 2020 5:04 PM, Michael S. Tsirkin
wrote:
> virtio_balloon_shrinker_count(struct shrinker *shrinker,
> >  					struct virtio_balloon, shrinker);
> >  	unsigned long count;
> >
> > -	count = vb->num_pages / VIRTIO_BALLOON_PAGES_PER_PAGE;
> > +	if (conservative_shrinker &&
global_node_page_state(NR_FILE_PAGES))
> 
> I'd rather have an API for that in mm/. In particular, do we want other
> shrinkers to run, not just pagecache? To pick an example I'm familiar
> with, kvm mmu cache for nested virt?
We could make it extendable:

#define BALLOON_SHRINKER_AFTER_PAGE_CACHE	(1 << 0)
#define BALLOON_SHRINKER_AFTER_KVM_MMU_CACHE	(1 << 1)
...

uint64_t conservative_shrinker;
if ((conservative_shrinker | BALLOON_SHRINKER_AFTER_PAGE_CACHE) &&
global_node_page_state(NR_FILE_PAGES))
	return 0;

For now, we probably only need BALLOON_SHRINKER_AFTER_PAGE_CACHE.

Best,
Wei

On Thursday, February 6, 2020 5:10 PM, David Hildenbrand
wrote:
> so dropping caches (echo 3 > /proc/sys/vm/drop_caches) will no longer
> deflate the balloon when conservative_shrinker=true?
> 
Should be. Need Tyler's help to test it.

Best,
Wei

On 2020/02/06 17:01, Wei Wang wrote:
> There are cases that users want to shrink balloon pages after the
> pagecache depleted. The conservative_shrinker lets the shrinker
> shrink balloon pages when all the pagecache has been reclaimed.
> 
> @@ -796,6 +800,10 @@ static unsigned long shrink_balloon_pages(struct
virtio_balloon *vb,
>  {
>  	unsigned long pages_freed = 0;
>  
> +	/* Balloon pages only gets shrunk when the pagecache depleted */
> +	if (conservative_shrinker &&
global_node_page_state(NR_FILE_PAGES))
> +		return 0;
> +
Is this NUMA aware? Can "node-A's NR_FILE_PAGES is already 0 and
node-B's
NR_FILE_PAGES is not 0, but allocation request which triggered this shrinker
wants to allocate from only node-B" happen? Can some thread keep this
shrinker
defunctional by keep increasing NR_FILE_PAGES?

Is this patch from "Re: Balloon pressuring page cache" thread? I hope
that
the guest could start reclaiming memory based on host's request (like OOM
notifier chain) which is issued when host thinks that host is getting close
to OOM and thus guests should start returning their unused memory to host.
Maybe "periodically (e.g. 5 minutes)" in addition to "upon close
to OOM
condition" is also possible.

On Saturday, February 8, 2020 8:33 PM, Tetsuo Handa
wrote:
> 
> Is this NUMA aware? Can "node-A's NR_FILE_PAGES is already 0 and
> node-B's NR_FILE_PAGES is not 0, but allocation request which triggered
this
> shrinker wants to allocate from only node-B" happen? 
No, it's a global counter.
>Can some thread keep
> this shrinker defunctional by keep increasing NR_FILE_PAGES?
Yes. Actually it's our intention - as long as there are pagecache pages,
balloon pages are avoided to be reclaimed.

> 
> Is this patch from "Re: Balloon pressuring page cache" thread? I
hope that
> the guest could start reclaiming memory based on host's request (like
OOM
> notifier chain) which is issued when host thinks that host is getting close
to
> OOM and thus guests should start returning their unused memory to host.
> Maybe "periodically (e.g. 5 minutes)" in addition to "upon
close to OOM
> condition" is also possible.
That's about the host usages. The host side management software decides when
to issue a request to balloon (either periodically or event driven), I think
there isn't anything we need to do in the balloon driver here.

Best,
Wei