Re: [PATCH V2 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code

Re: [PATCH V2 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code

[Jason Wang]

On 07/15/2013 01:38 PM, K. Y. Srinivasan wrote:
> As we hot-add 128 MB chunks of memory, we wait to ensure that the memory
> is onlined before attempting to hot-add the next chunk. If the udev rule for
> memory hot-add is not executed within the allowed time, we would rollback the
> state and abort further hot-add. Since the hot-add has succeeded and the only
> failure is that the memory is not onlined within the allowed time, we should not
> be rolling back the state. Fix this bug.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Cc: Stable <stable@vger.kernel.org>
> ---
>  drivers/hv/hv_balloon.c |   13 +++++--------
>  1 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index 4c605c7..61b7351 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -562,7 +562,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
>  				struct hv_hotadd_state *has)
>  {
>  	int ret = 0;
> -	int i, nid, t;
> +	int i, nid;
>  	unsigned long start_pfn;
>  	unsigned long processed_pfn;
>  	unsigned long total_pfn = pfn_count;
> @@ -607,14 +607,11 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
>  
>  		/*
>  		 * Wait for the memory block to be onlined.
> +		 * Since the hot add has succeeded, it is ok to
> +		 * proceed even if the pages in the hot added region
> +		 * have not been "onlined" within the allowed time.
>  		 */
> -		t = wait_for_completion_timeout(&dm_device.ol_waitevent, 5*HZ);
> -		if (t == 0) {
> -			pr_info("hot_add memory timedout\n");
> -			has->ha_end_pfn -= HA_CHUNK;
> -			has->covered_end_pfn -=  processed_pfn;
> -			break;
> -		}
> +		wait_for_completion_timeout(&dm_device.ol_waitevent, 5*HZ);
>  
>  	}
>  

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

[PATCH V2 0/2] Drivers: hv: balloon: Fix some issues in the balloon driver

[K. Y. Srinivasan]

This patch set adresses a couple of issues discovered in the balloon driver:

1. Prevent stale pressure report being posted.
2. If memory is not brought online within the allowed time,
   do not roll back hot add state if hot add succeeded.

In this version of the patch set, I have added additional comments to the
patch: 0001-Drivers-hv-balloon-Fix-a-bug-in-the-hot-add-code.patch based on
the comments from Ben Hutchings <ben@decadent.org.uk>.

K. Y. Srinivasan (2):
  Drivers: hv: balloon: Fix a bug in the hot-add code
  Drivers: hv: balloon: Do not post pressure status if interrupted

 drivers/hv/hv_balloon.c |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)

-- 
1.7.4.1

[PATCH V2 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code

[K. Y. Srinivasan]

As we hot-add 128 MB chunks of memory, we wait to ensure that the memory
is onlined before attempting to hot-add the next chunk. If the udev rule for
memory hot-add is not executed within the allowed time, we would rollback the
state and abort further hot-add. Since the hot-add has succeeded and the only
failure is that the memory is not onlined within the allowed time, we should not
be rolling back the state. Fix this bug.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Cc: Stable <stable@vger.kernel.org>
---
 drivers/hv/hv_balloon.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 4c605c7..61b7351 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -562,7 +562,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
 				struct hv_hotadd_state *has)
 {
 	int ret = 0;
-	int i, nid, t;
+	int i, nid;
 	unsigned long start_pfn;
 	unsigned long processed_pfn;
 	unsigned long total_pfn = pfn_count;
@@ -607,14 +607,11 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
 
 		/*
 		 * Wait for the memory block to be onlined.
+		 * Since the hot add has succeeded, it is ok to
+		 * proceed even if the pages in the hot added region
+		 * have not been "onlined" within the allowed time.
 		 */
-		t = wait_for_completion_timeout(&dm_device.ol_waitevent, 5*HZ);
-		if (t == 0) {
-			pr_info("hot_add memory timedout\n");
-			has->ha_end_pfn -= HA_CHUNK;
-			has->covered_end_pfn -=  processed_pfn;
-			break;
-		}
+		wait_for_completion_timeout(&dm_device.ol_waitevent, 5*HZ);
 
 	}
 
-- 
1.7.4.1

[PATCH V2 2/2] Drivers: hv: balloon: Do not post pressure status if interrupted

[K. Y. Srinivasan]

When we are posting pressure status, we may get interrupted and handle
the un-balloon operation. In this case just don't post the status as we
know the pressure status is stale.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Cc: Stable <stable@vger.kernel.org>
---
 drivers/hv/hv_balloon.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 61b7351..deb5c25 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -975,6 +975,14 @@ static void post_status(struct hv_dynmem_device *dm)
 				dm->num_pages_ballooned +
 				compute_balloon_floor();
 
+	/*
+	 * If our transaction ID is no longer current, just don't
+	 * send the status. This can happen if we were interrupted
+	 * after we picked our transaction ID.
+	 */
+	if (status.hdr.trans_id != atomic_read(&trans_id))
+		return;
+
 	vmbus_sendpacket(dm->dev->channel, &status,
 				sizeof(struct dm_status),
 				(unsigned long)NULL,
-- 
1.7.4.1