public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "K. Y. Srinivasan" <kys@microsoft.com>
To: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
	devel@linuxdriverproject.org, olaf@aepfle.de, apw@canonical.com,
	vkuznets@redhat.com, jasowang@redhat.com
Cc: Vitaly Kuznetsov 
	<"[mailto:vkuznets@redhat.com]"@linuxonhyperv.com>,
	"K. Y. Srinivasan" <kys@microsoft.com>
Subject: [PATCH 8/8] Drivers: hv: vmbus: handle various crash scenarios
Date: Tue,  5 Apr 2016 16:57:47 -0700	[thread overview]
Message-ID: <1459900667-20367-8-git-send-email-kys@microsoft.com> (raw)
In-Reply-To: <1459900667-20367-1-git-send-email-kys@microsoft.com>

From: Vitaly Kuznetsov <[mailto:vkuznets@redhat.com]>

Kdump keeps biting. Turns out CHANNELMSG_UNLOAD_RESPONSE is always
delivered to the CPU which was used for initial contact or to CPU0
depending on host version. vmbus_wait_for_unload() doesn't account for
the fact that in case we're crashing on some other CPU we won't get the
CHANNELMSG_UNLOAD_RESPONSE message and our wait on the current CPU will
never end.

Do the following:
1) Check for completion_done() in the loop. In case interrupt handler is
   still alive we'll get the confirmation we need.

2) Read message pages for all CPUs message page as we're unsure where
   CHANNELMSG_UNLOAD_RESPONSE is going to be delivered to. We can race with
   still-alive interrupt handler doing the same, add cmpxchg() to
   vmbus_signal_eom() to not lose CHANNELMSG_UNLOAD_RESPONSE message.

3) Cleanup message pages on all CPUs. This is required (at least for the
   current CPU as we're clearing CPU0 messages now but we may want to bring
   up additional CPUs on crash) as new messages won't be delivered till we
   consume what's pending. On boot we'll place message pages somewhere else
   and we won't be able to read stale messages.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c |   58 +++++++++++++++++++++++++++++++++-----------
 drivers/hv/hyperv_vmbus.h |   16 +++++++++++-
 drivers/hv/vmbus_drv.c    |    7 +++--
 3 files changed, 61 insertions(+), 20 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 38b682ba..b6c1211 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -597,27 +597,55 @@ static void init_vp_index(struct vmbus_channel *channel, u16 dev_type)
 
 static void vmbus_wait_for_unload(void)
 {
-	int cpu = smp_processor_id();
-	void *page_addr = hv_context.synic_message_page[cpu];
-	struct hv_message *msg = (struct hv_message *)page_addr +
-				  VMBUS_MESSAGE_SINT;
+	int cpu;
+	void *page_addr;
+	struct hv_message *msg;
 	struct vmbus_channel_message_header *hdr;
-	bool unloaded = false;
+	u32 message_type;
 
+	/*
+	 * CHANNELMSG_UNLOAD_RESPONSE is always delivered to the CPU which was
+	 * used for initial contact or to CPU0 depending on host version. When
+	 * we're crashing on a different CPU let's hope that IRQ handler on
+	 * the cpu which receives CHANNELMSG_UNLOAD_RESPONSE is still
+	 * functional and vmbus_unload_response() will complete
+	 * vmbus_connection.unload_event. If not, the last thing we can do is
+	 * read message pages for all CPUs directly.
+	 */
 	while (1) {
-		if (READ_ONCE(msg->header.message_type) == HVMSG_NONE) {
-			mdelay(10);
-			continue;
-		}
+		if (completion_done(&vmbus_connection.unload_event))
+			break;
 
-		hdr = (struct vmbus_channel_message_header *)msg->u.payload;
-		if (hdr->msgtype == CHANNELMSG_UNLOAD_RESPONSE)
-			unloaded = true;
+		for_each_online_cpu(cpu) {
+			page_addr = hv_context.synic_message_page[cpu];
+			msg = (struct hv_message *)page_addr +
+				VMBUS_MESSAGE_SINT;
 
-		vmbus_signal_eom(msg);
+			message_type = READ_ONCE(msg->header.message_type);
+			if (message_type == HVMSG_NONE)
+				continue;
 
-		if (unloaded)
-			break;
+			hdr = (struct vmbus_channel_message_header *)
+				msg->u.payload;
+
+			if (hdr->msgtype == CHANNELMSG_UNLOAD_RESPONSE)
+				complete(&vmbus_connection.unload_event);
+
+			vmbus_signal_eom(msg, message_type);
+		}
+
+		mdelay(10);
+	}
+
+	/*
+	 * We're crashing and already got the UNLOAD_RESPONSE, cleanup all
+	 * maybe-pending messages on all CPUs to be able to receive new
+	 * messages after we reconnect.
+	 */
+	for_each_online_cpu(cpu) {
+		page_addr = hv_context.synic_message_page[cpu];
+		msg = (struct hv_message *)page_addr + VMBUS_MESSAGE_SINT;
+		msg->header.message_type = HVMSG_NONE;
 	}
 }
 
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index e5203e4..718b5c7 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -625,9 +625,21 @@ extern struct vmbus_channel_message_table_entry
 	channel_message_table[CHANNELMSG_COUNT];
 
 /* Free the message slot and signal end-of-message if required */
-static inline void vmbus_signal_eom(struct hv_message *msg)
+static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
 {
-	msg->header.message_type = HVMSG_NONE;
+	/*
+	 * On crash we're reading some other CPU's message page and we need
+	 * to be careful: this other CPU may already had cleared the header
+	 * and the host may already had delivered some other message there.
+	 * In case we blindly write msg->header.message_type we're going
+	 * to lose it. We can still lose a message of the same type but
+	 * we count on the fact that there can only be one
+	 * CHANNELMSG_UNLOAD_RESPONSE and we don't care about other messages
+	 * on crash.
+	 */
+	if (cmpxchg(&msg->header.message_type, old_msg_type,
+		    HVMSG_NONE) != old_msg_type)
+		return;
 
 	/*
 	 * Make sure the write to MessageType (ie set to
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index a29a6c0..952f20f 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -712,7 +712,7 @@ static void hv_process_timer_expiration(struct hv_message *msg, int cpu)
 	if (dev->event_handler)
 		dev->event_handler(dev);
 
-	vmbus_signal_eom(msg);
+	vmbus_signal_eom(msg, HVMSG_TIMER_EXPIRED);
 }
 
 void vmbus_on_msg_dpc(unsigned long data)
@@ -724,8 +724,9 @@ void vmbus_on_msg_dpc(unsigned long data)
 	struct vmbus_channel_message_header *hdr;
 	struct vmbus_channel_message_table_entry *entry;
 	struct onmessage_work_context *ctx;
+	u32 message_type = msg->header.message_type;
 
-	if (msg->header.message_type == HVMSG_NONE)
+	if (message_type == HVMSG_NONE)
 		/* no msg */
 		return;
 
@@ -750,7 +751,7 @@ void vmbus_on_msg_dpc(unsigned long data)
 		entry->message_handler(hdr);
 
 msg_handled:
-	vmbus_signal_eom(msg);
+	vmbus_signal_eom(msg, message_type);
 }
 
 static void vmbus_isr(void)
-- 
1.7.4.1

  parent reply	other threads:[~2016-04-05 22:20 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-05 23:57 [PATCH 0/8] Drivers: hv: Miscellaneous vmbus and util driver fixes K. Y. Srinivasan
2016-04-05 23:57 ` [PATCH 1/8] Drivers: hv: kvp: fix IP Failover K. Y. Srinivasan
2016-04-05 23:57   ` [PATCH 2/8] Drivers: hv: vmbus: Introduce functions for estimating room in the ring buffer K. Y. Srinivasan
2016-04-05 23:57   ` [PATCH 3/8] Drivers: hv: vmbus: Use READ_ONCE() to read variables that are volatile K. Y. Srinivasan
2016-04-05 23:57   ` [PATCH 4/8] Drivers: hv: vmbus: Use the new virt_xx barrier code K. Y. Srinivasan
2016-04-05 23:57   ` [PATCH 5/8] Drivers: hv: vmbus: Export the vmbus_set_event() API K. Y. Srinivasan
2016-04-05 23:57   ` [PATCH 6/8] Drivers: hv: vmbus: Move some ring buffer functions to hyperv.h K. Y. Srinivasan
2016-04-05 23:57   ` [PATCH 7/8] Drivers: hv: vmbus: Implement APIs to support "in place" consumption of vmbus packets K. Y. Srinivasan
2016-04-05 23:57   ` K. Y. Srinivasan [this message]
2016-04-30 21:04   ` [PATCH 1/8] Drivers: hv: kvp: fix IP Failover Greg KH
2016-04-30 21:43     ` KY Srinivasan
2016-04-30 21:54       ` Greg KH
2016-05-01  0:21         ` KY Srinivasan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1459900667-20367-8-git-send-email-kys@microsoft.com \
    --to=kys@microsoft.com \
    --cc="[mailto:vkuznets@redhat.com]"@linuxonhyperv.com \
    --cc=apw@canonical.com \
    --cc=devel@linuxdriverproject.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jasowang@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=olaf@aepfle.de \
    --cc=vkuznets@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox