* [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code [not found] <1373637292-25855-1-git-send-email-kys@microsoft.com> @ 2013-07-12 13:56 ` K. Y. Srinivasan 2013-07-12 13:56 ` [PATCH 2/2] Drivers: hv: balloon: Do not post pressure status if interrupted K. Y. Srinivasan 2013-07-12 20:17 ` [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code Ben Hutchings 0 siblings, 2 replies; 6+ messages in thread From: K. Y. Srinivasan @ 2013-07-12 13:56 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, jasowang; +Cc: K. Y. Srinivasan, Stable 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 | 10 ++-------- 1 files changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c index 4c605c7..bfc7be2 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; @@ -608,13 +608,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size, /* * Wait for the memory block to be onlined. */ - 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] Drivers: hv: balloon: Do not post pressure status if interrupted 2013-07-12 13:56 ` [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code K. Y. Srinivasan @ 2013-07-12 13:56 ` K. Y. Srinivasan 2013-07-12 20:17 ` [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code Ben Hutchings 1 sibling, 0 replies; 6+ messages in thread From: K. Y. Srinivasan @ 2013-07-12 13:56 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, jasowang; +Cc: K. Y. Srinivasan, Stable 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 bfc7be2..4abaccf 100644 --- a/drivers/hv/hv_balloon.c +++ b/drivers/hv/hv_balloon.c @@ -972,6 +972,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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code 2013-07-12 13:56 ` [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code K. Y. Srinivasan 2013-07-12 13:56 ` [PATCH 2/2] Drivers: hv: balloon: Do not post pressure status if interrupted K. Y. Srinivasan @ 2013-07-12 20:17 ` Ben Hutchings 2013-07-12 21:07 ` KY Srinivasan 1 sibling, 1 reply; 6+ messages in thread From: Ben Hutchings @ 2013-07-12 20:17 UTC (permalink / raw) To: K. Y. Srinivasan; +Cc: gregkh, linux-kernel, devel, olaf, apw, jasowang, Stable On Fri, Jul 12, 2013 at 06:56:14AM -0700, 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. [...] > /* > * Wait for the memory block to be onlined. > */ > - 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); > > } > Well now it might look like a bug that you don't test the result of wait_for_completion_timeout(). Maybe update the comment to explain why it's OK to continue anyway? Ben. -- Ben Hutchings We get into the habit of living before acquiring the habit of thinking. - Albert Camus ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code 2013-07-12 20:17 ` [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code Ben Hutchings @ 2013-07-12 21:07 ` KY Srinivasan 2013-07-12 21:12 ` Ben Hutchings 0 siblings, 1 reply; 6+ messages in thread From: KY Srinivasan @ 2013-07-12 21:07 UTC (permalink / raw) To: Ben Hutchings Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, devel@linuxdriverproject.org, olaf@aepfle.de, apw@canonical.com, jasowang@redhat.com, Stable > -----Original Message----- > From: Ben Hutchings [mailto:ben@decadent.org.uk] > Sent: Friday, July 12, 2013 4:17 PM > To: KY Srinivasan > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; > jasowang@redhat.com; Stable > Subject: Re: [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code > > On Fri, Jul 12, 2013 at 06:56:14AM -0700, 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. > [...] > > /* > > * Wait for the memory block to be onlined. > > */ > > - 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); > > > > } > > > > Well now it might look like a bug that you don't test the result > of wait_for_completion_timeout(). Maybe update the comment to > explain why it's OK to continue anyway? I put in the comment in the patch explaining why it is ok to continue. To reiterate, it is ok to continue because hot add has succeeded. More importantly, what I was doing earlier - rolling back the state when in fact hot add had succeeded was incorrect. Regards, K. Y > > Ben. > > -- > Ben Hutchings > We get into the habit of living before acquiring the habit of thinking. > - Albert Camus > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code 2013-07-12 21:07 ` KY Srinivasan @ 2013-07-12 21:12 ` Ben Hutchings 2013-07-14 2:03 ` KY Srinivasan 0 siblings, 1 reply; 6+ messages in thread From: Ben Hutchings @ 2013-07-12 21:12 UTC (permalink / raw) To: KY Srinivasan Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, devel@linuxdriverproject.org, olaf@aepfle.de, apw@canonical.com, jasowang@redhat.com, Stable On Fri, Jul 12, 2013 at 09:07:19PM +0000, KY Srinivasan wrote: [...] > > Well now it might look like a bug that you don't test the result > > of wait_for_completion_timeout(). Maybe update the comment to > > explain why it's OK to continue anyway? > > I put in the comment in the patch explaining why it is ok to continue. [...] But that is not nearly as easy to see as the comment that is already *in the code* which your patch isn't updating. Ben. -- Ben Hutchings We get into the habit of living before acquiring the habit of thinking. - Albert Camus ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code 2013-07-12 21:12 ` Ben Hutchings @ 2013-07-14 2:03 ` KY Srinivasan 0 siblings, 0 replies; 6+ messages in thread From: KY Srinivasan @ 2013-07-14 2:03 UTC (permalink / raw) To: Ben Hutchings Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, devel@linuxdriverproject.org, olaf@aepfle.de, apw@canonical.com, jasowang@redhat.com, Stable > -----Original Message----- > From: Ben Hutchings [mailto:ben@decadent.org.uk] > Sent: Friday, July 12, 2013 5:13 PM > To: KY Srinivasan > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; > jasowang@redhat.com; Stable > Subject: Re: [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code > > On Fri, Jul 12, 2013 at 09:07:19PM +0000, KY Srinivasan wrote: > [...] > > > Well now it might look like a bug that you don't test the result > > > of wait_for_completion_timeout(). Maybe update the comment to > > > explain why it's OK to continue anyway? > > > > I put in the comment in the patch explaining why it is ok to continue. > [...] > > But that is not nearly as easy to see as the comment that is > already *in the code* which your patch isn't updating. Agreed; I will resend the patch with comments added. Thanks, K. Y > > Ben. > > -- > Ben Hutchings > We get into the habit of living before acquiring the habit of thinking. > - Albert Camus > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-07-14 2:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1373637292-25855-1-git-send-email-kys@microsoft.com>
2013-07-12 13:56 ` [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code K. Y. Srinivasan
2013-07-12 13:56 ` [PATCH 2/2] Drivers: hv: balloon: Do not post pressure status if interrupted K. Y. Srinivasan
2013-07-12 20:17 ` [PATCH 1/2] Drivers: hv: balloon: Fix a bug in the hot-add code Ben Hutchings
2013-07-12 21:07 ` KY Srinivasan
2013-07-12 21:12 ` Ben Hutchings
2013-07-14 2:03 ` KY Srinivasan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox