[PATCH 0/1] Drivers: hv: hv_balloon: do not online pages in offline blocks

[PATCH 0/1] Drivers: hv: hv_balloon: do not online pages in offline blocks

[Vitaly Kuznetsov]

Patch 1/1 is rather a hackaround. The root cause of the issue is the fact that
we're adding not fully backed memory blocks and allow to online them. It is 
relatively easy to forbid that and add only fully backed 128Mb regions. The whole
driver code will be much simpler: hv_bring_pgs_online() and hv_online_page()
will go away, ha_region_mutex won't probably be required,... Please let me know
if you think I should follow this way.

Vitaly Kuznetsov (1):
  Drivers: hv: hv_balloon: do not online pages in offline blocks

 drivers/hv/hv_balloon.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

-- 
1.9.3

[PATCH 1/1] Drivers: hv: hv_balloon: do not online pages in offline blocks

[Vitaly Kuznetsov]

Currently we add memory in 128Mb blocks but the request from host can be
aligned differently. In such case we add a partially backed block and
when this block goes online we skip onlining pages which are not backed
(hv_online_page() callback serves this purpose). When we receive next
request for the same host add region we online pages which were not backed
before with hv_bring_pgs_online(). However, we don't check if the the block
in question was onlined and online this tail unconditionally. This is bad as
we avoid all online_pages() logic: these pages are not accounted, we don't
send notifications (and hv_balloon is not the only receiver of them),...
And, first of all, nobody asked as to online these pages. Solve the issue by
checking if the last previously backed page was onlined and onlining the tail
only in case it was.

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

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 014256a..99afef9 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -778,7 +778,17 @@ static unsigned long handle_pg_range(unsigned long pg_start,
 			pgs_ol = has->ha_end_pfn - start_pfn;
 			if (pgs_ol > pfn_cnt)
 				pgs_ol = pfn_cnt;
-			hv_bring_pgs_online(start_pfn, pgs_ol);
+
+			/*
+			 * Check if the corresponding memory block is already
+			 * online by checking its last previously backed page.
+			 * In case it is we need to bring rest (which was not
+			 * backed previously) online too.
+			 */
+			if (start_pfn > has->start_pfn &&
+			    !PageReserved(pfn_to_page(start_pfn - 1)))
+				hv_bring_pgs_online(start_pfn, pgs_ol);
+
 			has->covered_end_pfn +=  pgs_ol;
 			pfn_cnt -= pgs_ol;
 		}
-- 
1.9.3

RE: [PATCH 0/1] Drivers: hv: hv_balloon: do not online pages in offline blocks

[KY Srinivasan]

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Wednesday, March 25, 2015 11:02 AM
> To: KY Srinivasan
> Cc: Haiyang Zhang; devel@linuxdriverproject.org; linux-
> kernel@vger.kernel.org; Dexuan Cui
> Subject: [PATCH 0/1] Drivers: hv: hv_balloon: do not online pages in offline
> blocks
> 
> Patch 1/1 is rather a hackaround. The root cause of the issue is the fact that
> we're adding not fully backed memory blocks and allow to online them. It is
> relatively easy to forbid that and add only fully backed 128Mb regions. The
> whole
> driver code will be much simpler: hv_bring_pgs_online() and
> hv_online_page()
> will go away, ha_region_mutex won't probably be required,... Please let me
> know
> if you think I should follow this way.

Vitaly,

The main issue I had to address here was that Windows has a 2M alignment for memory
hot add while Linux has a 128M alignment requirement. Furthermore, we will also have to deal with
the case where only a partial 128MB block might be hot-added. The strategy I chose of selectively 
onlining pages in a properly aligned block that would be hot added was to address the above two issues.

Regards,

K. Y
> 
> Vitaly Kuznetsov (1):
>   Drivers: hv: hv_balloon: do not online pages in offline blocks
> 
>  drivers/hv/hv_balloon.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> --
> 1.9.3

Re: [PATCH 0/1] Drivers: hv: hv_balloon: do not online pages in offline blocks

[Vitaly Kuznetsov]

KY Srinivasan <kys@microsoft.com> writes:

>> -----Original Message-----
>> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
>> Sent: Wednesday, March 25, 2015 11:02 AM
>> To: KY Srinivasan
>> Cc: Haiyang Zhang; devel@linuxdriverproject.org; linux-
>> kernel@vger.kernel.org; Dexuan Cui
>> Subject: [PATCH 0/1] Drivers: hv: hv_balloon: do not online pages in offline
>> blocks
>> 
>> Patch 1/1 is rather a hackaround. The root cause of the issue is the fact that
>> we're adding not fully backed memory blocks and allow to online them. It is
>> relatively easy to forbid that and add only fully backed 128Mb regions. The
>> whole
>> driver code will be much simpler: hv_bring_pgs_online() and
>> hv_online_page()
>> will go away, ha_region_mutex won't probably be required,... Please let me
>> know
>> if you think I should follow this way.
>
> Vitaly,
>
> The main issue I had to address here was that Windows has a 2M alignment for memory
> hot add while Linux has a 128M alignment requirement. Furthermore, we will also have to deal with
> the case where only a partial 128MB block might be hot-added. The strategy I chose of selectively 
> onlining pages in a properly aligned block that would be hot added was
> to address the above two issues.

Yes, we can keep things this was, basically everyhting works. I just
wanted to highlight the other possible solution for the issue which is
also a major simplification for the driver.

So what do you think about my patch? We definitely should avoid onlining
pages in offline blocks.

>
> Regards,
>
> K. Y
>> 
>> Vitaly Kuznetsov (1):
>>   Drivers: hv: hv_balloon: do not online pages in offline blocks
>> 
>>  drivers/hv/hv_balloon.c | 12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>> 
>> --
>> 1.9.3

-- 
  Vitaly

RE: [PATCH 0/1] Drivers: hv: hv_balloon: do not online pages in offline blocks

[KY Srinivasan]

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Thursday, March 26, 2015 2:40 AM
> To: KY Srinivasan
> Cc: Haiyang Zhang; devel@linuxdriverproject.org; linux-
> kernel@vger.kernel.org; Dexuan Cui
> Subject: Re: [PATCH 0/1] Drivers: hv: hv_balloon: do not online pages in
> offline blocks
> 
> KY Srinivasan <kys@microsoft.com> writes:
> 
> >> -----Original Message-----
> >> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> >> Sent: Wednesday, March 25, 2015 11:02 AM
> >> To: KY Srinivasan
> >> Cc: Haiyang Zhang; devel@linuxdriverproject.org; linux-
> >> kernel@vger.kernel.org; Dexuan Cui
> >> Subject: [PATCH 0/1] Drivers: hv: hv_balloon: do not online pages in
> offline
> >> blocks
> >>
> >> Patch 1/1 is rather a hackaround. The root cause of the issue is the fact
> that
> >> we're adding not fully backed memory blocks and allow to online them. It
> is
> >> relatively easy to forbid that and add only fully backed 128Mb regions. The
> >> whole
> >> driver code will be much simpler: hv_bring_pgs_online() and
> >> hv_online_page()
> >> will go away, ha_region_mutex won't probably be required,... Please let
> me
> >> know
> >> if you think I should follow this way.
> >
> > Vitaly,
> >
> > The main issue I had to address here was that Windows has a 2M alignment
> for memory
> > hot add while Linux has a 128M alignment requirement. Furthermore, we
> will also have to deal with
> > the case where only a partial 128MB block might be hot-added. The
> strategy I chose of selectively
> > onlining pages in a properly aligned block that would be hot added was
> > to address the above two issues.
> 
> Yes, we can keep things this was, basically everyhting works. I just
> wanted to highlight the other possible solution for the issue which is
> also a major simplification for the driver.
> 
> So what do you think about my patch? We definitely should avoid onlining
> pages in offline blocks.

Yes; looks good.

K. Y
> 
> >
> > Regards,
> >
> > K. Y
> >>
> >> Vitaly Kuznetsov (1):
> >>   Drivers: hv: hv_balloon: do not online pages in offline blocks
> >>
> >>  drivers/hv/hv_balloon.c | 12 +++++++++++-
> >>  1 file changed, 11 insertions(+), 1 deletion(-)
> >>
> >> --
> >> 1.9.3
> 
> --
>   Vitaly