Linux-HyperV Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] x86/Hyper-V: Balloon up according to request page number
@ 2020-01-14  7:44 lantianyu1986
  2020-01-14  9:30 ` Vitaly Kuznetsov
  0 siblings, 1 reply; 3+ messages in thread
From: lantianyu1986 @ 2020-01-14  7:44 UTC (permalink / raw)
  To: tglx, mingo, bp, hpa, x86, dave.hansen, luto, peterz, kys,
	haiyangz, sthemmin, sashal, akpm, michael.h.kelley, decui
  Cc: Tianyu Lan, linux-kernel, linux-hyperv, linux-mm, vkuznets, stable

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

Current code has assumption that balloon request memory size aligns
with 2MB. But actually Hyper-V doesn't guarantee such alignment. When
balloon driver receives non-aligned balloon request, it produces warning
and balloon up more memory than requested in order to keep 2MB alignment.
Remove the warning and balloon up memory according to actual requested
memory size.

Fixes: f6712238471a ("hv: hv_balloon: avoid memory leak on alloc_error of 2MB memory block")
Cc: stable@vger.kernel.org
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
 drivers/hv/hv_balloon.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 7f3e7ab22d5d..38ad0e44e927 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -1684,7 +1684,7 @@ static unsigned int alloc_balloon_pages(struct hv_dynmem_device *dm,
 	if (num_pages < alloc_unit)
 		return 0;
 
-	for (i = 0; (i * alloc_unit) < num_pages; i++) {
+	for (i = 0; i < num_pages / alloc_unit; i++) {
 		if (bl_resp->hdr.size + sizeof(union dm_mem_page_range) >
 			HV_HYP_PAGE_SIZE)
 			return i * alloc_unit;
@@ -1722,7 +1722,7 @@ static unsigned int alloc_balloon_pages(struct hv_dynmem_device *dm,
 
 	}
 
-	return num_pages;
+	return i * alloc_unit;
 }
 
 static void balloon_up(union dm_msg_info *msg_info)
@@ -1737,9 +1737,6 @@ static void balloon_up(union dm_msg_info *msg_info)
 	long avail_pages;
 	unsigned long floor;
 
-	/* The host balloons pages in 2M granularity. */
-	WARN_ON_ONCE(num_pages % PAGES_IN_2M != 0);
-
 	/*
 	 * We will attempt 2M allocations. However, if we fail to
 	 * allocate 2M chunks, we will go back to PAGE_SIZE allocations.
-- 
2.14.5


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] x86/Hyper-V: Balloon up according to request page number
  2020-01-14  7:44 [PATCH] x86/Hyper-V: Balloon up according to request page number lantianyu1986
@ 2020-01-14  9:30 ` Vitaly Kuznetsov
  2020-01-16 14:17   ` [EXTERNAL] " Tianyu Lan
  0 siblings, 1 reply; 3+ messages in thread
From: Vitaly Kuznetsov @ 2020-01-14  9:30 UTC (permalink / raw)
  To: lantianyu1986
  Cc: linux-kernel, linux-hyperv, linux-mm, stable, tglx, mingo, bp,
	hpa, x86, dave.hansen, luto, peterz, kys, haiyangz, sthemmin,
	sashal, akpm, michael.h.kelley, decui

lantianyu1986@gmail.com writes:

> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
>
> Current code has assumption that balloon request memory size aligns
> with 2MB. But actually Hyper-V doesn't guarantee such alignment. When
> balloon driver receives non-aligned balloon request, it produces warning
> and balloon up more memory than requested in order to keep 2MB alignment.
> Remove the warning and balloon up memory according to actual requested
> memory size.
>
> Fixes: f6712238471a ("hv: hv_balloon: avoid memory leak on alloc_error of 2MB memory block")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
>  drivers/hv/hv_balloon.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index 7f3e7ab22d5d..38ad0e44e927 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -1684,7 +1684,7 @@ static unsigned int alloc_balloon_pages(struct hv_dynmem_device *dm,
>  	if (num_pages < alloc_unit)
>  		return 0;
>  
> -	for (i = 0; (i * alloc_unit) < num_pages; i++) {
> +	for (i = 0; i < num_pages / alloc_unit; i++) {
>  		if (bl_resp->hdr.size + sizeof(union dm_mem_page_range) >
>  			HV_HYP_PAGE_SIZE)
>  			return i * alloc_unit;
> @@ -1722,7 +1722,7 @@ static unsigned int alloc_balloon_pages(struct hv_dynmem_device *dm,
>  
>  	}
>  
> -	return num_pages;
> +	return i * alloc_unit;
>  }
>  
>  static void balloon_up(union dm_msg_info *msg_info)
> @@ -1737,9 +1737,6 @@ static void balloon_up(union dm_msg_info *msg_info)
>  	long avail_pages;
>  	unsigned long floor;
>  
> -	/* The host balloons pages in 2M granularity. */
> -	WARN_ON_ONCE(num_pages % PAGES_IN_2M != 0);
> -
>  	/*
>  	 * We will attempt 2M allocations. However, if we fail to
>  	 * allocate 2M chunks, we will go back to PAGE_SIZE allocations.

This looks correct but I've noticed we also have 

	/* Refuse to balloon below the floor, keep the 2M granularity. */
	if (avail_pages < num_pages || avail_pages - num_pages < floor) {
		pr_warn("Balloon request will be partially fulfilled. %s\n",
			avail_pages < num_pages ? "Not enough memory." :
			"Balloon floor reached.");

		num_pages = avail_pages > floor ? (avail_pages - floor) : 0;
		num_pages -= num_pages % PAGES_IN_2M;
	}

in balloon_up(). If 2M granularity is not guaranteed in the first place
we can't keep it.

Also, when alloc_balloon_pages() is called with 2M alloc_unit and the
region is not 2M aligned, it will return someething < num_pages, the
next condition, however, only checks for 0:

                if (alloc_unit != 1 && num_ballooned == 0) {
                        alloc_unit = 1;
                        continue;
                }

we will proceed to sending a response to server and try doing next
iteration by calling alloc_balloon_pages() with 2M alloc_unit again,
this will finally return 0 and we will switch to 4k. I think we can
optimize this to:

                if (alloc_unit != 1 && num_ballooned != num_pages) {
			alloc_unit = 1;
		        continue;
		}

-- 
Vitaly


^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [EXTERNAL] Re: [PATCH] x86/Hyper-V: Balloon up according to request page number
  2020-01-14  9:30 ` Vitaly Kuznetsov
@ 2020-01-16 14:17   ` " Tianyu Lan
  0 siblings, 0 replies; 3+ messages in thread
From: Tianyu Lan @ 2020-01-16 14:17 UTC (permalink / raw)
  To: vkuznets, lantianyu1986
  Cc: linux-kernel, linux-hyperv, linux-mm, stable, tglx, mingo, bp,
	hpa, x86, dave.hansen, luto, peterz, KY Srinivasan,
	Haiyang Zhang, Stephen Hemminger, sashal, akpm, Michael Kelley,
	Dexuan Cui

Hi Vitaly:
	Thanks for your review. I fix comments in V2. 

> From: linux-hyperv-owner@vger.kernel.org <linux-hyperv-
> owner@vger.kernel.org> On Behalf Of Vitaly Kuznetsov
> Sent: Tuesday, January 14, 2020 5:31 PM
> To: lantianyu1986@gmail.com
> Cc: linux-kernel@vger.kernel.org; linux-hyperv@vger.kernel.org; linux-
> mm@kvack.org; stable@vger.kernel.org; tglx@linutronix.de;
> mingo@redhat.com; bp@alien8.de; hpa@zytor.com; x86@kernel.org;
> dave.hansen@linux.intel.com; luto@kernel.org; peterz@infradead.org; KY
> Srinivasan <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>;
> Stephen Hemminger <sthemmin@microsoft.com>; sashal@kernel.org;
> akpm@linux-foundation.org; Michael Kelley <mikelley@microsoft.com>;
> Dexuan Cui <decui@microsoft.com>
> Subject: [EXTERNAL] Re: [PATCH] x86/Hyper-V: Balloon up according to request
> page number
> 
> lantianyu1986@gmail.com writes:
> 
> > From: Tianyu Lan <Tianyu.Lan@microsoft.com>
> >
> > Current code has assumption that balloon request memory size aligns
> > with 2MB. But actually Hyper-V doesn't guarantee such alignment. When
> > balloon driver receives non-aligned balloon request, it produces
> > warning and balloon up more memory than requested in order to keep 2MB
> alignment.
> > Remove the warning and balloon up memory according to actual requested
> > memory size.
> >
> > Fixes: f6712238471a ("hv: hv_balloon: avoid memory leak on alloc_error
> > of 2MB memory block")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> > ---
> >  drivers/hv/hv_balloon.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c index
> > 7f3e7ab22d5d..38ad0e44e927 100644
> > --- a/drivers/hv/hv_balloon.c
> > +++ b/drivers/hv/hv_balloon.c
> > @@ -1684,7 +1684,7 @@ static unsigned int alloc_balloon_pages(struct
> hv_dynmem_device *dm,
> >  	if (num_pages < alloc_unit)
> >  		return 0;
> >
> > -	for (i = 0; (i * alloc_unit) < num_pages; i++) {
> > +	for (i = 0; i < num_pages / alloc_unit; i++) {
> >  		if (bl_resp->hdr.size + sizeof(union dm_mem_page_range) >
> >  			HV_HYP_PAGE_SIZE)
> >  			return i * alloc_unit;
> > @@ -1722,7 +1722,7 @@ static unsigned int alloc_balloon_pages(struct
> > hv_dynmem_device *dm,
> >
> >  	}
> >
> > -	return num_pages;
> > +	return i * alloc_unit;
> >  }
> >
> >  static void balloon_up(union dm_msg_info *msg_info) @@ -1737,9
> > +1737,6 @@ static void balloon_up(union dm_msg_info *msg_info)
> >  	long avail_pages;
> >  	unsigned long floor;
> >
> > -	/* The host balloons pages in 2M granularity. */
> > -	WARN_ON_ONCE(num_pages % PAGES_IN_2M != 0);
> > -
> >  	/*
> >  	 * We will attempt 2M allocations. However, if we fail to
> >  	 * allocate 2M chunks, we will go back to PAGE_SIZE allocations.
> 
> This looks correct but I've noticed we also have
> 
> 	/* Refuse to balloon below the floor, keep the 2M granularity. */
> 	if (avail_pages < num_pages || avail_pages - num_pages < floor) {
> 		pr_warn("Balloon request will be partially fulfilled. %s\n",
> 			avail_pages < num_pages ? "Not enough memory." :
> 			"Balloon floor reached.");
> 
> 		num_pages = avail_pages > floor ? (avail_pages - floor) : 0;
> 		num_pages -= num_pages % PAGES_IN_2M;
> 	}
> 
> in balloon_up(). If 2M granularity is not guaranteed in the first place we can't
> keep it.
> 
> Also, when alloc_balloon_pages() is called with 2M alloc_unit and the region is
> not 2M aligned, it will return someething < num_pages, the next condition,
> however, only checks for 0:
> 
>                 if (alloc_unit != 1 && num_ballooned == 0) {
>                         alloc_unit = 1;
>                         continue;
>                 }
> 
> we will proceed to sending a response to server and try doing next iteration by
> calling alloc_balloon_pages() with 2M alloc_unit again, this will finally return 0
> and we will switch to 4k. I think we can optimize this to:
> 
>                 if (alloc_unit != 1 && num_ballooned != num_pages) {
> 			alloc_unit = 1;
> 		        continue;
> 		}
> 
> --
> Vitaly


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14  7:44 [PATCH] x86/Hyper-V: Balloon up according to request page number lantianyu1986
2020-01-14  9:30 ` Vitaly Kuznetsov
2020-01-16 14:17   ` [EXTERNAL] " Tianyu Lan

Linux-HyperV Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-hyperv/0 linux-hyperv/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-hyperv linux-hyperv/ https://lore.kernel.org/linux-hyperv \
		linux-hyperv@vger.kernel.org
	public-inbox-index linux-hyperv

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-hyperv


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git