| Message ID | 20200114074435.12732-1-Tianyu.Lan@microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/Hyper-V: Balloon up according to request page number | expand |
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; }
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
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.