[PATCH 0/2] Drivers: hv: balloon: two memory hotplug fixes

[PATCH 0/2] Drivers: hv: balloon: two memory hotplug fixes

[Vitaly Kuznetsov]

Hi,

I found an issue (crash) while testing memory hotplug on WS2016TP4 host.
It should be easily reproducible with the following test sequence
 (4.6-rc2):

1) Start a VM with 4 vCPUs in 4 NUMA nodes, 4096Mb of RAM.
2) Enable memory auto onlining with 
   'echo online > /sys/devices/system/memory/auto_online_blocks'
3) Wait till hv_balloon driver finishes negotiation.
4) Increase memory to 8192Mb.
5) Increase memory to 10000Mb.

The first patch in the series fixes the issue. While writing it I stumbled
upon an unrelated issue which I fix with PATCH 2.

Please review.

Vitaly Kuznetsov (2):
  Drivers: hv: balloon: don't crash when memory is added in non-sorted
    order
  Drivers: hv: balloon: reset host_specified_ha_region

 drivers/hv/hv_balloon.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
2.5.5

[PATCH 1/2] Drivers: hv: balloon: don't crash when memory is added in non-sorted order

[Vitaly Kuznetsov]

When we iterate through all HA regions in handle_pg_range() we have an
assumption that all these regions are sorted in the list and the
'start_pfn >= has->end_pfn' check is enough to find the proper region.
Unfortunately it's not the case with WS2016 where host can hot-add regions
in a different order. We end up modifying the wrong HA region and crashing
later on pages online. Modify the check to make sure we found the region
we were searching for while iterating. Fix the same check in pfn_covered()
as well.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 drivers/hv/hv_balloon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index b853b4b..43af913 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -714,7 +714,7 @@ static bool pfn_covered(unsigned long start_pfn, unsigned long pfn_cnt)
 		 * If the pfn range we are dealing with is not in the current
 		 * "hot add block", move on.
 		 */
-		if ((start_pfn >= has->end_pfn))
+		if (start_pfn < has->start_pfn || start_pfn >= has->end_pfn)
 			continue;
 		/*
 		 * If the current hot add-request extends beyond
@@ -768,7 +768,7 @@ static unsigned long handle_pg_range(unsigned long pg_start,
 		 * If the pfn range we are dealing with is not in the current
 		 * "hot add block", move on.
 		 */
-		if ((start_pfn >= has->end_pfn))
+		if (start_pfn < has->start_pfn || start_pfn >= has->end_pfn)
 			continue;
 
 		old_covered_state = has->covered_end_pfn;
-- 
2.5.5

[PATCH 2/2] Drivers: hv: balloon: reset host_specified_ha_region

[Vitaly Kuznetsov]

We set host_specified_ha_region = true on certain request but this is a
global state which stays 'true' forever. We need to reset it when we
receive a request where ha_region is not specified. I did not see any
real issues, the bug was found by code inspection.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 drivers/hv/hv_balloon.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 43af913..df35fb7 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -1400,6 +1400,7 @@ static void balloon_onchannelcallback(void *context)
 				 * This is a normal hot-add request specifying
 				 * hot-add memory.
 				 */
+				dm->host_specified_ha_region = false;
 				ha_pg_range = &ha_msg->range;
 				dm->ha_wrk.ha_page_range = *ha_pg_range;
 				dm->ha_wrk.ha_region_range.page_range = 0;
-- 
2.5.5

Re: [PATCH 0/2] Drivers: hv: balloon: two memory hotplug fixes

[Vitaly Kuznetsov]

Vitaly Kuznetsov <vkuznets@redhat.com> writes:

> Hi,
>
> I found an issue (crash) while testing memory hotplug on WS2016TP4 host.
> It should be easily reproducible with the following test sequence
>  (4.6-rc2):
>
> 1) Start a VM with 4 vCPUs in 4 NUMA nodes, 4096Mb of RAM.
> 2) Enable memory auto onlining with 
>    'echo online > /sys/devices/system/memory/auto_online_blocks'
> 3) Wait till hv_balloon driver finishes negotiation.
> 4) Increase memory to 8192Mb.
> 5) Increase memory to 10000Mb.
>
> The first patch in the series fixes the issue. While writing it I stumbled
> upon an unrelated issue which I fix with PATCH 2.

K. Y.,

it seems these two patches were lost. Should I resend them?

>
> Please review.
>
> Vitaly Kuznetsov (2):
>   Drivers: hv: balloon: don't crash when memory is added in non-sorted
>     order
>   Drivers: hv: balloon: reset host_specified_ha_region
>
>  drivers/hv/hv_balloon.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

-- 
  Vitaly

RE: [PATCH 0/2] Drivers: hv: balloon: two memory hotplug fixes

[KY Srinivasan]

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Monday, April 18, 2016 5:59 AM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; Haiyang Zhang
> <haiyangz@microsoft.com>; Alex Ng (LIS) <alexng@microsoft.com>; Cathy
> Avery <cavery@redhat.com>; devel@linuxdriverproject.org
> Subject: Re: [PATCH 0/2] Drivers: hv: balloon: two memory hotplug fixes
> 
> Vitaly Kuznetsov <vkuznets@redhat.com> writes:
> 
> > Hi,
> >
> > I found an issue (crash) while testing memory hotplug on WS2016TP4 host.
> > It should be easily reproducible with the following test sequence
> >  (4.6-rc2):
> >
> > 1) Start a VM with 4 vCPUs in 4 NUMA nodes, 4096Mb of RAM.
> > 2) Enable memory auto onlining with
> >    'echo online > /sys/devices/system/memory/auto_online_blocks'
> > 3) Wait till hv_balloon driver finishes negotiation.
> > 4) Increase memory to 8192Mb.
> > 5) Increase memory to 10000Mb.
> >
> > The first patch in the series fixes the issue. While writing it I stumbled
> > upon an unrelated issue which I fix with PATCH 2.
> 
> K. Y.,
> 
> it seems these two patches were lost. Should I resend them?

No I have them. I will be sending these soon.

K. Y
> 
> >
> > Please review.
> >
> > Vitaly Kuznetsov (2):
> >   Drivers: hv: balloon: don't crash when memory is added in non-sorted
> >     order
> >   Drivers: hv: balloon: reset host_specified_ha_region
> >
> >  drivers/hv/hv_balloon.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> --
>   Vitaly