Skip to contentSkip to conversation

Drivers: hv: balloon: Fix some issues in the balloon driver

PATCH In review K. Y. Srinivasan 2 participants 4 messages Jul 15, 2013

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v2 cover Drivers: hv: balloon: Fix some issues in the balloon driver

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 <[email protected]>.

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(-)
Signature · 1 line
1.7.4.1
#1 lore raw

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

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.

---
 drivers/hv/hv_balloon.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)
drivers/hv/hv_balloon.c +5 -8
index 4c605c7..61b7351 100644
@@ -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);
 
 	}
 
Signature · 1 line
1.7.4.1
#2 lore raw

PATCH v2 2/2 Drivers: hv: balloon: Do not post pressure status if interrupted

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.

---
 drivers/hv/hv_balloon.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
drivers/hv/hv_balloon.c +8 -0
index 61b7351..deb5c25 100644
@@ -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,
Signature · 1 line
1.7.4.1
Acked-by on v2 1/2
#3 lore raw

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

In reply to K. Y. Srinivasan
Quoted 35 lines · 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.

---
 drivers/hv/hv_balloon.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)
drivers/hv/hv_balloon.c +4 -2
index 4c605c7..61b7351 100644
@@ -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);

drivers/hv/hv_balloon.c static void hv_mem_hot_add(unsigned long start, unsigned long size, · line 617

Quoted 10 lines
drivers/hv/hv_balloon.c +1 -6
-		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);
 
 	}
 

Thanks